Closed Bug 1331482 Opened 3 years ago Closed 3 years ago
Upload a list of manifests that were run as part of a mochitest job
59 bytes, text/x-review-board-request
We will be experimenting with some new manifest based UIs in treeherder this quarter and beyond. The view will likely be test and/or manifest centric as opposed to job centric, though jobs will still be used under the hood. This means treeherder will need to know which manifests (and possibly tests?) were (or will) run in a given job. In the future, we might want to have a way to bake this information into the task definition itself so treeherder can use it before the task has finished. But that is hard. For now, we can just upload an artifact after the fact containing this information.
Here's an example of what a manifests.list artifact would look like: https://public-artifacts.taskcluster.net/MCYWoo5YTI2xVxQEMKjB9w/0/public/test_info/manifests.list I also want to point out that treeherder could consume the structured log to get information on which specific tests were run. There's an argument to be made that instead of uploading this manifests.list, we should include manifest information directly in the structured log and let treeherder parse it out of there. But that will mean larger downloads for treeherder, and more post-processing (though maybe treeherder will want to process the structured log anyway). Obviously the structured log also wouldn't ever be available before hand (but neither will this manifests.list). For now I'm tempted to just go ahead with this patch and move it into the structured log later if it makes sense.. but if you guys think we will want to process the structured log anyway, we could find a way to store manifests in there from the start.
And here's a working url: https://public-artifacts.taskcluster.net/MCYWoo5YTI2xVxQEMKjB9w/0/public/test_info//manifests.list
(In reply to Andrew Halberstadt [:ahal] from comment #3) > I also want to point out that treeherder could consume the structured log to > get information on which specific tests were run. There's an argument to be > made that instead of uploading this manifests.list, we should include > manifest information directly in the structured log and let treeherder parse > it out of there. Treeherder doesn't currently consume the full structured log, but instead the uploaded error summary excerpt generated by the harness from it. I don't have a preference as to whether we use a separate file for the manifests, or whether we combine them with this structured error summary, but I think either way we will want to keep the "summary" type files (describing errors, the manifest etc) separate from the full structured log - and instead make the harness do the work, in keeping with the errors implementation. The more post-processing we can do in the harness the better - since worker capacity scales with the number of jobs, whereas Treeherder capacity does not (we can of course crank up the number of log parsers, but then our budget is the one that takes the hit, and it's harder to vary them according to load). In addition, the smaller the file Treeherder has to download, the less we're affected by issues like bug 1286935/bug 1305768.
Cool, I agree, just wanted to point it out as an option before I went ahead and landed this patch.
Attachment #8827287 - Flags: review?(james)
Comment on attachment 8827287 [details] Bug 1331482 - Upload a list of manifests as an artifact in mochitest jobs https://reviewboard.mozilla.org/r/105000/#review106280
Attachment #8827287 - Flags: review?(james) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/0be9ebdc6488 Upload a list of manifests as an artifact in mochitest jobs r=jgraham
For JS reftests this results in 26,000 test groups (and so a 3.6MB jsreftest_errorsummary.log even when no errors occurred). Is this expected? eg: https://queue.taskcluster.net/v1/task/Yl818SXQQHScaMg3gTuTFg/runs/0/artifacts/public/test_info//jsreftest_errorsummary.log
You probably meant to comment in bug 1340551. Those aren't test groups, those are the actual tests.. You'll notice they're all grouped under a single 'default' group. So yes, it's expected even if there are no errors. I agree this makes the name of the file confusing, but I didn't want to change it for fear of breaking something else. In bug 1340551 comment 16 I asked if it would be useful to include the tests in the error summary, and the answer seemed to be yes. If this is too burdensome we could either: 1) Rename errorsummaries.txt to summary.txt 2) Move that information to a separate file 3) Not print that information anywhere at all (and go back to including a group key in each errorsummary line)
Ah yes sorry I meant to comment on that bug instead. I think the extra additions may end up making the file take too long to download (particularly initially when we're doing the fetching client side in the prototype), but happy to wait until such time that's confirmed :-)
You need to log in before you can comment on or make changes to this bug.