Opened 14 months ago

Closed 14 months ago

Last modified 12 months ago

#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):

https://codereview.adblockplus.org/29864561

Description (last modified by sebastian)

Background

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).

Change History (7)

comment:1 Changed 14 months ago by sebastian

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

comment:2 Changed 14 months ago by sebastian

  • Description modified (diff)

comment:3 Changed 14 months ago by sebastian

  • Description modified (diff)

comment:4 Changed 14 months ago by tlucas

  • Cc kzar hfiguiere added

comment:5 Changed 14 months ago by abpbot

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

comment:6 Changed 14 months ago 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 12 months ago by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Done. Looks to be working as expected.

adblockpluschrome / master / 2175:02d136073262

Note: See TracTickets for help on using tickets.