Opened on 07/31/2018 at 01:10:11 PM

Closed on 05/23/2019 at 02:04:32 PM

#6820 closed change (fixed)

Investigate moving away from nodeunit for tests

Reported by: hfiguiere Assignee: hfiguiere
Priority: Unknown Milestone:
Module: Core Keywords:
Cc: greiner, erikvold, sebastian, jsonesen, mjethani Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

Description (last modified by hfiguiere)


adblockpluscore uses nodeunit for the testing framework. In May 2018, the package has been marked as deprecated:

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


  • adblockpluschrome uses qunit with mocha.

Attachments (0)

Change History (16)

comment:1 Changed on 08/06/2018 at 05:31:05 PM by greiner

  • Cc greiner added

comment:2 Changed on 09/20/2018 at 06:08:34 AM by erikvold

  • Cc erikvold added

comment:3 Changed on 09/21/2018 at 04:14:51 PM by hfiguiere

  • Description modified (diff)

comment:4 Changed on 09/21/2018 at 05:52:43 PM by sebastian

  • Cc sebastian added

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.
Last edited on 09/21/2018 at 05:57:17 PM by sebastian

comment:5 Changed on 11/15/2018 at 05:08:19 AM by erikvold

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

I like ava fwiw.

comment:6 Changed on 11/15/2018 at 03:05:35 PM by erikvold

comment:7 Changed on 03/07/2019 at 01:35:33 AM by jsonesen

  • Cc jsonesen added

comment:8 Changed on 03/07/2019 at 01:38:20 AM by 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!

Last edited on 03/07/2019 at 01:39:31 AM by jsonesen

comment:9 Changed on 03/07/2019 at 01:16:36 PM by hfiguiere

  • Owner set to hfiguiere
  • Review URL(s) modified (diff)

comment:10 Changed on 03/07/2019 at 01:17:14 PM by hfiguiere

  • Description modified (diff)

comment:11 Changed on 03/08/2019 at 01:25:03 AM by sebastian

  • Cc mjethani added

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?

comment:12 Changed on 03/08/2019 at 03:41:06 PM by 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 which is basically Mocha+Browserify to run the code in the browser. Maybe this is worth investigating for the browser tests.

comment:13 Changed on 03/16/2019 at 12:27:04 PM by 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.

comment:14 Changed on 05/09/2019 at 07:58:45 PM by hfiguiere

comment:15 Changed on 05/10/2019 at 04:35:58 PM by 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. ​

comment:16 Changed on 05/23/2019 at 02:04:32 PM by hfiguiere

  • Resolution set to fixed
  • Status changed from new to closed

Moving to mocha:

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

Add Comment

Modify Ticket

Change Properties
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from hfiguiere.
Note: See TracTickets for help on using tickets.