Prevent WPT tests from potentially affecting other tests
Categories
(Core :: DOM: Notifications, task)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox128 | --- | fixed |
People
(Reporter: saschanaz, Assigned: saschanaz)
References
(Regressed 1 open bug)
Details
Attachments
(3 files)
Running ./mach wpt /notifications/constructor-basic.https.html /notifications/constructor-invalid.https.html for example causes Notification::CloseInternal being called for the notifications opened by the first one while running the second one. This technically shouldn't affect the second test itself, but we shouldn't run any task after the test finish to prevent confusing intermittent results.
|
Assignee | |
Comment 1•4 months ago
|
||
What's weird is that we shouldn't even call CloseInternal as constructor-basic has no permission to open anything 🤔
|
|
Assignee | |
Updated•2 months ago
|
|
|
Assignee | |
Comment 2•16 days ago
|
||
|
|
Assignee | |
Comment 3•16 days ago
|
||
Fixing one test to set permission properly and another to not trigger external network request.
Pushed by krosylight@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/460cba558bfc Part 1: Stop requirng notification.prompt.testing for set-permission r=webdriver-reviewers,Sasha https://hg.mozilla.org/integration/autoland/rev/92ba50ef9857 Part 2: Adjust notification tests r=asuth,dom-worker-reviewers
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/46342 for changes under testing/web-platform/tests
|
||
Comment 6•15 days ago
|
||
Backed out for causing mozilla::PermissionManager::CommonTestPermissionInternal related wpt crashes.
- Backout link
- Push with failures
- Failure Log
- Failure line: PROCESS-CRASH | MOZ_ASSERT(PermissionAvailable(prin, aType)) [@ mozilla::PermissionManager::CommonTestPermissionInternal] | /notifications/permission.html
|
|
Assignee | |
Comment 7•15 days ago
|
||
But it was clean in https://treeherder.mozilla.org/jobs?repo=try&revision=4502ed827177847cc26dabd18a955c273becaede 😞
|
|
Assignee | |
Comment 8•15 days ago
|
||
So it's always permission.html, at least that's clear. For now I'll just mark it as CRASH as this is blocking other test changes and I think this crash has been there for a while, if not forever.
Upstream PR was closed without merging
|
||
Comment 10•11 days ago
|
||
Hey @saschanaz, are you planning to push these changes again soon? I just have the stack on bug 1524074 which I rebased on these changes, and now they are ready to go. If you don't have time at the moment to finish, I can revert the update and land my stack, and then you will have to rebase on my changes. Let me know, what works for you.
|
|
Assignee | |
Comment 11•11 days ago
|
||
I think I'll investigate a bit more, so feel free to land first 👍 (I locally modified the test and moved the current test to crashtest, and then the crashtest does not crash anymore, so 🤷♀️)
|
|
Assignee | |
Comment 12•11 days ago
|
||
This is sad but at least we'll have a test that fails so that GeckoView people can take a look.
Removing OK/TIMEOUT as I think that's just bug 1798871.
|
||
Comment 13•10 days ago
|
||
Pushed by krosylight@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/51eecbe291e0 Part 1: Stop requirng notification.prompt.testing for set-permission r=webdriver-reviewers,Sasha https://hg.mozilla.org/integration/autoland/rev/5ab2cff5dfce Part 2: Adjust notification tests r=asuth,dom-worker-reviewers https://hg.mozilla.org/integration/autoland/rev/03da35eb0a10 Part 3: Mark permission test CRASH on Android r=asuth,dom-worker-reviewers,smaug
|
||
Comment 14•9 days ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/51eecbe291e0
https://hg.mozilla.org/mozilla-central/rev/5ab2cff5dfce
https://hg.mozilla.org/mozilla-central/rev/03da35eb0a10
Upstream PR merged by moz-wptsync-bot
Description
•