Closed Bug 1603844 Opened 5 years ago Closed 5 years ago

Remove `install-to-subdir` hack

Categories

(Testing :: General, task, P2)

Version 3
task

Tracking

(firefox73 fixed)

RESOLVED FIXED
mozilla73
Tracking Status
firefox73 --- fixed

People

(Reporter: ahal, Assigned: ahal)

References

Details

Attachments

(5 files)

There exists a install-to-subdir key you can specify in mochitests that will get tests from said manifests installed to a new location. There is only a single place in-tree that uses it:
https://searchfox.org/mozilla-central/rev/2f09184ec781a2667feec87499d4b81b32b6c48e/toolkit/components/extensions/test/mochitest/mochitest-remote.ini#8

Quoting the comment there:

# This is a horrible hack:
# In order to run tests under two configurations, we create two mochitest
# manifests, and include a manifest with a common set of tests from each. In
# order to detect which manifest we're running from, we install the tests listed
# in this manifest to the sub-directory "test-oop-extensions", and then check
# whether we're running from that directory from head.js

I discovered that this isn't playing nicely with my changes in bug 1583353, as after my changes the test-oop-extensions/manifest-common.ini tests were no longer being run. It looks like there is a lot of complexity this key creates, so ideally we can figure out an alternative way of solving the problem and removing install-to-subdir completely.

It looks like head.js detects test-oop-extensions and then uses that to set a pref. Since we can now set prefs directly in manifests, I think an appropriate way of solving this is to set the pref in the manifest, then use SpecialPowers.getPref to determine whether or not we are running under remote.

Assignee: nobody → ahal
Status: NEW → ASSIGNED
Depends on: 1604360

I realized that we don't combine prefs from parent manifests, and that even when I enabled that there was a bug in manifestparser. I'll tackle these problems in bug 1604360.

While the 'prefs' key can only exist in the DEFAULT section of manifests, this
allows parent manifests to propagate their prefs down to included ones.

Attachment #9115870 - Attachment description: Bug 1603844 - Stop using 'install-to-subdir' in toolkit/components/extensions/test/mochitest/mochitest-remote.ini → Bug 1603844 - Stop using 'install-to-subdir' in toolkit/components/extensions/test/mochitest/mochitest-remote.ini, r?mixedpuppy
Attachment #9115871 - Attachment description: Bug 1603844 - Remove ability to 'install-to-subdir' from test objects → Bug 1603844 - Remove ability to 'install-to-subdir' from test objects, r?#firefox-build-system-reviewers

In mochitest, we use a test's manifest to:

  1. Log the test groups at suite_start
  2. Ensure all tests in a manifest have the same prefs/env
  3. Implement run-by-manifest mode

However, we don't factor in a test's ancestor manifest. This can cause problems
if the same manifest is imported more than once by different parent manifests.
This patch ensures that if a test has an ancestor manifest, we use the key
'<ancestor_manifest>:<manifest>' for the purposes listed above.

Pushed by ahalberstadt@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ac51a01611dc [manifestparser] Merge 'prefs' from defaults instead of overwriting them, r=gbrown https://hg.mozilla.org/integration/autoland/rev/ce035f29f7ff [mochitest] Use 'ancestor_manifest' as part of the key for determining a test's manifest, r=gbrown https://hg.mozilla.org/integration/autoland/rev/a7ee8fc74f3a Stop using 'install-to-subdir' in toolkit/components/extensions/test/mochitest/mochitest-remote.ini, r=mixedpuppy https://hg.mozilla.org/integration/autoland/rev/5c2a2641fd12 Remove ability to 'install-to-subdir' from test objects, r=firefox-build-system-reviewers,rstewart

Ugh, I had made a last minute tweak to the patch and it ended up turning the test I added orange. Sorry. I'll get a patch up to fix it soon.

(This is the one in the mochitest harness commit, nothing related to the extensions tests)

Pushed by gbrown@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/faa19d649f36 [mochitest] Fix a failure in the 'test_get_active_tests.py' selftest, r=gbrown
Regressions: 1605515
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: