1415247 - Check that firefox processes are cleaned up across test runs

Status: RESOLVED FIXED

(Testing :: General, enhancement)

Description

[Geoff Brown [:gbrown] (pto Jun 2-Jun 5)]

In bug 1397201, it seems that bad things are happening to tests on Windows, at least in part because firefox processes from previous test runs are still running.

Are we doing everything we can to kill firefox at the end of a test run?

Can we kill firefox if it is found running before a test is started?

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I already added that to bug 1415245, but will do it here again:

Please note that for bug 1397201 we seem to collect orphaned Firefox processes while mochitests are running. Not sure how mochitests are working, but they start Firefox a couple of times. So ideally for each start of Firefox such an output would be good to have.

Comment 2

[Geoff Brown [:gbrown] (pto Jun 2-Jun 5)]

I realized that I run Firefox while running mochitests locally all the time -- that's not a problem (since none of my local Firefox instances are running marionette servers). Anyway, we cannot fail just because Firefox is running.

We can report when 'firefox' is running, even warn about it, and that might be useful in automation, where we don't expect to see extra Firefox instances at all.

Also, runtests.py can fail if it finds a browser running that it knows it started and thinks was cleaned up.

Comment 3

[Geoff Brown [:gbrown] (pto Jun 2-Jun 5)]

Attachment: report when 'firefox' running during mochitests; fail if it is one of ours

Seems to do no harm:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9741b7aedce919707e2850c94a20951daa1e3889

Comment 4

[Joel Maher ( :jmaher ) (UTC -8) (PTO'ish until July 5)]

Comment on attachment 8927469 [details] [diff] [review]
report when 'firefox' running during mochitests; fail if it is one of ours

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

lots of good checking, just one thing to look out for here.

::: testing/mochitest/runtests.py
@@ +2179,4 @@
>                           'onTimeout': [timeoutHandler]}
>              kp_kwargs['processOutputLine'] = [outputHandler]
>  
> +            self.checkForRunningBrowsers()

will this conflict when running locally with ./mach or debugging on a loaner?

Comment 5

[Geoff Brown [:gbrown] (pto Jun 2-Jun 5)]

Comment on attachment 8927469 [details] [diff] [review]
report when 'firefox' running during mochitests; fail if it is one of ours

Oh darn, with more testing I hit several "Found old browser" exceptions and upon further examination found that the "old browser" was actually a new process using the recycled pid. I didn't think that was likely, but apparently there's a good chance on Windows 7. I'll need a new strategy.

https://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-2afc90db1d209ae21cbcbd911db39a492d9eb5b4/try-win32-stylo-disabled/try_win7_ix_stylo_disabled_test-mochitest-clipboard-e10s-bm111-tests1-windows-build371.txt.gz

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Should we better try to find out which tests do not clean-up correctly? As it looks like the media job seems to be a good candidate for that. 

I don't think that we can do more than reporting here, as you both already said. The other solution would only be to let Marionette use a free and not hard-coded port.

Comment 7

[Geoff Brown [:gbrown] (pto Jun 2-Jun 5)]

Attachment: just warn when firefox running before browser is started

This removes the error and just warns when 'firefox' is found running before mochitest starts a browser. I expect warnings when running locally, but I doubt anyone will be troubled by that. In automation, when we hit problems that might be related to extra browsers, we can check for this warning. I think that's the best we can do here.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cf0dc4097849
Warn if Firefox is running before mochitest starts Firefox; r=jmaher

Comment 9

[Dorel Luca [:dluca]]

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

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/c8042c122490