-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move stability checking to wpt run --verify
#9874
Comments
wpt run --stability
wpt run --verify
Hey @jgraham . I would like to work on this issue. Can I get a little insight on how to proceed. |
@akshitac8 Are you still working on this one ? |
Sorry, I somehow missed the comments here. I think this isn't a trivial issue and probably not suitable as a first bug because I currently don't know what's required to land this. |
yes @kriti21, Its taking a little time. sorry for the inconvience:) |
Hello, |
We have both:
What's the difference between these? |
|
@jgraham do we want to support both? should we drop support for one? we should at least document the difference somewhere, because at the moment the |
The intent of this issue is to move to using |
There's a great deal of overlap with #10269 and perhaps they'll be fixed by the same PR, but for now I'll just align the labels and assign both to @gsnedders. |
OK, so in short, the current flow is:
The hardest part is that we need to get the data structure that |
@gsnedders, could @lukebjerring @Hexcles, is that what we'd want for web-platform-tests/wpt.fyi#118? |
@foolip Without having tested, I would assume it would support any of wptrunner's output formats, given it's part of wptrunner? That said, pulls.web-platform-tests.org expects to get data POSTed in a specific format, so somewhere we need to convert the data into that form (or kill the dashboard entirely; see #10923) before making the request there. |
If, as a part of the work on this issue, posting to pulls.web-platform-tests.org no longer happens (because it would require additional work) I think that'd be fine, since it's no longer commenting, and when it was it was incredible hard to figure out if there were regressions or not. If we want to revive it before solving the same problem with wpt.fyi, lumping that into the same work seems OK. @jugglinmike, does that sound reasonable, or too willing to break things? |
I have yet to contribute to the pull request dashboard, but out of coincidence (i.e. our effort to centralize secrets), I'm almost in a position to deploy changes to that application. My preference would be to diagnose and correct the existing problem before making further changes since proceeding in the reverse order (or consolidating two steps) would make bug fixing more difficult.
Do you mean "regressions [in the WPT pull request being validated]" or "regressions [in the integration between WPT and the pull request dashboard"? |
I mean "regressions [in the WPT pull request being validated]". |
I'm pretty sure the immediate state after we fix this isn't going to stop posting results, FWIW. |
@foolip I don't know how multiple runs will interact with wptreprot. Even if they end up in the report, I don't think various readers on the wpt.fyi project can understand multiple runs of a same test. However, the whole code path shouldn't be too hard to fix. And using wptreport is the right way to go IMHO. |
@Hexcles, I suppose the options are N wptreport JSON files, or 1 wptreport JSON files representing all runs. I take it you prefer separate files? But how would one represent a run with AABB test order? (I'm guessing that's possible to achieve with some combination of flags.) |
Note this doesn't yet address web-platform-tests#9874, as it currently only runs the repeat_restart mode of --verify (as it did previously).
So what I haven't done so far is move over to running the multi-part verify (with/without restarts, with/without chaos mode in Firefox), but it does now use the verify code. At the moment, it's still calling into the code through Python, rather than letting it run separately and then reading the logs. If we want to avoid going through the logs, we'd need to either make |
Note this doesn't yet address web-platform-tests#9874, as it currently only runs the repeat_restart mode of --verify (as it did previously).
Note this doesn't yet address web-platform-tests#9874, as it currently only runs the repeat_restart mode of --verify (as it did previously).
Note this doesn't yet address #9874, as it currently only runs the repeat_restart mode of --verify (as it did previously).
Note this doesn't yet address web-platform-tests#9874, as it currently only runs the repeat_restart mode of --verify (as it did previously).
Since we developed the stability checker, wptrunner got a
--verify
flag for checking test stability. Ideally instead of having separate codepaths here we could keep all the travis handling logic from the check_stability script but do the actual run using the--verify
option to wpt run, so that we are confident that the same CI checks apply to both.The text was updated successfully, but these errors were encountered: