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

RESOLVED FIXED in mozilla17

Status

defect
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: jgriffin, Assigned: jgriffin)

Tracking

unspecified
mozilla17
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Assignee

Description

7 years ago
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 on attachment 654869 [details] [diff] [review]
Prevent circular references to Marionette instances,

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

lgtm, good catch!
Attachment #654869 - Flags: review?(mdas) → review+
Assignee

Comment 3

7 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/3c0b16397ce9
Assignee: nobody → jgriffin
Target Milestone: --- → mozilla17
https://hg.mozilla.org/mozilla-central/rev/3c0b16397ce9
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Assignee

Comment 5

7 years ago
Whoops, I broke Marionette JS tests with this patch.  Landed a follow-up fix as https://hg.mozilla.org/mozilla-central/rev/e934a9d8be1f.
You need to log in before you can comment on or make changes to this bug.