Closed Bug 1370359 Opened 2 years ago Closed 2 years ago

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

Categories

(Testing :: Mozbase, enhancement)

Version 3
enhancement
Not set

Tracking

(firefox55 fixed)

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: camd, Assigned: ahal)

References

Details

Attachments

(2 files)

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).
Hey Andrew, here's the bug we talked about today.  Thanks heaps!  :)
Flags: needinfo?(ahalberstadt)
Blocks: 1337488
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.
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 :/
Flags: needinfo?(ahalberstadt) → needinfo?(cdawson)
(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?
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.  :)
Flags: needinfo?(cdawson)
Great, makes sense! Should be an easy change, I'll have it up shortly.
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
Summary: Create a manifest-centric Mozbase error summary file → [mozlog] Add manifest to test objects in errorsummary.log
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.
Attachment #8875339 - Flags: review?(james)
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 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
Attachment #8875339 - Flags: review?(james) → review+
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
Attachment #8875417 - Flags: review?(james) → review+
Pushed by ahalberstadt@mozilla.com:
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
This looks awesome!  Thanks, Andrew!  :)
https://hg.mozilla.org/mozilla-central/rev/189df493159b
https://hg.mozilla.org/mozilla-central/rev/09b1aee5ad9f
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Blocks: 1347945
You need to log in before you can comment on or make changes to this bug.