Closed Bug 1334093 Opened 3 years ago Closed 3 years ago

Test jobs should always extract the mach script and mozinfo.json (for manifestparser) from common.tests.zip

Categories

(Release Engineering :: Applications: MozharnessCore, defect)

defect
Not set

Tracking

(firefox52 fixed, firefox-esr52 fixed, firefox53 fixed, firefox54 fixed)

RESOLVED FIXED
Tracking Status
firefox52 --- fixed
firefox-esr52 --- fixed
firefox53 --- fixed
firefox54 --- fixed

People

(Reporter: whimboo, Assigned: whimboo)

References

Details

Attachments

(1 file)

As seen on bug 1330253 at least the firefox-ui-functional jobs are not extracting the mozinfo.json file from the common.tests.zip package. As result the test harness might not be able to filter tests for skipping based on values inside the mozinfo.json file, like:

> skip-if = debug

A quick look showed me that at least all Marionette based test jobs are affected, but not the marionette script itself.
Given that this is somewhat urgent for being able to skip the correct tests and especially for bug 1330253, I'm going to take it.
Assignee: nobody → hskupin
Status: NEW → ASSIGNED
So the two files we are talking about here are actually the mach script and the mozinfo.json file. Both are in the root directory of the common.tests archive and should be unpacked everytime IMHO. So special-casing it for specific test suites doesn't make sense. It means I think it's better to put some code into download_and_extract() for testbase.py, which adds those by default, even if not specified by extract_dirs.
Currently it's not that easy to always white-list top-level files for extraction of common.tests.zip. The method uses fnmatch.filter and this only supports '*'. In our own code-base we also have '**' which means recursively.

https://dxr.mozilla.org/mozilla-central/rev/1d025ac534a6333a8170a59a95a8a3673d4028ee/testing/mozharness/mozharness/base/script.py#532-538

So we would have to get a copy of fnmatch.filter() and make the modifications. Aki, do you have some feedback for me, or another idea? Thanks.
Flags: needinfo?(aki)
It appears that adding 'mozinfo.json' to the extract_dirs Just Works.

>>> import fnmatch
>>> import functools
>>> import itertools
>>> import zipfile
>>> bundle = zipfile.ZipFile("/Users/asasaki/Downloads/target.common.tests.zip")
>>> 'mozinfo.json' in bundle.namelist()
True
>>> filter_partial = functools.partial(fnmatch.filter, bundle.namelist())
>>> entries = itertools.chain(*map(filter_partial, ['mozinfo.json', 'reftest/*']))
>>> for entry in entries:
...     print entry
...
mozinfo.json
reftest/automation.py
reftest/reftest.ini

So after https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/mozharness/mozilla/testing/testbase.py#472 we could possibly

    if isintance(unpack_dirs, list) and 'mozinfo.json' not in unpack_dirs:
        unpack_dirs.append('mozinfo.json')

It would be good to push that to try to verify it doesn't break anything :)
Flags: needinfo?(aki)
Adding mozinfo.json to the list of files to unpack from common.tests.zip there is a permanent failure for a single browser chrome test with the name browser_verify_content_about_newtab.js. 

https://dxr.mozilla.org/mozilla-central/rev/1cc159c7a0445ec51e335c8a1a1cceea7bbf8380/dom/security/test/contentverifier/browser_verify_content_about_newtab.js

Interestingly the same test gets run in bc2, bc4, bc5, and bc6 chunks, and shows failures like:

TEST-UNEXPECTED-FAIL | dom/security/test/contentverifier/browser_verify_content_about_newtab.js | Valid remote newtab page must have built-in CSP. - Got {}, expected {"csp-policies":[{"report-only":false,"script-src":["https://example.com","'unsafe-inline'"],"style-src":["https://example.com"]}]}

The content of mozinfo.json looks like the following for Windows:

{
    "addon_signing": true, 
    "appname": "firefox", 
    "artifact": false, 
    "asan": false, 
    "bin_suffix": ".exe", 
    "bits": 32, 
    "buildapp": "browser", 
    "buildtype_guess": "debug", 
    "crashreporter": true, 
    "datareporting": true, 
    "debug": true, 
    "healthreport": true, 
    "mozconfig": "c:/builds/moz2_slave/try-w32-d-00000000000000000000/build/src/.mozconfig", 
    "nightly_build": true, 
    "official": true, 
    "os": "win", 
    "pgo": false, 
    "platform_guess": "win32", 
    "processor": "x86", 
    "release_or_beta": false, 
    "require_signing": false, 
    "stylo": false, 
    "sync": true, 
    "telemetry": false, 
    "tests_enabled": true, 
    "toolkit": "windows", 
    "topsrcdir": "c:/builds/moz2_slave/try-w32-d-00000000000000000000/build/src", 
    "tsan": false, 
    "updater": true
}

Franziskus or Mike, do you have an idea what this could be?
Flags: needinfo?(franziskuskiefer)
Flags: needinfo?(mconley)
Oh, wait. The browser.ini manifest file shows that this test started perma failing lately and got skipped 3 days ago via bug 1336654. I was clearly working on an old changeset when pushing it to try.

So the changes introduced with my patch should not have caused any regression.
Flags: needinfo?(mconley)
Flags: needinfo?(franziskuskiefer)
Summary: test jobs should always extract mozinfo.json from common.tests.zip → Test jobs should always extract the mach script and mozinfo.json (for manifestparser) from common.tests.zip
For safety I rebased my patch against current central, and pushed a new try build for browser-chrome tests only:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5cb54e07ccdff2a3f3b8716c97aeaf9a2c21a294
Comment on attachment 8834016 [details]
Bug 1334093 - Test jobs should always extract the mach script and mozinfo.json from common.tests.zip.

https://reviewboard.mozilla.org/r/110128/#review111340

I don't love the hardcode, but we're already hardcoding jsshell, and we're only affecting tests. r=me
Attachment #8834016 - Flags: review?(aki) → review+
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c0a4ffb1eb40
Test jobs should always extract the mach script and mozinfo.json from common.tests.zip. r=aki
https://hg.mozilla.org/mozilla-central/rev/c0a4ffb1eb40
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Test-only change which we would like to see uplifted to aurora and beta. Thanks.
Whiteboard: [checkin-needed-aurora][checkin-needed-beta]
https://hg.mozilla.org/releases/mozilla-aurora/rev/b064a2c3e631
Whiteboard: [checkin-needed-aurora][checkin-needed-beta] → [checkin-needed-beta]
You need to log in before you can comment on or make changes to this bug.