Investigate moving away from nodeunit for tests

[hfiguiere]

Background

adblockpluscore uses nodeunit for the testing framework. In May 2018, the package has been marked as deprecated:
​https://github.com/caolan/nodeunit/commit/ec2ea882bfb7976c91a25256f855c153594d1b8c

While this doesn't stop it from working, it is a clear indication that it may break in the future.

What to change

• Evaluate a framework replacement
  • mocha was picked
• Update the tests to use that framework

Note

• adblockpluschrome uses qunit with mocha.

[sebastian]

See #6884 for the migration from nodeunit to Mocha in adblockpluschrome, and the reasons for that decision (besides nodeunit being deprecated).

FWIW, I'm not a huge fan of Mocha, but I chose it because (unlike nodeunit) it does the job, and it seems to be the most popular test framework in Node.js environments at the moment.

Where it shines:

• It works great for testing asynchronous code (with minimal boilerplate). Tests (and hooks) can simply return a promise, and if the promise is rejected, it will report a failure.
  • nodeunit on the other hand doesn't know anything about promises and you have to make sure to call the provided callback if your async test is done, after manually asserting that any promise involved is resolved successfully. Even worse, once the first test fails the process terminates immediately, while asynchronous cleanup tasks might still be pending.
• You can group tests together, and have hooks that run either before/after each test in the group, or once before/after all tests in the group.
  • nodeunit on the other hand can only run setup and teardown code for each test, which makes it impossible to share resources across multiple tests.

What I personally dislike:

• The API involves a little too much magic for my taste. You pass a function that defines tests (and hooks) by passing more functions around, and the way to configure stuff (e.g. timeouts) or to share state is through a shared object that is bound to this in those functions.
• If you don't do BDD, the terminology used in the API appears a little weird. You call describe() in order to define a group of tests, you call it() in order to define a test case.
• It doesn't report (multiple) successful assertions. As long as there is no error (failed assertion, other thrown exception or rejected promise), the test is simply marked as passed.

[erikvold]

Here is a list of the common ones that I have seen used (sorted by currently most popular to least):

• ​https://www.npmjs.com/package/jest (by facebook)
• ​https://www.npmjs.com/package/mocha
• ​https://www.npmjs.com/package/tape
• ​https://www.npmjs.com/package/ava
• ​https://www.npmjs.com/package/tap

I like ava fwiw.

[erikvold]

Here is an "awesome" list:

​https://github.com/sindresorhus/awesome-nodejs#testing

[jsonesen]

Mocha is a commonly used framework which I have a small amount of experience with and find it pretty decent. I agree with Sebastian as well, plus since it is widely used support will most likely be pretty good in the longish term. Happy to help out here as well!

[sebastian]

I personally, wouldn't mind if we go ahead with Mocha. But Erik mentioned some other frameworks that seem at least worth evaluating.

FWIW, replacing Mocha in adblockpluschrome would be relatively trivial. However, replacing the test framework in adblockpluscore would be a fairly large change. So we should have good confidence that we want to stick with whatever we are going to migrate here.

Hubert, did you look into any of the alternatives?

Manish, what do you think?

[hfiguiere]

My only motivation is to get away from nodeunit, not because it is bad, but because it is deprecated and unmaintained.

Mocha does the jobs and we use it in the WebExtension, so I deemed that it was the right choice.

And I discovered there is Mochify ​https://github.com/mantoni/mochify.js which is basically Mocha+Browserify to run the code in the browser. Maybe this is worth investigating for the browser tests.

[mjethani]

It seems that ​AVA is almost as popular at Mocha, at least from the number of stars on the GitHub project. It seems to fix some of Nodeunit's problems (e.g. relevant diffs between actual and expected values). Now somebody needs to do a proper evaluation.

In my opinion the criteria for picking a framework should be:

1. Whether the framework is being actively developed/maintained (i.e. why we are moving away from Nodeunit)
2. Ease of migration
3. Additional features

If this migration becomes an opportunity to refactor some of the test code (as it is often painful to update it), even better.

[hfiguiere]

See also for step 1 ​https://gitlab.com/eyeo/adblockplus/adblockpluscore/issues/14

[hfiguiere]

After discussions with Erik, it was determined that AVA isn't practical in our case. So Mocha it is.

Making the changes progressively will help with the review. So will do that.

Step 1 is use assert as an assertion library instead of nodeunit. This change is compatible. ​​https://gitlab.com/eyeo/adblockplus/adblockpluscore/issues/14

[hfiguiere]

Moving to mocha:
​https://gitlab.com/eyeo/adblockplus/adblockpluscore/issues/26

Closing this issue now. Since we have a solution, marking as FIXED.