1569559 - Metadata update still removing release_or_beta conditions

Status: RESOLVED FIXED

(Testing :: web-platform-tests, defect, P1)

Description

[James Graham [:jgraham]]

See https://bugzilla.mozilla.org/show_bug.cgi?id=1568835

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a task, but please change it back in case of error.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:jgraham, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 3

[James Graham [:jgraham]]

As best as I can tell this is now working; I only get release_or_beta conditions removed when the value of the condition matches the new default (e.g. if we have if release_or_beta: PASS and then the test starts passing everywhere). I wonder if the previous incident was caused by running an update with an older gecko base not including the fixes. Please reopen if the bug reappears.

Comment 4

[James Graham [:jgraham]]

The problem here is that we are ending up with different subtest names on beta and nightly. The logic to remove stale subtests doesn't account for that possibility. Ideally we wouldn't have different test names in the PASS vs FAIL case. Can we look at the specific webaudio test that's causing problems and try to figure out why we're seeing that? If possible we should fix the test, otherwise we might need to make the cleanup code ignore conditions that it couldn't itself have created. That does mean that we may end up with stale conditions if there are release_or_beta conditions on subtests that are later removed.

Comment 5

[Karl Tomlinson (:karlt)]

Thank you for the analysis.

audioworklet-addmodule-resolution.https.html is failing on release_or_beta before any subtests have been created.
The harness then assumes the test is a single page test and creates an imaginary subtest name based on the title for the file.

This test is not using setup() to wrap the code that is failing. Wrapping in setup() changes the failure to a toplevel ERROR instead of a subtest FAIL, which the avoids the problem of unexpected subtest names. I've proposed this change in https://phabricator.services.mozilla.com/D46685.

In general, can a failure during set-up cause a different number of subtests to run? We often may not notice fallout from removing stale state because hopefully at least as many subtests would run on nightly as on beta, but the reverse seems quite possible.

It would seem reasonable that release_or_beta conditions would be considered stale only if there were data to support that, but addressing that may be less urgent with https://phabricator.services.mozilla.com/D46685.

Comment 6

[Karl Tomlinson (:karlt)]

As observed in https://bugzilla.mozilla.org/show_bug.cgi?id=1585946#c2, and reported in https://github.com/web-platform-tests/wpt/issues/10201 and https://bugzilla.mozilla.org/show_bug.cgi?id=1458068 there are still a bunch of imported webaudio tests that do not have consistent subtest names and so are continuing to cause problems.

https://phabricator.services.mozilla.com/D47025 is one part of changes required to make test names consistent.

Comment 7

[Karl Tomlinson (:karlt)]

Even if tests are fixed to aim to produce consistent subtest names on failure, there is still a chance of differing sets of names if there is a crash. I guess that situation can be special cased, if desired. I haven't checked whether or not it already is.

Comment 8

[James Graham [:jgraham]]

I"m going to look at this either this week or at the start of next year.

Comment 9

[James Graham [:jgraham]]

Attachment: Bug 1569559 - Don't remove subtests where a condition is outside the updatable set,

Sometimes we see subtests that only appear in certain conditions e.g. if we
get an early error on nightly but the tests run as expected in stable. In this
case the wpt sync bot will update the metadata with --full and remove the
missing subtests. To work around this, check if subtests contain any
conditions that aren't part of the update set, and if so never
remove them

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/256f8dffafec
Don't remove subtests where a condition is outside the updatable set, r=maja_zf

Comment 11

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

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

Comment 12

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

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 13

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

Upstream PR merged by moz-wptsync-bot

Comment 14

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/256f8dffafec