1337488 - [Meta] Add a test-centric UI view that aggregates results by test/manifest rather than platform/job

Status: RESOLVED FIXED

(Tree Management Graveyard :: Treeherder: Test-based View, defect, P2)

Description

[Cameron Dawson [:camd]]

This bug will track the steps required to ingest manifest-based test results into Treeherder and have APIs to retrieve that data in a manifest-based UI.

Comment 2

[Ed Morley [:emorley]]

Prototype...
- Mockup: https://cloudup.com/cykpfMjpazy
- Repository: https://github.com/mozilla-rpweb/treeherder-manifest

The current prototype is in its own repository, since it's making use Of Neutrino/WebPack and the React build/dev workflow.

As we discussed in the last Treeherder meeting, I think it's important that we move it back to the Treeherder repo so:
- it can be hosted on the same domain (and benefit from API auth and localstorage TC tokens etc)
- it's easier for us to work on as we iterate on API design etc

The sooner we do that, the less busy-work we'll have trying to merge Git histories / design workarounds for retrigger redirection (to work around lack of auth tokens).

This means IMO bug 1336556 (or some subset of it) is on the critical path for finishing this bug.

Comment 3

[Ed Morley [:emorley]]

Example generic "all manifests that could (but might not always) run against this platform" file (generated by each build):
https://public-artifacts.taskcluster.net/MF1u7CKkSrGsXBgaS7hySA/0/public/build/target.test_manifests.json

eg:

{
  "a11y": [
    "accessible/tests/mochitest/a11y.ini",
    "accessible/tests/mochitest/actions/a11y.ini",
    "accessible/tests/mochitest/aom/a11y.ini",
    ...
  ],
  "browser-chrome": [
    "accessible/tests/browser/browser.ini",
    "accessible/tests/browser/e10s/browser.ini",
    "addon-sdk/test/browser.ini",
    "browser/base/content/test/alerts/browser.ini",
    ...
  ]
}

Example specific-job "manifests that ran in this job" file:
https://public-artifacts.taskcluster.net/GnS6rgjLT2yB6B3Ljkz-UA/0/public/test_info//manifests.list

eg:
dom/media/mediasource/test/mochitest.ini
dom/media/test/mochitest.ini

Example (from the same job as the above) structured error summary file:
https://public-artifacts.taskcluster.net/GnS6rgjLT2yB6B3Ljkz-UA/0/public/test_info//mochitest-media_errorsummary.log

eg:
{"status": "FAIL", "line": 624, "subtest": "[17:31:56.526] WebM vorbis audio & vp9 video clearkey with subsample encryption-22_case2 key session type should match", "action": "test_result", "test": "dom/media/test/test_eme_stream_capture_blocked_case2.html", "message": "Result logged after SimpleTest.finish()", "stack": null, "expected": "PASS"}
{"status": "FAIL", "line": 626, "subtest": "[17:31:56.527] WebM vorbis audio & vp9 video clearkey with subsample encryption-22_case2 message event should contain key ID array", "action": "test_result", "test": "dom/media/test/test_eme_stream_capture_blocked_case2.html", "message": "Result logged after SimpleTest.finish()", "stack": null, "expected": "PASS"}

In order to implement this feature we need to be able to associate the failures in the structured error summary with the manifest under which they actually ran.

In the example above we'd have to compare the full test filepath for each error with the list of manifests and see which was the closest match (simply removing the test filename wouldn't be sufficient I imagine, since one manifest might have multiple subfolders of tests within).

