Opened on 07/01/2016 at 03:22:25 PM
Closed on 10/06/2016 at 08:53:55 AM
Last modified on 10/06/2016 at 09:11:26 AM
#4223 closed change (fixed)
Move tests from adblockplustests to adblockpluscore/tests
| Reported by: | kzar | Assignee: | trev |
|---|---|---|---|
| Priority: | P4 | Milestone: | |
| Module: | Core | Keywords: | |
| Cc: | trev, fhd | Blocked By: | #4465 |
| Blocking: | #4495 | Platform: | Unknown / Cross platform |
| Ready: | yes | Confidential: | no |
| Tester: | Unknown | Verified working: | no |
| Review URL(s): |
https://codereview.adblockplus.org/29354864/ |
||
Description
Background
We started moving tests into the adblockpluscore repository, using NodeJS and nodeunit to run them from the command line but we never finished.
What to change
Finish moving all the tests from adblockplustest where possible. (Some tests like the element hiding ones require parts of the browser that we no longer have access to and can't easily be ported.)
Attachments (0)
Change History (23)
comment:1 Changed on 07/01/2016 at 03:22:53 PM by kzar
comment:2 Changed on 07/04/2016 at 11:44:12 AM by trev
- Priority changed from Unknown to P4
- Ready set
comment:3 Changed on 09/22/2016 at 06:04:16 PM by kzar
- Blocked By 4465 added
comment:4 Changed on 09/22/2016 at 06:35:38 PM by kzar
- Cc fhd added
(Adding Felix as CC since we were discussing this issue in IRC the other day.)
Found a bit of time to work on this today, made some progress but it's slow work. Next up is the signature tests which require the rsa module changes.
comment:5 Changed on 09/25/2016 at 01:53:27 PM by kzar
- Review URL(s) modified (diff)
- Status changed from new to reviewing
(I realised my changes were getting too large so I've started a review for the test I have migrated so far. Still more to be done after this lands.)
comment:6 Changed on 10/04/2016 at 12:30:30 PM by abpbot
A commit referencing this issue has landed:
Issue 4223 - Migrate some more of adblockplustests
comment:7 Changed on 10/04/2016 at 12:53:10 PM by abpbot
A commit referencing this issue has landed:
Issue 4223 - Use the node command for our tests
comment:9 Changed on 10/04/2016 at 02:54:12 PM by abpbot
A commit referencing this issue has landed:
Issue 4223 - Remove tests migrated to adblockpluscore
comment:10 Changed on 10/04/2016 at 02:59:39 PM by kzar
- Owner kzar deleted
We've made some good progress, quite a few tests have been migrated. Problem is that the tests that are left are not so easy to migrate, they rely on stuff in the Firefox extension API that I'm guessing we can't easily emulate in Node.js.
Unassigning myself since I don't have time to look at those remaining tests for now.
comment:11 Changed on 10/04/2016 at 03:00:13 PM by kzar
- Status changed from reviewing to reopened
comment:12 Changed on 10/04/2016 at 03:05:16 PM by trev
Most of the remaining tests are really integration tests, these are not meaningful outside Firefox. However, it should be possible to migrate filterStorage_readwrite by creating a proper io emulation as well as synchronizer by emulating XMLHttpRequest (this also make the hacks in the current test unnecessary).
comment:13 Changed on 10/04/2016 at 04:47:58 PM by trev
- Owner set to trev
comment:14 Changed on 10/05/2016 at 08:12:36 AM by trev
- Review URL(s) modified (diff)
- Status changed from reopened to reviewing
comment:15 Changed on 10/05/2016 at 12:34:25 PM by trev
- Review URL(s) modified (diff)
comment:16 Changed on 10/05/2016 at 12:36:14 PM by abpbot
A commit referencing this issue has landed:
Issue 4223 - Move synchronizer tests from adblockplustests to adblockpluscore
comment:17 Changed on 10/05/2016 at 02:56:37 PM by abpbot
A commit referencing this issue has landed:
Issue 4223 - Move notification unit tests from adblockplustests to adblockpluscore
comment:18 Changed on 10/05/2016 at 08:10:52 PM by trev
- Review URL(s) modified (diff)
comment:19 Changed on 10/06/2016 at 08:10:34 AM by abpbot
A commit referencing this issue has landed:
Issue 4223 - Move filter storage read/write unit tests from adblockplustests to adblockpluscore
comment:20 Changed on 10/06/2016 at 08:21:51 AM by trev
- Review URL(s) modified (diff)
comment:21 Changed on 10/06/2016 at 08:53:21 AM by abpbot
A commit referencing this issue has landed:
Issue 4223 - Remove more tests migrated to adblockpluscore
comment:22 Changed on 10/06/2016 at 08:53:55 AM by trev
- Resolution set to fixed
- Status changed from reviewing to closed
comment:23 Changed on 10/06/2016 at 09:11:26 AM by kzar
- Blocking 4495 added
(I've been working on this in my down time for a while now, but never got around to filing an issue.)