Closed Bug 1675835 Opened 4 years ago Closed 4 years ago

Selecting jobs at https://treeherder.mozilla.org/jobs?repo=try&revision=6ba73edce7fc63e15799dc027183753932539329 fails

Categories

(Tree Management :: Treeherder: Frontend, defect)

Product:
Tree Management
Component:
Treeherder: Frontend
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: nalexander, Unassigned)

References

Details

Attachments

(3 files)

Treeherder web console errrors
1.58 MB, image/png
Details
Treeherder load failure.gif
90.06 KB, image/gif
Details
Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/6858
47 bytes, text/x-github-pull-request
Details | Review

For some reason, selecting jobs from the single push at https://treeherder.mozilla.org/jobs?repo=try&revision=6ba73edce7fc63e15799dc027183753932539329 seems to fail. There are JS errors that suggest a bad TH rollout, but I don't know how to determine the TH version or identifier to narrow down the TH rollout.

For example, selecting the "Linux 18.04 x64 opt" job (https://treeherder.mozilla.org/jobs?repo=try&revision=6ba73edce7fc63e15799dc027183753932539329&selectedTaskRun=WslBVTtGRKakALBIywPOgA.0) fails for both me and :jmaher. I'll attach a GIF.

The web console has a few errors, all of which are like:

Uncaught (in promise) TypeError: can't access property "length", e is undefined
    render JobTestGroups.jsx:48
    React 7
    unstable_runWithPriority scheduler.production.min.js:19
    React 6
    fetch JobTestGroups.jsx:31
JobTestGroups.jsx:48:53

I can confirm I see this error- but all other pushes seem fine. Could there be an ingestion error and Treeherder is not dealing with it properly?

Oh -- macOS, repros on Nightly 20201106093443, release 82.0.2, and Chrome 86.

It looks like we are expecting a specific string value for MOZHARNESS_TEST_PATHS in the payload when fetching https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/WslBVTtGRKakALBIywPOgA (for the job mentioned above), but we're receiving an empty object in a string instead, so once it's parsed its overriding the default state value of testGroups: [] with undefined since it's an empty object.

It's happening here: https://github.com/mozilla/treeherder/blob/master/ui/job-view/details/JobTestGroups.jsx#L33

I can push a patch so it doesn't break the UI but I'm curious if something with the config in taskcluster has changed (this code hasn't changed in 4 months). Joel, do you have an idea of who I could ask about this?

sclements: fascinating. These pushes are coming from commands like

./mach try fuzzy toolkit/modules/subprocess/test/xpcshell/xpcshell.ini

which I was old (in #developers on Matrix, IIRC) is the correct way to ensure specific tests get run. Clearly something is going wrong with that invocation of mach try fuzzy.

ahal: is this not the right syntax to ask for specific tests on try?

Flags: needinfo?(ahal)

Looks like a bug in test resolving when you pass in a manifest. Running:

[~/dev/mozilla-central](81a3ef82)$ ./mach try fuzzy --no-push -q "!gpu !wpt !android 'xpcshell" toolkit/modules/subprocess/test/xpcshell/xpcshell.ini
Artifact builds enabled, pass --no-artifact to disable
Task configuration changed, generating target task set
Commit message:
Fuzzy query=!gpu !wpt !android 'xpcshell&paths=toolkit/modules/subprocess/test/xpcshell/xpcshell.ini

Pushed via `mach try fuzzy`
Calculated try_task_config.json:
{
    "env": {
        "MOZHARNESS_TEST_PATHS": "{}",
        "TRY_SELECTOR": "fuzzy"
    },
    "tasks": [
        "test-linux1804-64-asan/opt-xpcshell-e10s-1",
        "test-linux1804-64-qr/debug-xpcshell-e10s-1",
        "test-linux1804-64-qr/opt-xpcshell-e10s-1",
        "test-linux1804-64-tsan/opt-xpcshell-e10s-1",
        "test-linux1804-64/debug-xpcshell-e10s-1",
        "test-linux1804-64/debug-xpcshell-spi-nw-e10s-1",
        "test-linux1804-64/opt-xpcshell-e10s-1",
        "test-macosx1014-64-qr/debug-xpcshell-e10s-1",
        "test-macosx1014-64-qr/opt-xpcshell-e10s-1",
        "test-windows10-64/debug-xpcshell-e10s-1",
        "test-windows10-64/opt-xpcshell-e10s-1",
        "test-windows7-32/debug-xpcshell-e10s-1",
        "test-windows7-32/opt-xpcshell-e10s-1"
    ],
    "use-artifact-builds": true,
    "version": 1
}

[~/dev/mozilla-central](81a3ef82)$ ./mach try fuzzy --no-push -q "!gpu !wpt !android 'xpcshell" toolkit/modules/subprocess/test/xpcshell
Artifact builds enabled, pass --no-artifact to disable
Commit message:
Fuzzy query=!gpu !wpt !android 'xpcshell&paths=toolkit/modules/subprocess/test/xpcshell

Pushed via `mach try fuzzy`
Calculated try_task_config.json:
{
    "env": {
        "MOZHARNESS_TEST_PATHS": "{\"xpcshell\": [\"toolkit/modules/subprocess/test/xpcshell\"]}",
        "TRY_SELECTOR": "fuzzy"
    },
    "tasks": [
        "test-linux1804-64-asan/opt-xpcshell-e10s-1",
        "test-linux1804-64-qr/debug-xpcshell-e10s-1",
        "test-linux1804-64-qr/opt-xpcshell-e10s-1",
        "test-linux1804-64-tsan/opt-xpcshell-e10s-1",
        "test-linux1804-64/debug-xpcshell-e10s-1",
        "test-linux1804-64/debug-xpcshell-spi-nw-e10s-1",
        "test-linux1804-64/opt-xpcshell-e10s-1",
        "test-macosx1014-64-qr/debug-xpcshell-e10s-1",
        "test-macosx1014-64-qr/opt-xpcshell-e10s-1",
        "test-windows10-64/debug-xpcshell-e10s-1",
        "test-windows10-64/opt-xpcshell-e10s-1",
        "test-windows7-32/debug-xpcshell-e10s-1",
        "test-windows7-32/opt-xpcshell-e10s-1"
    ],
    "use-artifact-builds": true,
    "version": 1
}

Using the directory rather than the manifest gets MOZHARNESS_TEST_PATHS populated properly. I'll take a look.

Flags: needinfo?(ahal)
See Also: → 1676204

Looks like it's just been broken for awhile. Kind of surprised, I guess people typically just do the directory rather than the manifest and/or haven't bothered filing. I filed bug 1676204 so this bug can stay focused on the treeherder UI issue (though once the other bug is fixed, this will be lower priority I'm sure).

Thanks for investigating Ahal. I can push a patch to at least check for the correct value type, although I'd argue that we wouldn't need that if we could count on always receiving the same value - a string and not an empty object once parsed. :)

There are various environment variables that store JSONified strings. I can't think of an instance where it would be the case, but "{}" is a valid value as far as these environment variables are concerned. Even if it never should be empty in practice, there may be bugs like this in the future. So I wouldn't advise counting on this not happening again :).

What tool is parsing this? I thought maybe it was a quirk of Javascript, but typeof("{}") == 'string'. Considering "{}" an object feels like the bug.

(In reply to Andrew Halberstadt [:ahal] from comment #10)

There are various environment variables that store JSONified strings. I can't think of an instance where it would be the case, but "{}" is a valid value as far as these environment variables are concerned. Even if it never should be empty in practice, there may be bugs like this in the future. So I wouldn't advise counting on this not happening again :).

What tool is parsing this? I thought maybe it was a quirk of Javascript, but typeof("{}") == 'string'. Considering "{}" an object feels like the bug.

We're using the native JSON.parse and what I've noticed is that parsing only the value of MOZHARNESS_TEST_PATHS converts it to an object (which is what we're doing), but parsing the entire MOZHARNESS_TEST_PATHS object preserves the "{}". So yeah, that should be changed to avoid quirks like this in the future.

Looks like it's been fixed already - thanks!

This has been merged and should be deployed to production today or tomorrow.

Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: