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)
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).
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: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
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
A commit referencing this issue has landed:
Issue 6884 - Migrate from nodeunit to mocha