1782700 - Test spams log if test fails in browser_setup_state.js

Status: RESOLVED FIXED

(Testing :: Mochitest, task, P3)

Description

[Sarah Clements [:sclements]]

In bug 1776779, tests were changed from navigating to a new tab to using withFirefoxView.

After a rebase, I had to fix some failing tests that relied on these changes and I discovered that if a test just hanged (before it would time out or fail), the terminal gets spammed with FAIL Found a tab after previous test timed out: about:blank - until you stop it with ctrl + c.

Comment 1

[:Gijs (he/him)]

FWIW I suspect this is an issue with the browser test framework which tries to close the offending (fxview) tab, but is unable to, and then gets stuck in an infinite loop, so we may wish to move this to Testing :: mochitest. But being sure about this suspicion would require spending time investigating it which I don't currently have...

Comment 2

[Sam Foster [:sfoster] (he/him)]

Yeah we are getting stuck in a while loop. When the firefoxview hidden tab has been shown, it shows up in the gBrowser.tabs array. The test framework has a while (gBrowser.tabs.length > 1) loop to clean up tabs left open by a test. However testing what that loop does manually, removing the fxview tab seems to work fine in all the conditions I've tried so far, so I'm not sure what is going on here.

I agree that while we can try not to fail so horribly in the fxview tests, the framework shouldn't allow itself to get stuck in an infinite loop so I'll move this. To reproduce, you just need to make a task timeout, e.g. by changing the element id in this call to waitForVisibleStep to an element which won't get shown, like "#tabpickup-steps-view0".

Comment 3

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

in the while loop if we hit the same tab multiple times, then maybe we terminate the harness/testrun (no future tests run in that manifest). It would stop the spamming of errors, the downside I see is in this case we would terminate tests earlier and possibly skip other test coverage- although if we assume these issues are not perma failing, then this will only happen periodically (quite possibly this is the same result as we have now)

Comment 4

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

I hacked on this and reproduced the problem on try (the patch that was backed out), then added some logic to the while loop to exit quickly:
https://treeherder.mozilla.org/jobs?repo=try&tier=1%2C2%2C3&revision=7b1c170fd7006357e052697acd979c50fe70fbae

this seemed to be doing what I thought. Aryx, from a sheriff point of view, what messaging would you like to have with this fix?

Comment 5

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

The following failure message would be useful
TEST-UNEXPECTED-FAIL | browser/components/firefoxview/tests/browser/browser_keyboard_focus.js | terminating browser early - unable to close tabs; skipping remaining tests in folder
This creates an obvious connect to the executed test, else one would have to check log if this is a different issue.

Comment 6

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

Attachment: Bug 1782700 - terminate browser-chrome session if unable to close tab. r=gbrown!

Comment 7

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/b2741f381524