Closed
Bug 1083131
Opened 10 years ago
Closed 10 years ago
Marionette does not always remove temporary profiles
Categories
(Remote Protocol :: Marionette, defect)
Tracking
(firefox40 fixed)
RESOLVED
FIXED
mozilla40
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: ato, Assigned: chmanchester)
References
Details
(Keywords: pi-marionette-runner)
Attachments
(1 file, 1 obsolete file)
mozrunner produces temporary folders in /tmp that are not cleaned up. I haven't investigated if this happens for clean exits, interrupts, terminations, or all of these.
This morning the mozrunner related directories in /tmp had grown to ~790M.
Comment 1•10 years ago
|
||
Which type of tests were causing this behavior? What are those files? The profile data?
Reporter | ||
Comment 2•10 years ago
|
||
They look like directories containing the profile data as you say. They appear to be generated from running `./mach marionette-test`.
~ % lc -d /tmp/*mozrunner
tmp0ybIF_.mozrunner tmp8kX8cz.mozrunner tmpBqyKwK.mozrunner tmpNvnqZq.mozrunner tmpWs5pYO.mozrunner tmpbxBEq3.mozrunner tmpi1XoFm.mozrunner tmplGIFFP.mozrunner tmpqJpz9u.mozrunner
tmp11nnL8.mozrunner tmp9MWuWG.mozrunner tmpLVuEhI.mozrunner tmpS8EOr4.mozrunner tmpWve283.mozrunner tmpc248KF.mozrunner tmpieS1wE.mozrunner tmplcvvw8.mozrunner tmptK6WUk.mozrunner
tmp6N9XAy.mozrunner tmp9swc2l.mozrunner tmpMQgvsJ.mozrunner tmpTBa0SY.mozrunner tmpXigj1a.mozrunner tmpd9jcmO.mozrunner tmpj8RCk1.mozrunner tmpmRCyAI.mozrunner tmpusH4ed.mozrunner
tmp6_VBVs.mozrunner tmpA2WUoJ.mozrunner tmpMvXVZq.mozrunner tmpTol_Lm.mozrunner tmpXzLin0.mozrunner tmpda1XSi.mozrunner tmpl28Hg0.mozrunner tmpoA6BJ2.mozrunner tmpwiwgsr.mozrunner
tmp7LOBKV.mozrunner tmpBai08l.mozrunner tmpN50YK_.mozrunner tmpUxnK88.mozrunner tmpaIwKiJ.mozrunner tmpdsXPUB.mozrunner tmplBcNVR.mozrunner tmpooSArl.mozrunner tmpwrbgSf.mozrunner
tmp7SpHBt.mozrunner tmpBhSrAO.mozrunner tmpNtpstG.mozrunner tmpVkp9_G.mozrunner tmpbJizkh.mozrunner tmpgmGBOv.mozrunner tmplBcZdt.mozrunner tmppCR24L.mozrunner
~ % lc /tmp/tmp0ybIF_.mozrunner
.parentlock compatibility.ini extensions.ini minidumps prefs.js sessionstore-backups webappsstore.sqlite
blocklist.xml content-prefs.sqlite extensions.json permissions.sqlite safebrowsing startupCache webappsstore.sqlite-shm
bookmarkbackups cookies.sqlite key3.db places.sqlite search.json thumbnails webappsstore.sqlite-wal
cache2 crashes lock places.sqlite-shm secmod.db user.js
cert8.db directoryLinks.json mimeTypes.rdf places.sqlite-wal sessionCheckpoints.json webapps
Comment 3•10 years ago
|
||
Yeah, that seems to be the case. It would be good to know if that is always happening for you. You may also wanna run mozrunner yourself, and check afterward if the created profile is still present.
It may be that some code in Marionette doesn't let mozrunner remove the profile.
Summary: mozrunner does not clean up temporary folders → mozrunner does not always clean-up temporary profiles
Comment 4•10 years ago
|
||
I actually did this myself, and all is fine for the mozrunner command line. But when running Marionette client I can see that the newly created profile is still present in the temporary folder. So this is clearly a Marionette issue.
Component: Mozbase → Marionette
QA Contact: hskupin
Summary: mozrunner does not always clean-up temporary profiles → Marionette does not always remove temporary profiles
Updated•10 years ago
|
Keywords: ateam-marionette-runner
Comment 6•10 years ago
|
||
This is also a blocker of our upcoming Firefox ui tests. Adding dependency for bug 1080766.
Comment 7•10 years ago
|
||
Ok, I found the cause:
It appears that we are setting:
profile_args["restore"] = False
here:
https://dxr.mozilla.org/mozilla-central/source/testing/marionette/driver/marionette_driver/geckoinstance.py#72
And that cause the profile to not be deleted (I set it to True, and temp dirs are deleted). According to the doc:
:param restore: Flag for removing all custom settings during cleanup
So it seems *normal* to have this behaviour, since we set that to False. The question is why was this set to False ?
Comment 8•10 years ago
|
||
Hm, I think that this piece of code (in the context given in comment 7):
if hasattr(self, "profile_path") and self.profile is None:
if not self.profile_path:
profile_args["restore"] = False
Could be translated by something like "if the profile given as an argument was None (or at least not evaluated to True), then set restore=False". This is because "profile_path" is only defined if the profile argument was not a Profile instance, and in this case we set profile_path = profile (in our case, None).
Does that make sense ? Maybe we should just get rid of this 'profile_args["restore"] = False' line.
BTW, all the logic here with profile/profile_path/profile_args seems tricky and a bit broken. I'm not sure to understand what we wanted to do with this code.
Comment 9•10 years ago
|
||
Thank you Julien for investigating the problem! I kinda appreciate that. I'm also a bit new to Marionette, so I don't know the details about that code and the expected behavior.
David, can you give us some more information here how Marionette handles profiles?
Flags: needinfo?(dburns)
Comment 10•10 years ago
|
||
Marionette uses mozrunner and mozprofile to start the browser into a state where we can automate things.
see https://dxr.mozilla.org/mozilla-central/source/testing/marionette/driver/marionette_driver/geckoinstance.py#55. We merge them together on start()
Flags: needinfo?(dburns)
Comment 11•10 years ago
|
||
Ok, so given that you can pass in an existing profile via the --profile option, we should only remove the profile if it has really been created by Marionette. Or do we copy the existing profile to a temporarily location? Maybe that is what we have here?
https://dxr.mozilla.org/mozilla-central/source/testing/marionette/driver/marionette_driver/geckoinstance.py#76
So if we always create a new profile, we should indeed remove it at every time.
Assignee | ||
Comment 12•10 years ago
|
||
Removing the 'profile_args["restore"] = False' line quoted in comment 8 fixes this when I tried it. This appears to be the correct behavior -- this is the common case no profile was passed in and a fresh one was created. If we need a mode for "post mortem" debugging a profile created during the test run I suppose that could be added.
Assignee | ||
Comment 13•10 years ago
|
||
/r/7135 - Bug 1083131 - Always remove a profile created by marionette when the runner shuts down.;r=ato
Pull down this commit:
hg pull -r eef071a3398c65a2150febbc2b0e67e4a47e1271 https://reviewboard-hg.mozilla.org/gecko/
Attachment #8593518 -
Flags: review?(ato)
Assignee | ||
Comment 14•10 years ago
|
||
Anyhow, there's a patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=55ce1b1fb237
Reporter | ||
Comment 15•10 years ago
|
||
Comment on attachment 8593518 [details]
MozReview Request: bz://1083131/chmanchester
https://reviewboard.mozilla.org/r/7133/#review5933
Ship It!
Attachment #8593518 -
Flags: review?(ato) → review+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 17•10 years ago
|
||
(In reply to Chris Manchester [:chmanchester] from comment #16)
> Aha. Windows.
The only platform where removing the profile while the browser is running is considered a fatal error. This looks ok locally: https://treeherder.mozilla.org/#/jobs?repo=try&revision=93a35acfe73b
Assignee | ||
Updated•10 years ago
|
Attachment #8593518 -
Flags: review+ → review?(ato)
Assignee | ||
Comment 18•10 years ago
|
||
Comment on attachment 8593518 [details]
MozReview Request: bz://1083131/chmanchester
/r/7135 - Bug 1083131 - Always remove a profile created by marionette when the runner shuts down.;r=ato
Pull down this commit:
hg pull -r d63b9ac77254a08b73507c706e44545b5df095c4 https://reviewboard-hg.mozilla.org/gecko/
Assignee | ||
Comment 19•10 years ago
|
||
Ignore the review request if try in comment 17 isn't ok, but this fixes things on a windows build locally.
Reporter | ||
Comment 20•10 years ago
|
||
Comment on attachment 8593518 [details]
MozReview Request: bz://1083131/chmanchester
https://reviewboard.mozilla.org/r/7133/#review5959
Ship It!
Attachment #8593518 -
Flags: review?(ato) → review+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 21•10 years ago
|
||
Assignee: nobody → cmanchester
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 10 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Assignee | ||
Comment 24•9 years ago
|
||
Attachment #8593518 -
Attachment is obsolete: true
Attachment #8618406 -
Flags: review+
Assignee | ||
Comment 25•9 years ago
|
||
Updated•2 years ago
|
Product: Testing → Remote Protocol
You need to log in
before you can comment on or make changes to this bug.
Description
•