785273 - Marionette doesn't always cleanup emulator instances after tests are done

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Jonathan Griffin (:jgriffin)]

The work to add xUnit output to Marionette caused us to start persisting test results in self.results.  These result objects include references to the test classes being run, which in turn contain references to the Marionette instance used to run the test.

This produces a circular reference such that the garbage collector never calls Marionette's __del__ method, which means it doesn't clean up properly after itself.  This is primarily noticeable when running mult-emulator tests; secondary emulators are currently not shut down after the end of tests.  The primary emulator is shut down, thanks to code in run_tests which was added explicitly to do this.

However, it's probably better to prevent the circular reference from occurring, and let Marionette clean up after itself the correct way.  We can do this by preventing the MarionetteTestCase instances from holding an active reference to Marionette except when the test is actually running.

Comment 1

[Jonathan Griffin (:jgriffin)]

Attachment: Prevent circular references to Marionette instances,

Comment 2

[Malini Das [:mdas] - Away, not checking bugmail]

Comment on attachment 654869 [details] [diff] [review]
Prevent circular references to Marionette instances,

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

lgtm, good catch!

Comment 3

[Jonathan Griffin (:jgriffin)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3c0b16397ce9

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3c0b16397ce9

Comment 5

[Jonathan Griffin (:jgriffin)]

Whoops, I broke Marionette JS tests with this patch.  Landed a follow-up fix as https://hg.mozilla.org/mozilla-central/rev/e934a9d8be1f.