Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Tests exit with error code 0 #9

Open
piuccio opened this issue Feb 29, 2020 · 4 comments · May be fixed by #17
Open

Tests exit with error code 0 #9

piuccio opened this issue Feb 29, 2020 · 4 comments · May be fixed by #17

Comments

@piuccio
Copy link

piuccio commented Feb 29, 2020 •
edited

I just converted from nodeunit to baretest and here's my feedback:

Failing tests still exit with error code 0

const test = require('baretest')('Sum tests'),
  assert = require('assert');

test('1 + 2', function() {
  assert.equal(1 + 2, 12);
});

test.run();

node test will report that the test fails and highlight it in red, but the exit code is 0, so any CI will mark the build as success and carry on as if nothing happens.

Try this node test && echo "SUCCESS!!!".

Annoying to split tests in multiple files

The example about organizing multiple tests is a bit too minimal.
First of all, you'll have to remember to add your new test file inside that test index, so you risk forgetting adding one.
Second, all tests are called My App which doesn't help that much when you're trying to fix a broken test.

We think a good test runner stays out of your way.

I think it should also prevent people from making mistakes. It's a test tool after all.

I think it would be nice to have a baretest-cli or something else to better manage tests split in multiple files.

Here's what I ended up using:

test/
 |- index.js
 |- test1.test.js
 |- test2.test.js
 |- ...test.js

And inside test/index.js

const fs = require("fs");
const baretest = require("baretest");

fs.readdir(__dirname, async (err, files) => {
  if (err) throw err;
  for (let fileName of files.filter((_) => _.endsWith(".test.js"))) {
    const test = baretest(fileName);
    require(`./${fileName}`)(test);
    const success = await test.run();
    if (success === false) {
      process.exit(1);
    }
  }
});

So any file that ends with .test.js is automatically included and gets a different test name. Also checks the return value of test.run().
@tipiirai
Copy link
Contributor

Thanks — great feedback. The error code is clearly an issue to be fixed.

Unsure if the CLI should be part of Baretest. I'm personally organizing the tests as described on the docs and enjoy the flexibility it offers. I can comment/uncomment tests as needed. Or I can put the tests into an array and run the desired slices from it. I can create multiple Baretest instances with a desired name.

And seems you can also do a CLI quite easily on top of the core. The code you did makes a good new project. I'm guessing it needs some work though since people have different needs. There needs to be configuration options, dependency to minimist or similar, pretty output, etc.. Maybe I'm wrong, but feels it would slip away from the minimalistic idea of Baretest when incorporated to the core.

Thanks!

@lusbuab
Copy link

lusbuab commented Apr 23, 2020

Hi @piuccio, I love the minimalistic approach of Baretest. It makes it quite easy to build on top of it.

Regarding your problem with all tests having the same name: maybe this comment is helpful.

@prantlf prantlf linked a pull request Feb 11, 2021 that will close this issue
Open
@prantlf
Copy link

prantlf commented Feb 11, 2021 •
edited

Setting a non-zero exit code is absolutely necessary for being able to use baretest seriously. If you want to use it in a CI/CD pipeline, it has to be able to report the errors to the build orchestrator.

A workaround for this would be setting the exit code in the test code:

process.exit(await test.run() ? 0 : 1)

But cluttering all tests by this should prevented by adding this feature to a single place in baretest.

@prantlf
Copy link

prantlf commented Feb 15, 2021 •
edited

About the test runner - a workaround is to:

1: include a test-executiong statement at the end of each file (test suite):

if (module === require.main) test.run()
else module.exports = test

2: include an index.js in the test directory to execute all test suite:

(async () => {
  await require('./test1.test').run()
  await require('./test2.test').run()
})()

Having the following files:

test/
 |- index.js
 |- test1.test.js
 |- test2.test.js

You can execute all tests by node test and only one test suite by node test/test1.test.js.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Set the process exit code to 1 in case of a failure prantlf/baretest
4 participants
@tipiirai @piuccio @prantlf @lusbuab