web-platform-tests / wpt.fyi

web-platform-tests dashboard
https://wpt.fyi/
Other
188 stars 90 forks source link

Use MANIFEST.json as the ground truth for validating and displaying results #56

Open foolip opened 6 years ago

foolip commented 6 years ago

Moved from https://github.com/web-platform-tests/results-collection/issues/123.

Two related problems:

  1. Some test suites have a lot of manual tests, and making this clear would be useful. Example: https://wpt.fyi/fullscreen/ vs. https://github.com/w3c/web-platform-tests/tree/master/fullscreen
  2. Not all runners will run all test types (especially wdspec) and may have bugs that cause certain tests to have no recorded results at all.

These could be solved by somehow using MANIFEST.json for each commit, or information derived from it. That could also be used to do something more useful for https://wpt.fyi/does/not/exist/, although that's probably already doable, just not implemented.

foolip commented 6 years ago

The discussion in https://github.com/web-platform-tests/results-collection/issues/98 might also be relevant to this, although we could at most align the tests, not the subtests.

lukebjerring commented 6 years ago

From https://github.com/web-platform-tests/results-collection/issues/123, @gsnedders said:

That relies on everyone implementing runners correctly. I think there's definitely an argument that it's the collection side that should handle it?

There's an imbalance here with WPT run not working cross-platform that means we really are likely to have multiple runner implementations (already do for Edge, Content Shell). I feel like if we can't necessarily trust the clients/runners to do the right thing, then the step for fleshing out "missing"/manual/un-run tests as per manifest can be shoved server-side, but I still think it needs to be (implicit) in the results JSON blobs, not implemented by storing a copy of the as-was manifest for every run (or retrospectively loading it from the repo, or other bug-prone approaches).

i.e. It should be part of the post-processing of the results receiver.

foolip commented 6 years ago

@Hexcles, WDYT, would it make sense for the results receiver to combine information from the run and the manifest to produce the final form suitable for wpt.fyi?

Hexcles commented 6 years ago

I agree with @gsnedders and @lukebjerring here.

The benefits of implementing this inside the results-receiver is two-fold:

I'd like to implement this together with "validating the incoming results (against manifest)" in the second iteration (the first batch of non-crucial features).

foolip commented 6 years ago

@Hexcles can you rename this issue to better describe the solution you have in mind?

Hexcles commented 6 years ago

The problem is twofold:

  1. Use manifest to validate results and calculate completeness.
  2. Listing tests based on manifest. If a test doesn't have result (from any browser), including manual tests, put a placeholder N/A in the UI.

The two goals can be achieved in the same process. In the results processor, we iterate over the tests listed in the manifest and try to find the corresponding results from the wptreport JSON (instead of the other way around).

And we must make sure we have the correct manifest (at the right revision). I imagine (and certainly wish) all test runs happen at PR merge points. If so, we can easily download the exact manifest from GitHub releases.

foolip commented 6 years ago

Could the results receiver use wpt manifest locally? That will download when possible and otherwise generate.

Hexcles commented 6 years ago

Yes! The Flex environment has a lot of flexibility. In fact, we can run any Docker image, and for wpt manifest we don't even need to shell out.

On Wed, Apr 25, 2018, 05:49 Philip Jägenstedt notifications@github.com wrote:

Could the results receiver use wpt manifest locally? That will download when possible and otherwise generate.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/web-platform-tests/wpt.fyi/issues/56#issuecomment-384229764, or mute the thread https://github.com/notifications/unsubscribe-auth/ABXsjLaLjU7_ZNLm6fIt8qSDJOfXJBKhks5tsEa_gaJpZM4TX6my .

foolip commented 6 years ago

Sounds like it was named appropriately :)

foolip commented 5 years ago

I've been working in https://github.com/foolip/wpt-stats to see if we can ensure there are manifests available for download for at least all master runs, and I've filed https://github.com/web-platform-tests/wpt/issues/19311 for a few exceptions where that's not the case.

@Hexcles I'm not sure this is really how you want to get the manifest. Generating one in the receiver is probably doable, but I think there will be some commits which aren't reachable from any ref. Should we instead require a manifest to be submitted together with the reports? Or fall back to a manifest that's close in the commit graph?