Exit code. about zora HOT 4 CLOSED

Hi @Raynos

We have already debated about exit code. You can read my take on it

But to sum up.

1. Adding env detection code can mislead some tools. I believe it is less the case nowadays.
2. Saying a failing test should result in a exit code different than 0 is an opinion that may not be shared by every user (although it is the case for many CI platforms)

I am not very enthusiastic setting this behavior as default given there are others ways to handle it:

1. If you are using node, you can use a specific test runner: pta is a good one
2. You can embrace unix philosophy and delegate to a specific reporting process node test.js | tap-set-exit
3. You can create you own harness (like a tiny test runner)

//test/util.js
const {createHarness} = require('zora');

const harness = createHarness();
const {test} = harness;

// auto start, that does not need to be the case if you wish not
setTimeout(async () => {
  await test.report();
  process.exitCode = harness.pass ? 0 : 1;
},0)

module.exports = test;

and

//test/foo.js
const test = require('./util.js');

test(`blah`, () => { /* ... */})

In what way do you feel blocked by the lack of exit code in zora's code base ?

from zora.

In what way do you feel blocked by the lack of exit code in zora's code base ?

I wish to switch from tape to zora and not break too many of my coworkers behaviors.

Having to remember to do node test/index.js | tap-set-exit is frustrating and adds another dependency when node test/index.js with tape already works out of the box.

I can't imagine any downsides for setting the exit code. It's actually super useful by default to do nice things like

while node test/index.js ; do echo next; done

^ This re-runs my test until the flappy failing test fails.

Other things are like:

{
  "scripts": {
    "publish": "node test/index.js && publish"
  }
}

This short circuits the publish if the tests are failing.

from zora.

You have valid points but please consider:

I wish to switch from tape to zora and not break too many of my coworkers behaviors.

Be careful, zora is not a drop in replacement for tape. For example, in zora there is no t.end or t.plan. So you will have to modify your spec files in some way.

Having to remember to do node test/index.js | tap-set-exit is frustrating and adds another dependency when node test/index.js with tape already works out of the box.

Compared to the flexibility it brings, I don't think adding a small dependency and change one line in a package.json is much of an effort, especially if you need to modify your testing program anyway.
Also you probably already use a custom reporter with tape... which eventually handle exit codes.
zora is not tape, so if you want to use zora maybe you should use it accordingly to its guideline (I mean no offense of course).

I think you misunderstood the target of this project. Zora is not a test runner: it is a library to write Javascript testing programs. It does not make any assumption on how the program is run (browser, node, tape - yes you can use tape as test runner for zora programs ;), custom test runners, spawn node processes, bash, etc).

Exit codes are platform specific, opinionated and in my opinion should be handled by the test runner.

If you need a test runner to run zora programs which handles exit code the way you wish, you can use:

• pta I think it would be the obvious choice in your case
• tape (tape test/*.js)
• pipe the tap stream into any reporter specific to nodejs
• build a simple custom test runner tailored for your needs and eventually publish it (see below) or more examples in this article

#!/usr/bin/env node
const arg = require('arg');
const globby = require('globby');
const {createHarness} = require('zora');
const path = require('path');

const DEFAULT_FILE_PATTERN = ['./test/*.spec.js'];

(async function () {
    const {_: filePatternArg, ['--only']: runOnly} = arg({'--only': Boolean, '-o': '--only'}, {
        permissive: false,
        argv: process.argv.slice(2)
    });

    const filePattern = filePatternArg.length > 0 ? filePatternArg : DEFAULT_FILE_PATTERN;

    // create a custom test harness
    const testHarness = createHarness({
        indent: true,
        runOnly
    });
    try {
        const files = await globby(filePattern);
        for (const f of files) {
            testHarness.test(f, require(path.resolve(process.cwd(), f)));
        }
        // force indented reporting
        await testHarness.report();
    } catch (e) {
        console.error(e);
        process.exit(1);
    } finally {
        process.exit(testHarness.pass ? 0 : 1);
    }
})();

I can't imagine any downsides for setting the exit code. It's actually super useful by default to do nice things like

Because you focus on your use case. Again zora does not make any assumption on how you run your program:

First of all, even if you are having failing tests, if the program manages to run to its completion there is no obvious reason to exit with something other than 0, especially if you have some other processes downstream using the TAP output or if the zora program was launched with some parent process. It also allows to make the difference with an unexpected error (which will make the Nodejs process crash and give a different exit code).

Your bash examples are very nice but I don't see how the actual state of zora is blocking

{
  "scripts": {
    "test": "node test/index.js",
    "test:ci": "npm t | tap-set-exit",
    "publish": "npm run test:ci && publish"
  }
}

To sum up, exit codes are not in the scope of this package as it will definitely affect the current flexibility. It will unlikely change given you have a lot of simple alternatives.

related: #20

from zora.

I respect that you've thought this through very thoroughly.

I'll agree to disagree :)

from zora.