Opened 19 months ago

Closed 12 months ago

Last modified 10 months ago

#6391 closed change (fixed)

Run browser tests with headless Firefox as well as Chromium

Reported by: hfiguiere Assignee: hfiguiere
Priority: P2 Milestone:
Module: Core Keywords:
Cc: kzar, sebastian, fhd, sergz, greiner, mjethani Blocked By:
Blocking: #6472 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29720661/
https://codereview.adblockplus.org/29850577/
https://codereview.adblockplus.org/29858555/

Description (last modified by kzar)

Background

adblockpluscore has both unit tests which we run via Node.js and "browser tests" which we run via headless Chromium (#4726, #4796). The browser tests are used to test things like element hiding which are hard / impossible to test otherwise.

Unfortunately since Firefox does not always behave the same as Chromium bugs can still slip through (e.g. #6382). To avoid that happening it would be nice if we could also run the browser tests with headless Firefox.

What to change

  • Modify test_runner.js to also run the browser tests with headless Firefox.
  • While by default the browser tests will run with both Firefox and Chromium add an enviornment variable to configure which are used called BROWSER_TEST_RUNNERS which contains a comma separated list of browser names. Valid browser names are chromium and firefox. For example:
    BROWSER_TEST_RUNNERS=chromium,firefox npm test
    
  • Update the test documentation in the README to explain all this.
  • Update the Travis CI configuration (#6220) to install Firefox as well as Chromium.

Change History (31)

comment:1 Changed 19 months ago by kzar

  • Cc fhd added
  • Description modified (diff)
  • Priority changed from Unknown to P2

This is a good idea, thanks for filing the issue.

Please could you figure out how the command line interface will look for selecting between Firefox and Chromium? Once that's decided I'll mark this Ready.

comment:2 Changed 19 months ago by kzar

  • Summary changed from Run browser test in Firefox. to Run browser test with headles Firefox as well as Chromium

comment:3 follow-up: Changed 19 months ago by hfiguiere

Issue #6382 is one of the motivation for this.

comment:4 Changed 19 months ago by kzar

  • Description modified (diff)

comment:5 in reply to: ↑ 3 Changed 19 months ago by kzar

Replying to hfiguiere:

Issue #6382 is one of the motivation for this.

Thanks, I've added a mention of that to the description.

comment:6 Changed 19 months ago by hfiguiere

To select thw runtime, my suggestion is:

We have a default hardcoded in the script.

But one can pass the runtime selection through the TEST_RUNNERS env. It's a list, possible values are "chromium" and "firefox". Separated by commas. This can be extended.

TEST_RUNNERS=chromium,firefox npm test would run the test on both.

Last edited 19 months ago by hfiguiere (previous) (diff)

comment:7 Changed 18 months ago by kzar

I wouldn't mind if the browser tests always ran with both, you can specify which tests you want to run (e.g. one unit test file) anyway for times that speed matters.

comment:8 Changed 18 months ago by hfiguiere

I'm fine to have both runtime used by default. But it is important to be able to disable either runtime, like we can select the tests to run:

  1. for speed (you iterate quickly on a platform specific bug)
  2. problems: a bug of some kind prevent from running anything else, so you should be able to skip.

comment:9 Changed 18 months ago by kzar

  • Description modified (diff)
  • Ready set

Fair enough I think you're right. I've gone with your suggestion as well, except I've called it "BROWSER_TEST_RUNNERS" since it only applies to the browser tests. The unit tests are always run using Node.js so far.

comment:10 Changed 18 months ago by hfiguiere

  • Summary changed from Run browser test with headles Firefox as well as Chromium to Run browser test with headless Firefox as well as Chromium

comment:11 Changed 18 months ago by kzar

  • Summary changed from Run browser test with headless Firefox as well as Chromium to Run browser tests with headless Firefox as well as Chromium

comment:12 Changed 18 months ago by kzar

  • Description modified (diff)

comment:13 Changed 18 months ago by hfiguiere

  • Owner set to hfiguiere
  • Review URL(s) modified (diff)

comment:14 Changed 18 months ago by hfiguiere

  • Blocking 6472 added

comment:15 Changed 16 months ago by sergz

  • Cc sergz added

comment:16 Changed 16 months ago by greiner

  • Cc greiner added

comment:17 Changed 16 months ago by mjethani

  • Cc SabinaGreiner9 mjethani added; greiner removed

comment:18 Changed 16 months ago by sergz

  • Cc greiner added; SabinaGreiner9 removed

I hope I fixed it. I saw the same behaviour a couple of times too but spotted it before submitting.

comment:19 Changed 16 months ago by kzar

(The eyeo-helpers extension might help, it improves the Trac user suggestions.)

Last edited 16 months ago by kzar (previous) (diff)

comment:20 Changed 13 months ago by hfiguiere

  • Status changed from new to reviewing

comment:21 Changed 13 months ago by abpbot

A commit referencing this issue has landed:
Issue 6391 - Allow running the browser unit tests with Firefox

comment:22 Changed 13 months ago by hfiguiere

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

comment:23 Changed 13 months ago by hfiguiere

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:24 Changed 13 months ago by hfiguiere

Fails on TravisCI.

comment:25 Changed 13 months ago by abpbot

A commit referencing this issue has landed:
Issue 6391 - Temporarily disable Firefox testing by default

comment:26 Changed 13 months ago by hfiguiere

  • Review URL(s) modified (diff)

comment:27 Changed 13 months ago by hfiguiere

FWIW it runs fine on the Ubunutu 14.04 I have here.

There must be something different on TravisCI that make webdriver not work properly. Still investigating.

comment:28 Changed 12 months ago by hfiguiere

  • Review URL(s) modified (diff)

comment:29 Changed 12 months ago by abpbot

A commit referencing this issue has landed:
Issue 6391 - WebDriver: Fix runtime problen with Gecko and Firefox

comment:30 Changed 12 months ago by hfiguiere

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

comment:31 Changed 10 months ago by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Automated tests run successfully in Firefox as well as Chromium.

adblockpluschrome / master / 2175:02d136073262

Note: See TracTickets for help on using tickets.