Skip to content
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

Add github checks output support to stability check #24741

Merged
merged 2 commits into from
Aug 4, 2020

Conversation

stephenmcgruer
Copy link
Contributor

No description provided.

@stephenmcgruer stephenmcgruer changed the title Add github checks output support to stability check [WIP] Add github checks output support to stability check Jul 24, 2020
@wpt-pr-bot wpt-pr-bot temporarily deployed to wpt-preview-24741 July 24, 2020 18:32 Inactive
@wpt-pr-bot wpt-pr-bot temporarily deployed to wpt-preview-24741 July 24, 2020 19:31 Inactive
@wpt-pr-bot wpt-pr-bot temporarily deployed to wpt-preview-24741 July 24, 2020 20:33 Inactive
@wpt-pr-bot wpt-pr-bot temporarily deployed to wpt-preview-24741 July 27, 2020 21:29 Inactive
@wpt-pr-bot wpt-pr-bot temporarily deployed to wpt-preview-24741 July 30, 2020 14:22 Inactive
@wpt-pr-bot wpt-pr-bot temporarily deployed to wpt-preview-24741 July 30, 2020 16:17 Inactive
@wpt-pr-bot wpt-pr-bot temporarily deployed to wpt-preview-24741 July 30, 2020 20:59 Inactive
@wpt-pr-bot wpt-pr-bot temporarily deployed to wpt-preview-24741 July 30, 2020 21:46 Inactive
@stephenmcgruer stephenmcgruer changed the title [WIP] Add github checks output support to stability check Add github checks output support to stability check Jul 31, 2020
@stephenmcgruer stephenmcgruer marked this pull request as ready for review July 31, 2020 02:29
@wpt-pr-bot wpt-pr-bot added ci dom infra wptrunner The automated test runner, commonly called through ./wpt run labels Jul 31, 2020
@wpt-pr-bot wpt-pr-bot temporarily deployed to wpt-preview-24741 July 31, 2020 02:34 Inactive
@stephenmcgruer stephenmcgruer assigned jgraham and unassigned zqzhang Jul 31, 2020
@wpt-pr-bot wpt-pr-bot temporarily deployed to wpt-preview-24741 July 31, 2020 11:23 Inactive
dom/historical.html Outdated Show resolved Hide resolved
tools/wptrunner/wptrunner/wptlogging.py Outdated Show resolved Hide resolved
Due to taskcluster/taskcluster#3191, this
requires us to set both textArtifactName and annotationsArtifactName.
After that fix is rolled out to Taskcluster, we can switch to just using
textArtifactName.
@wpt-pr-bot wpt-pr-bot temporarily deployed to wpt-preview-24741 August 3, 2020 14:50 Inactive
@wpt-pr-bot wpt-pr-bot temporarily deployed to wpt-preview-24741 August 3, 2020 14:59 Inactive
@wpt-pr-bot wpt-pr-bot temporarily deployed to wpt-preview-24741 August 3, 2020 15:25 Inactive
@wpt-pr-bot wpt-pr-bot temporarily deployed to wpt-preview-24741 August 3, 2020 15:33 Inactive
Copy link
Contributor Author

@stephenmcgruer stephenmcgruer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please take another look. I don't love my code here...

On a more meta level, I'm also having trouble in framing the instructions for test authors. Specifically, I want to be able to say something like "please use the printed WPT command to reproduce", but the command as dumped will have multiple flags they probably don't want to use (i.e. --github-checks-text-file=/home/test/artifacts/checkrun.md and --affected base_head). So far I've changed it to a weaker sentence, but maybe I should be even more explicit, something like:

Please try to reproduce; see the above WPT command for what the bot ran, but note you may need to remove some flags to run locally.

Which is still confusing to test authors :/.

tools/ci/tc/tasks/test.yml Show resolved Hide resolved
tools/wptrunner/wptrunner/wptlogging.py Outdated Show resolved Hide resolved
Copy link
Contributor

@jgraham jgraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty exciting!

I don't have a great suggestion for showing the command that you should run to get things working locally without some terrible hack where you have a blocklist of flags to remove. We never quite managed to solve this issue for gecko ci (although in that case things are worse because the ci uses a totally different entrypoint compared to the local runs).

@wpt-pr-bot wpt-pr-bot temporarily deployed to wpt-preview-24741 August 4, 2020 12:53 Inactive
@wpt-pr-bot wpt-pr-bot temporarily deployed to wpt-preview-24741 August 4, 2020 13:03 Inactive
@stephenmcgruer stephenmcgruer removed the dom label Aug 4, 2020
@stephenmcgruer
Copy link
Contributor Author

Reverted the test change, will land this once CI is green. For posterity, this is what it looks like:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci infra wptrunner The automated test runner, commonly called through ./wpt run
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants