Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#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/
https://codereview.adblockplus.org/29355728/
https://codereview.adblockplus.org/29355735/
https://codereview.adblockplus.org/29355872/
https://codereview.adblockplus.org/29356001/
https://codereview.adblockplus.org/29356024/
https://codereview.adblockplus.org/29356056/

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

Change History (23)

comment:1 Changed 4 years ago by kzar

(I've been working on this in my down time for a while now, but never got around to filing an issue.)

comment:2 Changed 4 years ago by trev

  • Priority changed from Unknown to P4
  • Ready set

comment:3 Changed 4 years ago by kzar

  • Blocked By 4465 added

comment:4 Changed 4 years ago 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 4 years ago 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 4 years ago by abpbot

A commit referencing this issue has landed:
Issue 4223 - Migrate some more of adblockplustests

comment:7 Changed 4 years ago by abpbot

A commit referencing this issue has landed:
Issue 4223 - Use the node command for our tests

comment:8 Changed 4 years ago by kzar

  • Review URL(s) modified (diff)

comment:9 Changed 4 years ago by abpbot

A commit referencing this issue has landed:
Issue 4223 - Remove tests migrated to adblockpluscore

comment:10 Changed 4 years ago 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.

Last edited 4 years ago by kzar (previous) (diff)

comment:11 Changed 4 years ago by kzar

  • Status changed from reviewing to reopened

comment:12 Changed 4 years ago 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 4 years ago by trev

  • Owner set to trev

comment:14 Changed 4 years ago by trev

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

comment:15 Changed 4 years ago by trev

  • Review URL(s) modified (diff)

comment:18 Changed 4 years ago by trev

  • Review URL(s) modified (diff)

comment:20 Changed 4 years ago by trev

  • Review URL(s) modified (diff)

comment:21 Changed 4 years ago by abpbot

A commit referencing this issue has landed:
Issue 4223 - Remove more tests migrated to adblockpluscore

comment:22 Changed 4 years ago by trev

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

comment:23 Changed 4 years ago by kzar

  • Blocking 4495 added
Note: See TracTickets for help on using tickets.