1370359 - [mozlog] Add manifest to test objects in errorsummary.log

Status: RESOLVED FIXED

(Testing :: Mozbase, enhancement)

Description

[Cameron Dawson [:camd]]

Treeherder will at some point be ingesting and storing the entire list of Manifests and any tests that failed for it.  This data will be used for the Test-Group based UI currently under development.

It would greatly help with data ingestion to have a file with this data in the following format:

{
    "suite": "mochitest",
    "manifests": [
        "manifest-foo.ini": [],  # all tests within manifest passed
        "manifest-bar.ini": [
            "test-abc-1.html": [
                {"status": "fail", ...},
                {"stackwalk_stdout": "...", ...}
            ],
        ],
        ...
    ],
}

or perhaps it would be more like:

{
    "suite": "mochitest",
    "manifests": [
        "manifest-foo.ini": [],  # all tests within manifest passed
        "manifest-bar.ini": [
            {"test": "test-abc-1.html", "status": "fail", ...},
            {"test": "test-abc-2.html", "stackwalk_stdout": "...", ...}
        ],
        ...
    ],
}

This is a structure Ed came up with when I talked to him today (with a few tweaks).

Comment 1

[Cameron Dawson [:camd]]

Hey Andrew, here's the bug we talked about today.  Thanks heaps!  :)

Comment 2

[James Graham [:jgraham]]

Requiring a single json structure is bad for streaming. Just a list of tests from suite_start would be OK; a list of failing results is problematic because if the process dies we end up with either entirely missing data or corrupted JSON.

Comment 3

[Andrew Halberstadt [:ahal]]

Hm, true.. Also if we ever wanted to fail fast (i.e turn a job orange as soon as a failure came in over pulse or something), this structure would prevent that.

Cam, is this solution more about reducing code complexity or reducing computation? If it's the former, maybe it would be better to just live with it.. if the latter, maybe we need to rethink using artifacts in the first place :/

Comment 4

[Ed Morley [:emorley]]

(In reply to Andrew Halberstadt [:ahal] from comment #3)
> Cam, is this solution more about reducing code complexity or reducing computation?

It was a bit of both, but I think we're happy to forgo given the streaming and crashing mid run considerations.

What we do really need however, if possible, is to have the correlation between manifest and failing test to be made in that file, rather than having to cross-reference.

So:

{"manifests": [m1, m2, m3])
{"testname": "foo", "manifest" m1, status: fail}

Differences compared to current:
* The `manifests` key lists all manifests (regardless of pass/fail), but no longer lists every test name (since this bloats the file, and Treeherder won't be ingesting the name of every passing test due to data volume).
* The row for each failure not only lists the test name, but also the manifest from which that test was run (to save Treeherder having to cross-reference).

Does that sounds viable?

Comment 5

[Cameron Dawson [:camd]]

Heh, what Ed said, yeah.  :)  It's not even that much extra work to have them separate.  But the scheme Ed mentioned will be totally fine to work with.  Thanks for chiming in, guys.  :)

Comment 6

[Andrew Halberstadt [:ahal]]

Great, makes sense! Should be an easy change, I'll have it up shortly.

Comment 7

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1370359 - Stop dumping all tests to errorsummary.log; include manifest in error lines,

Review commit: https://reviewboard.mozilla.org/r/146752/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/146752/

Comment 8

[Andrew Halberstadt [:ahal]]

Still waiting for a failing task in my try push, but here's what a passing errorsummary looks like:
https://public-artifacts.taskcluster.net/feT-gZ7UTdu3VSLDiiu4Sg/0/public/test_info//plain-chunked_errorsummary.log

Failure lines should have a "group" key in them now that contains the manifest.

Comment 9

[Andrew Halberstadt [:ahal]]

Looks like chrome and a11y test paths have chrome:// in front, which makes them hard to match against the list of tests dumped at the beginning (which are relative paths to topsrcdir). I can't simply munge them in errorsummary.py, as that would break other harnesses where urls are actually desired.

I guess I'll try to get the mochitest harness to log just the test path. Browser and plain mochitests already do this.

Comment 10

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1370359 - Add ally to list of test path prefixes that mochitest harness normalizes,

Review commit: https://reviewboard.mozilla.org/r/146862/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/146862/

Comment 11

[Andrew Halberstadt [:ahal]]

Comment on attachment 8875339 [details]
Bug 1370359 - Stop dumping all tests to errorsummary.log; include manifest in error lines,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/146752/diff/1-2/

Comment 12

[Andrew Halberstadt [:ahal]]

Here's an example of the new format (with failures):
https://public-artifacts.taskcluster.net/amNaNFDLQa-7PJ3qvxJZIA/0/public/test_info//browser-chrome-chunked_errorsummary.log

Comment 13

[James Graham [:jgraham]]

Comment on attachment 8875339 [details]
Bug 1370359 - Stop dumping all tests to errorsummary.log; include manifest in error lines,

https://reviewboard.mozilla.org/r/146752/#review151226

Comment 14

[James Graham [:jgraham]]

Comment on attachment 8875417 [details]
Bug 1370359 - Add ally to list of test path prefixes that mochitest harness normalizes,

https://reviewboard.mozilla.org/r/146862/#review151228

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/189df493159b
Add ally to list of test path prefixes that mochitest harness normalizes, r=jgraham
https://hg.mozilla.org/integration/autoland/rev/09b1aee5ad9f
Stop dumping all tests to errorsummary.log; include manifest in error lines, r=jgraham

Comment 16

[Cameron Dawson [:camd]]

This looks awesome!  Thanks, Andrew!  :)

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/189df493159b
https://hg.mozilla.org/mozilla-central/rev/09b1aee5ad9f