More useful would be if we could have the second and third files above merged, along with a pass/fail state for each manifest (and even perhaps the manifest group type eg {"ally, "browser-chrome", ...} too).

Andrew, does this sound doable?

Comment 4

[Ed Morley [:emorley]]

He's a (most likely wrong) first pass at the data structure that the UI might need from Treeherder's API/wherever:

{
  "mochitest": [
    "dom/media/test/mochitest.ini": {
        "osx-10-7": {
          "opt": {
            "result": "PASS",
            "errors": []
          },
          "debug": {
            "result": "FAIL",
            "errors": [
              {"status": "FAIL", "line": 624, "subtest": "[17:31:56.526] WebM vorbis audio & vp9 video clearkey with subsample encryption-22_case2 key session type should match", "action": "test_result", "test": "dom/media/test/test_eme_stream_capture_blocked_case2.html", "message": "Result logged after SimpleTest.finish()", "stack": null, "expected": "PASS"},
              {"status": "FAIL", "line": 626, "subtest": "[17:31:56.527] WebM vorbis audio & vp9 video clearkey with subsample encryption-22_case2 message event should contain key ID array", "action": "test_result", "test": "dom/media/test/test_eme_stream_capture_blocked_case2.html", "message": "Result logged after SimpleTest.finish()", "stack": null, "expected": "PASS"}
            ]
          }
        },
        "linux64": {
          "opt": {
            "result": "PASS",
            "errors": []
          },
          "debug": {
            "result": "PASS",
            "errors": []
          }
        }
      ...
    },
    "dom/media/mediasource/test/mochitest.ini": {
      ...
    },
    ...
  ],
  "browser-chrome": [
    ...
  ],
  ...
}

(The contents of `errors` is taken verbatim from the current structured error summary files, but we likely don't need all of the properties within)

Andrew/James/Will/Cameron/Eli, thoughts?

Also, one idea to prototype faster might be to fetch the manifest lists client side (either by hardcoding some examples URLs in the UI, or adding an API to Treeherder to easily retrieve all manifests for a given push) until we have a solid understanding of the data structure required.

Comment 5

[Ed Morley [:emorley]]

Oh bah I missed out how to handle infra errors. Well I guess they'd be nested directly under "mochitest", though we couldn't use the structured error summary as the source, since the file is typically empty in that case:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3bcb8220f99f47fcdd2f924e16855c172032e629&filter-searchStr=mochitest&selectedJob=77659772
http://mozilla-releng-blobs.s3.amazonaws.com/blobs/try/sha512/7dcffde40a52eaababd9894a711ab660d7f261ec2c707fc699afc7b56f00dd2e9ce45bb3909e6fe9c1b6a83c289a3df47e1189496222088e60f9f0928fbd1b5f

...so I guess we'd have to just show the standard log parser summary instead?

Comment 6

[Andrew Halberstadt [:ahal]]

(In reply to Ed Morley [:emorley] from comment #3)

Yes, we can definitely associate tests in the errorsummary with their manifests.. In fact, we could change the errorsummary format to simply include the manifest directly in it. That doesn't happen now, but we could incorporate the manifests somewhere in the structured logging data format and have the errorsummary formatter do that lookup.

If we do that, it's debatable if this build generated "master manifest list" is even necessary, especially for a prototype. The only thing it would be needed for is if you want to display a list of "pending" manifests. But we could easily skirt around that in the UI. For example, you could just say "X mochitest-browser-chrome jobs are pending", then only start displaying manifest level information when the errorsummaries/manifests.list files start coming in.

Comment 7

[Ed Morley [:emorley]]

(In reply to Andrew Halberstadt [:ahal] from comment #6)
> For example, you could just say "X
> mochitest-browser-chrome jobs are pending", then only start displaying
> manifest level information when the errorsummaries/manifests.list files
> start coming in.

Yeah I like the sound of that :-)

Comment 8

[Andrew Halberstadt [:ahal]]

(In reply to Ed Morley [:emorley] from comment #4)

I think something like this makes sense. You could probably collapse the "platform" and "buildtype" into a single key if you wanted. Would it make more sense to have platforms be a higher level key than manifests? I don't have any particular reason why they should be other than having them below manifests feels somewhat counter intuitive.

I think client side fetching makes sense for the prototype as well.. Anything that can get us *something* to play around with live data sooner, even if it's a dirty hack.

Comment 9

[Ed Morley [:emorley]]

> Would it make more sense to have platforms be a higher level key than manifests? I don't have any particular reason why they should be other than having them below manifests feels somewhat counter intuitive.

I agree it looks counter-intuitive - I'd only ordered them that way to reduce the manipulation of the data that the UI has to perform (since the UI will be nesting platforms deeper visually).

Comment 10

[William Lachance (:wlach)]

(In reply to Ed Morley [:emorley] from comment #4)
> Andrew/James/Will/Cameron/Eli, thoughts?

I think this looks pretty ok to me as a first pass. Both for the manifest errors, as well as the job errors, I think we want a reference to the job id so one can get more context in the logviewer. But that can come later.

> Also, one idea to prototype faster might be to fetch the manifest lists
> client side (either by hardcoding some examples URLs in the UI, or adding an
> API to Treeherder to easily retrieve all manifests for a given push) until
> we have a solid understanding of the data structure required.

A big ++ to this approach. There are probably some fairly minimal changes we could make to the server backend to enable us to efficiently(ish) download the manifest artifacts to generate the UI.

Comment 11

[Ed Morley [:emorley]]

The initial phase of this is complete, and we now have a dedicated Bugzilla component for this feature, making this meta-bug redundant:
https://bugzilla.mozilla.org/buglist.cgi?component=Treeherder%3A%20Test-based%20View&product=Tree%20Management&bug_status=__open__