1878741 - Prevent WPT tests from potentially affecting other tests

Status: RESOLVED FIXED

(Core :: DOM: Notifications, task)

Description

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

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.

Comment 1

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

What's weird is that we shouldn't even call CloseInternal as constructor-basic has no permission to open anything 🤔

Comment 2

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Attachment: Bug 1878741 - Part 1: Stop requirng notification.prompt.testing for set-permission r=whimboo,#webdriver-reviewers

Comment 3

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Attachment: Bug 1878741 - Part 2: Adjust notification tests r=asuth,#dom-worker-reviewers

Fixing one test to set permission properly and another to not trigger external network request.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
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

Comment 5

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/46342 for changes under testing/web-platform/tests

Comment 6

[Serban Stanca [:SerbanS]]

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

Comment 7

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

But it was clean in https://treeherder.mozilla.org/jobs?repo=try&revision=4502ed827177847cc26dabd18a955c273becaede 😞

Comment 8

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

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.

Comment 9

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 10

[Alexandra Borovova [:Sasha]]

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.

Comment 11

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

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 🤷‍♀️)

Comment 12

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Attachment: Bug 1878741 - Part 3: Mark permission test CRASH on Android r=asuth,#dom-worker-reviewers

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

[Pulsebot]

Pushed by [e-mail]:
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

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/51eecbe291e0
https://hg.mozilla.org/mozilla-central/rev/5ab2cff5dfce
https://hg.mozilla.org/mozilla-central/rev/03da35eb0a10

Comment 15

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot