Opened on 08/25/2018 at 11:16:23 PM

Closed on 08/27/2018 at 09:57:48 AM

Last modified on 10/18/2018 at 12:18:00 PM

#6884 closed change (fixed)

Migrate from nodeunit to mocha (in adblockpluschrome)

Reported by: sebastian Assignee: sebastian
Priority: P3 Milestone: Adblock-Plus-3.4-for-Chrome-Opera-Firefox
Module: Platform Keywords:
Cc: tlucas, kzar, hfiguiere Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

Description (last modified by sebastian)


With #6717 we introduced nodeunit in adblockpluschrome in order to wrap WebDriver to run the actual (qunit) tests from the command line and the CI. We decided to use nodeunit becasue this is historically what we use in adblockpluscore (we might want to migrate to Mocha there as well).

There are following issues with nodeunit:

  • nodeunit is deprecated and no longer maintained.
  • If a test failed, the process appears to be terminated before completing pending asynchronous tasks, which calls for obscure workarounds (if available) when dealing with resources that need to be cleaned up (e.g. browser processes managed through WebDriver).
  • There is no concept of shared resources, setUp and tearDown are always called for each test, making it difficult to share resources (that require cleanup) across tests.

The latter two make it in particular impossible to extend the test suite (e.g. by running against our test pages), without resulting into an inferior test architecture and slower performance.

Mocha doesn't seem to have any of these issues and appears to have become the defacto standard testing framework for Node.js in modern days.

What to change

Replace nodeunit with the Mocha test framework in adblockpluchrome. While on it, leverage the before and after hooks (which wasn't possible before and will help to add more tests).

Attachments (0)

Change History (7)

comment:1 Changed on 08/25/2018 at 11:16:47 PM by sebastian

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:2 Changed on 08/25/2018 at 11:20:04 PM by sebastian

  • Description modified (diff)

comment:3 Changed on 08/25/2018 at 11:22:32 PM by sebastian

  • Description modified (diff)

comment:4 Changed on 08/26/2018 at 09:58:37 AM by tlucas

  • Cc kzar hfiguiere added

comment:5 Changed on 08/27/2018 at 09:55:00 AM by abpbot

A commit referencing this issue has landed:
Issue 6884 - Migrate from nodeunit to mocha

comment:6 Changed on 08/27/2018 at 09:57:48 AM by sebastian

  • Milestone set to Adblock-Plus-for-Chrome-Opera-Firefox-next
  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:7 Changed on 10/18/2018 at 12:18:00 PM by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Done. Looks to be working as expected.

adblockpluschrome / master / 2175:02d136073262

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 sebastian.
Note: See TracTickets for help on using tickets.