1523303 - Suite name mismatch with mozharness breaks MOZHARNESS_TEST_PATHS in some cases

Status: RESOLVED FIXED

(Developer Infrastructure :: Try, defect, P2)

Description

[Andrew Halberstadt [:ahal]]

Bianca found a bug over in bug 1517083. Here is her push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=31c8e51e58a655d06884b2ad9e5542e3c5b66c48&selectedJob=224097353

If you look at the top commit you'll see MOZHARNESS_TEST_PATHS in her try_task_config.json:

{
    "tasks": [
        "test-linux64/opt-mochitest-browser-chrome-e10s-1"
    ],
    "templates": {
        "artifact": {
            "enabled": "1"
        },
        "env": {
            "MOZHARNESS_TEST_PATHS": "{\"browser-chrome\": \"testing/extensions/browser_chrome/browser_experiment_installed_check.js\"]}",
            "TRY_SELECTOR": "fuzzy"
        }
    },
    "version": 1
}

However, in the log we see the entire suite running:
https://taskcluster-artifacts.net/CWHidhXUSmuWcbMcfAVtKA/0/public/logs/live_backing.log

I believe the reason is a suite mismatch here:
https://searchfox.org/mozilla-central/source/testing/mozharness/scripts/desktop_unittest.py#365

In the try_task_config.json, we call the suite "browser-chrome", but in the mozharness commandline we pass in --mochitest-suite=browser-chrome-chunked. I think the latter is not an actual suite name, but an invention of mozharness. Stripping the "-chunked" if it exists should fix this.

Comment 1

[Ian Moody [:Kwan] (UTC+0)]

Attachment: 043662fcb40bd0c480149d74850b43c2fca69c6_window7debug_run2.log

I believe I may have encountered the same thing.
With the following try_task_config.json

{
    "tasks": [
        "test-linux32/debug-mochitest-browser-chrome-e10s-1",
        "test-linux32/opt-mochitest-browser-chrome-e10s-1",
        "test-linux64-asan/opt-mochitest-browser-chrome-e10s-1",
        "test-linux64-pgo/opt-mochitest-browser-chrome-e10s-1",
        "test-linux64/debug-mochitest-browser-chrome-e10s-1",
        "test-macosx64/debug-mochitest-browser-chrome-e10s-1",
        "test-macosx64/opt-mochitest-browser-chrome-e10s-1",
        "test-windows10-64-asan/opt-mochitest-browser-chrome-e10s-1",
        "test-windows10-64-pgo/opt-mochitest-browser-chrome-e10s-1",
        "test-windows10-64/debug-mochitest-browser-chrome-e10s-1",
        "test-windows7-32-pgo/opt-mochitest-browser-chrome-e10s-1",
        "test-windows7-32/debug-mochitest-browser-chrome-e10s-1",
        "test-windows7-32/opt-mochitest-browser-chrome-e10s-1"
    ],
    "templates": {
        "artifact": {
            "enabled": "1"
        },
        "env": {
            "MOZHARNESS_TEST_PATHS": "{\"browser-chrome\": [\"browser/components/shell/test/browser_setDesktopBackgroundPreview.js\"]}",
            "TRY_SELECTOR": "fuzzy"
        },
        "rebuild": 2
    },
    "version": 1
}

Some jobs run the requested browser_setDesktopBackgroundPreview.js, others run other tests entirely.
try push

One of the few jobs it appears to have actually run the right test is the 2nd run on Windows 7 debug (log, backup attached) but even then it included other tests.

Comment 2

[Andrew Halberstadt [:ahal]]

I think this was a regression from bug 1489100, and there's a lot more than browser-chrome affected. I wrote up a test that shows which suites are currently failing. I'll mark the failures expected then fix them up one by one.

Comment 4

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1523303 - [tryselect] Add test making sure |mach try fuzzy <path>| generates suite names that mozharness recognizes, r?gbrown

It turns out bug 1489100 regressed the ability to specify test paths for most
suites by naively assuming that mozharness uses suite names that look like:

<flavor>-<subsuite>

In reality, there is no consistency to how suite names are generated. This test
does a few things:

1. Patches the moztest.TestResolver to return a list of all possible
   suites/subsuites (assuming the lists in moztest.resolve are up to date).

2. Finds all the suites defined in the mozharness configs (e.g
   linux_unittest.py), and uses these are parametrized inputs.

3. Checks that for each test suite,
   DesktopUnittest._get_mozharness_test_paths() returns something.

I've marked all of the test suites that currently fail as expected. This way I
have a good sense of what needs to be fixed, and can validate my changes as I
move through the list.

Comment 5

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1523303 - [mozharness] Normalize '-chunked' out of suite name when reading MOZHARNESS_TEST_PATHS in desktop_unittest.py, r?gbrown

Mozharness appends -chunked/-coverage to some suites, but the build system/test
resolver don't have any concept of these things. We need to normalize these out
for the purposes of MOZHARNESS_TEST_PATHS.

Depends on D25014

Comment 6

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1523303 - [moztest] Split mochitest-gpu/mochitest-clipboard suite definitions into distinct flavors, r?gbrown

Depends on D25015

Comment 7

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1523303 - [moztest] Exclude mochitest-webgl tasks from the mochitest-plain regexes, r?gbrown

We should use filter functions instead of regexes here. Would be a lot more robust.

Depends on D25076

Comment 8

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1523303 - [tryselect] Define 'mozharness_name' key in relevant TEST_SUITE definitions, r?gbrown

