Closed Bug 1656010 Opened 4 years ago Closed 1 year ago

Provide a contextually appropriate navigation link to https://jgraham.github.io/wptdash/ from the Navigation panel and/or test info boxes

Categories

(Webtools :: Searchfox, enhancement)

enhancement

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: asuth, Assigned: asuth)

References

(Blocks 1 open bug)

Details

In bug 1653986 we added a minimal test info boxes implementation with a link to wpt.fyi in the navigation box. :jgraham has an invaluable tool at https://jgraham.github.io/wptdash/ that provides a higher level cross-cutting view of WPT metadata, including divergences between Firefox, Chrome, and Safari that we should figure out how to link to in a contextually useful fashion. Specifically, I was hoping that we might be able to pre-filter/pre-populate the dashboard constraints so that it is focused on the test file that is linked from, perhaps inside the greater context of the same directory?

:jgraham, what are your thoughts on this? Is this something already possible, or could wptdash be enhanced to do this? My goal would be to make sure the dashboard provides directly useful information related to the test file it's linked from without requiring the user to manipulate the dashboard first. (But providing a means of onboarding so that they could easily permute the configuration they're linked to to explore more.)

Flags: needinfo?(james)

I don't think it's already possible. At the moment you can just link to a specific component, and we can know what component the test is in, so we can do that part. Doing more would require changes on the wptdash side; it doesn't have any way to link to a specific test at the moment, and depending on the filters that are applied, a given test might not appear in the default view. So we'd have to think a bit about the UI we want. Probably making something like "if the supplied test is part of the view, open it up to view subtest results and scroll to it" wouldn't be too hard; making something that handles the "test just isn't in the selected test set" case nicely would be harder.

Flags: needinfo?(james)

Maybe we could generate a bug component link with an anchor that includes the full (WPT, not mozilla-central) test path and wptdash could be altered to emit id attributes for each path, maybe in this code and/or with magic CSS styling using :target to highlight it? That way if the test is present, it will be scrolled-to and/or highlighted, but if it's not, the user has a useful subset of information to look at?

And I guess something could also be done to emit an info box that says "the test you linked to is awesome and has no issues, but here's other things in the component" if people get confused.

Flags: needinfo?(james)

So I tried this; now something like https://jgraham.github.io/wptdash/#/mediacapture-streams/MediaStreamTrack-getCapabilities.https.html will open expanded, but the target bit doesn't work; presumably that's only resolved at page load and we haven't created the element at that point. It's probably possible to make something along those lines work though.

Flags: needinfo?(james)

Awesome!

Yeah, I think it's a dynamic load issue, as if I do document.location = document.location or document.location.hash = document.location.hash in the console, then the page scrolls to the element and it properly gets highlighted. I think we used to do something like this in searchfox (explicit delayed location update), but it ended up turning out that as long as we created the id element early enough that it wasn't a problem. I think right now the Highlight constructor handles this and is guaranteed to happen before the load event or things like that. Presumably whatever dynamic process triggers the initial content population could tap the location in an effect hook or componentDidMount or whatever.

I think this is now actionable from a searchfox perspective, but I need to focus on non-searchfox stuff for a little bit, so if someone would like to take this on in the meantime:

Keywords: good-first-bug
See Also: → 1732790
Assignee: nobody → bugmail
Status: NEW → ASSIGNED
See Also: → 1797855

I added links to the interop dashboard described here in a subset of the commits at https://github.com/mozsearch/mozsearch/pull/596 with the key notes being:

  • We now download and ingest the MANIFEST.json files for the wpt tests and our mozilla specific tests (downloaded in the change in https://github.com/mozsearch/mozsearch-mozilla/pull/192) so that we can understand the set of test IDs associated with a given source file.
    • All previous work in searchfox related to WPTs had assumed that test are identified by their source file, but in reality they are identified by their test IDs.
    • A nuance is that the test IDs canonically start with "/" but in our "concise" file info for the WPT files that we extract, I omit the leading "/" because for a lot of our string-space path manipulations we don't want that, so the cases where we want the true test ID, we need to have a "/".
  • Since there can be a bunch of test IDs for a given .any.js file, we list them in the test info box, and the WPT dashboard link got pulled out of the navigation panel to instead be on each of the test IDs since there is a different link for each test ID. The new interop links (the point of this bug) end up there too.
  • The URLs do need to include the ?bugComponent=... payload or the dashboard seems to use the top bug component in the list.

I'm going to need to spin off a follow-up for one problem I realized at the end of testing that is not yet addressed and won't fix in my current (exceeded) time box: A string like This test has a WPT meta file that expects 1 subtest issues. is going to be potentially wrong if the test has more than 1 test ID. The issue is that we should either be exposing the subtest issues on a per-test ID basis or we need to sum the subtests. My original approach with the subtests was "these seem verbose, let's just indicate any exist and be done with it", but it's also possible it makes sense to provide more details than just the aggregate number.

Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Blocks: 1804196
You need to log in before you can comment on or make changes to this bug.