-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Wrap Feature Policy common code in setup() #15577
Conversation
1ab1e9a
to
0f2f73e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know much about the ReportingObserver, but the code looks fine to me. Do you want me to confirm it's addressing the payment request API failures in Gecko?
@marcoscaceres this wasn't intended to change the results in Firefox, and it looks like results are unaffected: What it should do is change the failure mode for Safari, so that there's a harness error and it doesn't look like Firefox has a lone failure there. |
Ah, apologies, I'd misunderstood. |
web-platform-tests/wpt.fyi#1160 is the bug about Safari results not appearing. The results were uploaded though, and I was able to find the two runs to compare: This had the intended effect, and should remove some things from a list of Firefox-specific failures, and maybe something from a list of Safarai-specific failures too. |
That ReportingObserver isn't available in Safari causes the test to be treated as a single page test with a single failing subtest: https://wpt.fyi/results/feature-policy/experimental-features/layout-animations-disabled-violation-report-js-tentative.html?run_id=5374658471788544&run_id=5135755412242432&run_id=5719614843518976 Wrapping it will cause there to be a harness error instead. This will help with https://bugzilla.mozilla.org/show_bug.cgi?id=1530648.
0f2f73e
to
458d829
Compare
There are no reviewers for this pull request. Please reach out on W3C's irc server (irc.w3.org, port 6665) on channel #testing (web client) to get help with this. Thank you! |
I just rebased to runs checks again before pinging review of this. I see that @bakulf is the only suggested reviewer, but not added because of not being an org member. @bakulf, I've sent an org invite to sort that out, please accept if you're willing to do reviews here. @clelland, looks like you were removed in #14798, sent #18537 to fix that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, and I'm happy to see the more useful results from the Safari bot
That ReportingObserver isn't available in Safari causes the test to be treated as a single page test with a single failing subtest: https://wpt.fyi/results/feature-policy/experimental-features/layout-animations-disabled-violation-report-js-tentative.html?run_id=5374658471788544&run_id=5135755412242432&run_id=5719614843518976 Wrapping it will cause there to be a harness error instead. This will help with https://bugzilla.mozilla.org/show_bug.cgi?id=1530648.
That ReportingObserver isn't available in Safari causes the test to be
treated as a single page test with a single failing subtest:
https://wpt.fyi/results/feature-policy/experimental-features/layout-animations-disabled-violation-report-js-tentative.html?run_id=5374658471788544&run_id=5135755412242432&run_id=5719614843518976
Wrapping it will cause there to be a harness error instead.
This will help with https://bugzilla.mozilla.org/show_bug.cgi?id=1530648.