1261816 - JS error thrown after end of test can cause finish() to be called twice

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

While working on bug 1261158 I ran into a situation where I was running |./mach mochitest --e10s -f plain image/test/mochitest/test_xultree_animation.xhtml|, the test ran and successfully completed, and then when I closed the browser window, the browser shutdown process would throw a JS error. The JS error was caught by the mochitest harness, handled in [1], but then [2] was invoked. This call to SimpleTest.finish() was in addition to the SimpleTest.finish() that the test already did, and so it caused a failure of the type "finish() already called" [3].

Now it may very well be that JS errors thrown during the shutdown process are intended to cause test failures, but if so, they should do so via handling in the code at [1], rather than the indirect "finish() already called" thing that was happening here. I think it makes sense to guard against this scenario, where the test harness is basically ignoring the actual error and then inducing some nonsensical error in its' place.

[1] http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/tests/SimpleTest/SimpleTest.js?rev=b78ed68af2fb#1543
[2] http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/tests/SimpleTest/SimpleTest.js?rev=b78ed68af2fb#1587
[3] http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/tests/SimpleTest/SimpleTest.js?rev=b78ed68af2fb#1018

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Attachment: Guard against finish() being called twice

Comment 2

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

Comment on attachment 8737776 [details] [diff] [review]
Guard against finish() being called twice

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

thanks!

Comment 3

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/126df4c0f1fb

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/126df4c0f1fb

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Uplifted to 47 with a=test-only

https://hg.mozilla.org/releases/mozilla-aurora/rev/6ebd54f5ed82