Opened 2 years ago

Closed 2 years ago

Last modified 7 months ago

#18923 closed enhancement (fixed)

Add a script in tor-browser.git to run all of our TBB-specific regression tests

Reported by: arthuredelstein Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: TorBrowserTeam201607R, tbb-no-uplift
Cc: gk, boklm Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

We are a variety of regression tests in tor-browser.git (mostly mochitests) that would be useful to run all at once. Let's add a script to the repo that does that.

Child Tickets

Change History (14)

comment:1 Changed 2 years ago by gk

Cc: gk added

comment:2 Changed 2 years ago by boklm

Cc: boklm added

comment:3 Changed 2 years ago by boklm

I added a first version of a patch adding a run-tbb-tests script which run the Tor Browser specific tests and save the logs in file tbb-tests.log.

I still have some issues running this script because for some tests the ./mach mochitest command get stuck after running the test and only exit after a long timeout.

comment:4 Changed 2 years ago by boklm

I attached a new version of the patch.

When running the test docshell/test/test_tor_bug16620.html alone, it only exits after a long timeout, for an unknown reason, but without an error. Running this test at the same time as the tests from the tbb-tests directory seems to fix the problem.

The test dom/tests/mochitest/localstorage/test_localStorageByFirstParty.html fails after a timeout, and is now commented in the script. It is ticket #19575.

The following tests are currently failing:

  • dom/base/test/test_tor_bug15703.html: ticket #19583
  • dom/events/test/test_tor_bug15646.html: ticket #19585
  • tbb-tests/browser_tor_TB4.js: ticket #17809

comment:5 Changed 2 years ago by gk

Neat. It seems I need quotes around the asterisks on the echo lines on my system. They are not shown otherwise.

I wonder what we can do to prevent the script from running out of sync with the actual tests available in the tree. Sure, we could make it a requirement to add a new test to the script every time it gets checked in. But that seems error-prone. Would it help to just say "run all tests following the *_tor_* pattern"? (in addition to all those already in the tbb-tests dir)?

comment:6 Changed 2 years ago by boklm

Keywords: TorBrowserTeam201607R added
Status: newneeds_review
Type: defectenhancement

I attached a new version of the patch.

To avoid having to maintain a list of tests, it is now using all the test files added or modified by one of our commits (all the commits since the last ffxbld commit) matching test_[^/]\+\.\(html\|xul\)$ or browser_[^/]\+\.js$.

The tests that we want to ignore can be added to the file tbb-tests-ignore.txt.

During our gitian build, we are removing the .git directory to save space needed for the build, so we don't have the git history available when we want to run the tests for #15994. If the WRITE_TESTS_LIST environment variable is set, the script will save the list of tests in the file tbb-tests-list.txt without running them, which will be used as the list of tests if the USE_TESTS_LIST environment variable is set. We can then run WRITE_TESTS_LIST=1 ./run-tbb-tests before removing the .git directory, and USE_TESTS_LIST=1 ./run-tbb-tests to run the tests after the build.

comment:7 Changed 2 years ago by gk

Just two nits:

1) Could you put quotes around echo The following tests will be run:
2) echo $'\n' 'Starting tests' leads to a one character indentation. We might want to have echo $'\n''Starting tests' instead?

This script takes only care of Mochitest tests so far which is fine to me as we don't have e.f. XPCShell tests right now. We should keep that in mind, though, and adapt the script as soon as this changes.

comment:8 in reply to:  7 Changed 2 years ago by boklm

Replying to gk:

Just two nits:

1) Could you put quotes around echo The following tests will be run:
2) echo $'\n' 'Starting tests' leads to a one character indentation. We might want to have echo $'\n''Starting tests' instead?

Thanks. I attached a new version of the patch with those changes.

This script takes only care of Mochitest tests so far which is fine to me as we don't have e.f. XPCShell tests right now. We should keep that in mind, though, and adapt the script as soon as this changes.

Yes. I added a comment at the top of the script to remind us that it only runs Mochitest tests.

comment:9 Changed 2 years ago by gk

Resolution: fixed
Status: needs_reviewclosed

Thanks. This is commit 1198d77c4ef82db7878e04920523b13ab0e678e6 on tor-browser-45.2.0esr-6.5-1.

comment:10 Changed 7 months ago by arthuredelstein

Keywords: tbb-no-uplift added
Note: See TracTickets for help on using tickets.