Some of these were working with the '<flavor>-<subsuite>' mechanism that was
previously being used, but better to be explicit wherever possible.

Depends on D25077

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ac9662d71fec
[tryselect] Add test making sure |mach try fuzzy <path>| generates suite names that mozharness recognizes, r=gbrown
https://hg.mozilla.org/integration/autoland/rev/06b200ff82e6
[mozharness] Normalize '-chunked' out of suite name when reading MOZHARNESS_TEST_PATHS in desktop_unittest.py, r=gbrown

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0f131b481e5e
[moztest] Split mochitest-gpu/mochitest-clipboard suite definitions into distinct flavors, r=gbrown
https://hg.mozilla.org/integration/autoland/rev/4a9748aea8e9
[moztest] Exclude mochitest-webgl tasks from the mochitest-plain regexes, r=gbrown
https://hg.mozilla.org/integration/autoland/rev/b332da5a285d
[tryselect] Define 'mozharness_name' key in relevant TEST_SUITE definitions, r=gbrown

Comment 11

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/ac9662d71fec
https://hg.mozilla.org/mozilla-central/rev/06b200ff82e6
https://hg.mozilla.org/mozilla-central/rev/0f131b481e5e
https://hg.mozilla.org/mozilla-central/rev/4a9748aea8e9
https://hg.mozilla.org/mozilla-central/rev/b332da5a285d

Comment 12

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1523303 - [tryselect] Also read mac/win_unittest.py configs in test_mozharness_integration.py, r?gbrown

There are a few windows/mac-only suites that were missed since we were only
reading the linux variant.

Depends on D25401

Comment 13

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1523303 - [mozharness] Remove definitions for defunct *-addons suites, r?gbrown

Depends on D25432

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c367d8ca7f3a
[tryselect] Also read mac/win_unittest.py configs in test_mozharness_integration.py, r=gbrown
https://hg.mozilla.org/integration/autoland/rev/d3ba5839e2c2
[mozharness] Remove definitions for defunct *-addons suites, r=gbrown

Comment 15

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/c367d8ca7f3a
https://hg.mozilla.org/mozilla-central/rev/d3ba5839e2c2

Comment 16

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1523303 - Align mozharness suite names with the ones in 'moztest.resolve', r?gbrown

This officially makes 'moztest.resolve' the source of truth when it comes to
suite names. It aligns that file with the names used in both the
desktop_unittest and android_emulator_unittest scripts.

Comment 17

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1523303 - [tryselect] Extend test_mozharness_integration.py to include the 'android_emulator_unittest' script, r?gbrown

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d689bb3db16e
[tryselect] Extend test_mozharness_integration.py to include the 'android_emulator_unittest' script, r=gbrown

Comment 19

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/d689bb3db16e

Comment 20

[:Gijs (he/him)]

Is there still something left to do here?

Comment 21

[Andrew Halberstadt [:ahal]]

Yes, have a few patches left to land to get Android working and need to test the marionette/wpt scripts as well.

Comment 22

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1523303 - [taskgraph] Define suite "categories" rather than flavours task configs, r?gbrown

Currently we have the concept of a "suite" and a "flavour" in our task
configuration. Typically, the "suite" refers to the high-level test harness
like "mochitest" or "reftest", whereas the flavour is more specific, e.g
"browser-chrome-instrumentation" or "crashtest". However the line between suite
and flavour is not applied with any semblance of consistency which results in
inconsistent naming throughout the tree.

This patch gets rid of the concept of "flavours" entirely (at least when it
comes to task configuration). A suite is a type of test run, for example:

- mochitest-plain
- mochitest-devtools-chrome
- mochitest-browser-chrome-instrumentation
- jsreftest
- reftest
- firefox-ui-functional-remote
etc

There is no confusion here between suites and flavours because flavours don't
exist. However, there are a couple of places where we do need to know what
"test harness" is used to run a suite. These cases are:

1. For SCHEDULES moz.build rules
2. For the desktop_unittest.py mozharness script which takes arguments like
   --mochitest-suite=browser (this is not a compelling use of this information
   and should be refactored to work more like the android_emulator_unittest.py
   script)

So to get this information, this patch introduces a new concept of a "category"
which is the overall "test harness" that runs the suite. For many suites, the
"category" is identical to the suite name. Unlike flavours, "categories" have
no bearing on how we call or refer to the suite.

Comment 23

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1523303 - Align mozharness suite names with the ones in 'moztest.resolve', r?gbrown

This officially makes 'moztest.resolve' the source of truth when it comes to
suite names. It aligns that file with the names used in both the
desktop_unittest and android_emulator_unittest scripts.

Depends on D27554

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/00c2b2449639
[taskgraph] Define suite "categories" rather than flavours task configs, r=gbrown
https://hg.mozilla.org/integration/autoland/rev/60c512bab3e9
Align mozharness suite names with the ones in 'moztest.resolve', r=gbrown

Comment 25

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/00c2b2449639
https://hg.mozilla.org/mozilla-central/rev/60c512bab3e9

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/409270f0615f
Port bug 1523303 - Update Thunderbird mochitest configuration to match mozilla-central changes; rs=bustage-fix

Comment 27

[Andrew Halberstadt [:ahal]]

I believe this was left open to test some of the non desktop_unittest.py mozharness scripts. But I haven't noticed any more complaints about test paths not working since this landed, so I'll resolve this for now and open a new bug if any further issues are discovered.