1688572 - accessible/tests/mochitest/elm/test_HTMLSpec.html permafails

Status: NEW

(Core :: Disability Access APIs, defect, P3)

Description

[Mats Palmgren (inactive)]

Here's a Try push with just a white-space change to test_HTMLSpec.html:
https://treeherder.mozilla.org/jobs?repo=try&selectedTaskRun=bjMsYbOFTzy5wOEsYnvOJg.0&revision=aef0c001a155ee8f764f129b2c6eeb7c50624422

It permafails with:

WARNING: YOU ARE LEAKING THE WORLD (at least one JSRuntime and everything alive inside it, that is) AT JS_ShutDown TIME. FIX THIS!

Normally, this failure never shows up because the test is only conditionally run. This is bad because when it finally runs the author of the patch may think they did something wrong that caused this error (and waste several hours investigating it).

I suggest we disable this test until it's green, or at least intermittently succeeds.

We have a policy that permafailing tests must be disabled, right?

Comment 1

[James Teh [:Jamie]]

That's deeply concerning. This test was last modified in November 2020 when plugins were removed, so I assume it must have passed at that time in order to land. So, something regressed this in the last couple of months.

I didn't realise these tests were conditional now and I'm not familiar with what conditions trigger them. I assume this should have run for any a11y engine changes, so that suggests something outside of a11y regressed this.

Comment 2

[Morgan Reschenberg [:morgan]]

not sure what to triage this as since its test stuff, but setting P2 since this seems like an important thing

Comment 3

[James Teh [:Jamie]]

I can't reproduce this locally. Urgh.

Comment 4

[James Teh [:Jamie]]

Hi Joel. Mats notes that this test (accessible/tests/mochitest/elm/test_HTMLSpec.html) is only run conditionally. Are you able to provide any insight into the conditions under which it would run? As I noted in comment 1, this test was only modified a couple of months ago and it passed then. Since then, Mats has discovered it is permafailing. However, we've definitely had code land in the accessible module since then and I assume the test would at least be run in that case.

If this test (and maybe other a11y tests) is only run for changes to the accessible module (or specific subsets of it), that's somewhat concerning because changes to dom, layout, toolkit and other modules (but mostly those three) can impact a11y.

Thanks for any info you can provide.

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

this test will run whenever a11y tests are run. For example every m-c push (~3 /day) runs a11y on all configs supported and I spot checked that test_HTMLSpec.html runs:
https://treeherder.mozilla.org/jobs?repo=mozilla-central&revision=2610d2d33a739cacaba82b55b0f81e4758cd6a43&test_paths=accessible%2Ftests%2Fmochitest%2Felm&selectedTaskRun=TJHQukhmQliiOXQK-ClSSw.0

With that said, the try push has failures on test-verify; these tests run standalone as a single test in a single browser session, no directory of tests. Given that test_htmlspec.html is the first test in the list, I assume it doesn't cleanup properly, therefore the subsequent tests do the cleanup or allow the browser enough time to clean up on its own.

at some point in time we would like to ensure that ALL tests run fine by themselves, this is why we break things and try to make sure everything runs in test-verify. If this test doesn't run standalone and it is too hard to fix, then there is a fix, add a skip-if = verify to the manifest (prior art: https://searchfox.org/mozilla-central/search?q=skip-if+%3D+verify&case=true&path= )

Comment 6

[Frank Griffith Jr]

Jamie, is this still an issue and if so, can we update this with a Severity? Thanks.