1391694 - test-verify frequently fails on browser-chrome, debug: leaked (n) windows until shutdown

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[Geoff Brown [:gbrown]]

https://treeherder.mozilla.org/logviewer.html#?job_id=124090904&repo=mozilla-inbound

https://treeherder.mozilla.org/logviewer.html#?job_id=124082764&repo=mozilla-inbound

https://treeherder.mozilla.org/logviewer.html#?job_id=124006345&repo=mozilla-inbound

Comment 1

[Geoff Brown [:gbrown]]

:ted's on to something here:

<ted> https://dxr.mozilla.org/mozilla-central/source/testing/mochitest/browser-test.js#208
<ted> j'accuse!
<ted> so the browser-chrome harness considers itself done if the current test is the last test
<ted> ...and when it goes to run the next test it calls this code: https://dxr.mozilla.org/mozilla-central/source/testing/mochitest/browser-test.js#603
<ted> and in *that* block is where we trigger the GC stuff: https://dxr.mozilla.org/mozilla-central/source/testing/mochitest/browser-test.js#637
<ted> so i suspect that whole block gets called too early in the --repeat case
<ted> and things break

Comment 3

[Geoff Brown [:gbrown]]

Attachment: avoid extra cleanup and leak checks in browser-chrome tests run with --repeat

:ted - Thanks much for pointing this out to me; it was a great help.

The issue here is that the block of cleanup code at https://dxr.mozilla.org/mozilla-central/rev/37b95547f0d27565452136d16b2df2857be840f6/testing/mochitest/browser-test.js#610 should only be run just before the browser is closed, but done() doesn't take --repeat into account, so that block runs after every test when --repeat is specified.

This patch updates done() to report false until all test repetitions are complete and re-organizes the repeat handling a little so that neither the cleanup code nor finish() is called until all test repetitions are complete.


This change does not break existing browser-chrome tests:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9fd783c676a33071dc328e1695d217a4c0445649

and fixes my concern with --verify (which uses --repeat):

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7a71eb28398b74ee5ff78f75de1c2dd558c752bc&filter-tier=1&filter-tier=2&filter-tier=3

I also tested locally with:

mach mochitest <single-test>
mach mochitest <directory>
mach mochitest <single-test> --repeat=10
mach mochitest <directory> --repeat=10

Comment 4

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

Comment on attachment 8906156 [details] [diff] [review]
avoid extra cleanup and leak checks in browser-chrome tests run with --repeat

Review of attachment 8906156 [details] [diff] [review]:
-----------------------------------------------------------------

thanks for this fix Geoff

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/670be1db5eed
Avoid false shutdown leaks in browser-chrome tests with --repeat; r=jmaher

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/670be1db5eed