Closed
Bug 1174766
Opened 6 years ago
Closed 6 years ago
Prevent loss of gecko.log when restarting gecko process with marionette
Categories
(Testing :: Marionette, defect)
Testing
Marionette
Tracking
(firefox41 fixed)
RESOLVED
FIXED
mozilla41
Tracking | Status | |
---|---|---|
firefox41 | --- | fixed |
People
(Reporter: chmanchester, Assigned: chmanchester)
Details
Attachments
(3 files)
When debugging a problem with marionette, the first thing I usually do is push to try and retrigger a lot with lots of logging enabled. To figure out what's going on in the server, there's really not much to go on without the log from the gecko process. We dump 'gecko.log' at the end of each run, but this is overwritten after each restart, so that log rarely contains the relevant info. I have a quick patch to make a fresh log for after each restart and upload them to blobber. It's a lot to upload for every single run, but I think it's worth it (a quick conversation on irc indicates I wouldn't be the only one to find this useful).
Why not appending to the same file after a restart? We would only have to open the file with a different mode. Using different files would be different from our internal logging.
Assignee | ||
Comment 2•6 years ago
|
||
We end up with 12 logs on a Mn run, which is a lot to sift through. I'll see about concatenating.
Assignee | ||
Comment 3•6 years ago
|
||
Bug 1174766 - Append to gecko logfile after marionette restarts the browser rather than removing the old file every time.
Attachment #8622684 -
Flags: review?(jgriffin)
Attachment #8622684 -
Flags: review?(ato)
Assignee | ||
Comment 4•6 years ago
|
||
Bug 1174766 - Specify gecko.log for upload in Mn tests.
Attachment #8622685 -
Flags: review?(jgriffin)
Attachment #8622685 -
Flags: review?(ato)
Assignee | ||
Updated•6 years ago
|
Summary: Rotate gecko logs after a restart when a directory is passed to --gecko-log → Prevent loss of gecko.log when restarting gecko process with marionette
Assignee | ||
Comment 5•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e2845da3a9e5
Comment 6•6 years ago
|
||
For data integrity reasons I think rotating the file would be better, but maybe we can consider doing that when the Mn job has been split up (see bug 1174491, especially comments 3 and 4). Not loosing data is already better than status quo so I’ll have a look at this review later today.
Assignee | ||
Comment 7•6 years ago
|
||
Yeah, a monolithic gecko.log is decent for debugging because I can just grep through for the filename of the test I care about, and was only a little harder to get working.
Comment 8•6 years ago
|
||
Comment on attachment 8622684 [details] MozReview Request: Bug 1174766 - Append to gecko logfile after marionette restarts the browser rather than removing the old file every time.;r=ato https://reviewboard.mozilla.org/r/11427/#review9991 Ship It!
Attachment #8622684 -
Flags: review?(ato) → review+
Comment 9•6 years ago
|
||
Comment on attachment 8622685 [details] MozReview Request: Bug 1174766 - Specify gecko.log for upload in Mn tests. https://reviewboard.mozilla.org/r/11431/#review9995 Ship It!
Attachment #8622685 -
Flags: review?(ato) → review+
Assignee | ||
Updated•6 years ago
|
Attachment #8622684 -
Flags: review?(jgriffin)
Assignee | ||
Updated•6 years ago
|
Attachment #8622685 -
Flags: review?(jgriffin)
Assignee | ||
Comment 10•6 years ago
|
||
https://hg.mozilla.org/build/mozharness/rev/d3632a67fe12
Assignee | ||
Updated•6 years ago
|
Keywords: checkin-needed
Comment 11•6 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/101932a675a4
Keywords: checkin-needed
Assignee | ||
Comment 12•6 years ago
|
||
Backed out for winXP failures in https://hg.mozilla.org/integration/mozilla-inbound/rev/8a25cd69c4ae
Assignee | ||
Comment 13•6 years ago
|
||
(In reply to Chris Manchester [:chmanchester] from comment #12) > Backed out for winXP failures in > https://hg.mozilla.org/integration/mozilla-inbound/rev/8a25cd69c4ae A prior test using the mozrunner initiated restart must be interfering with the profile, because we have: "[GFX1-]: Failed to create the graphics startup lockfile.", which is just an attempt to do something in the profile directory. This is sort of awful. The relevant difference with this patch must be that we aren't hitting the retry loop for deleting the old log file, which was apparently providing critical time between tests. I'm testing a patch to just use a new profile for this test that I can hopefully land, but this is a potentially significant hazard.
Assignee | ||
Comment 14•6 years ago
|
||
Actually we're having trouble accessing the profile when restarting the browser with mozrunner _after_ restarting from within Firefox. This makes a bit more sense and is bit less disconcerting.
Assignee | ||
Comment 15•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=21927f4aaeeb
Assignee | ||
Updated•6 years ago
|
Attachment #8622684 -
Attachment description: MozReview Request: Bug 1174766 - Append to gecko logfile after marionette restarts the browser rather than removing the old file every time. → MozReview Request: Bug 1174766 - Append to gecko logfile after marionette restarts the browser rather than removing the old file every time.;r=ato
Attachment #8622684 -
Flags: review+ → review?(ato)
Assignee | ||
Comment 16•6 years ago
|
||
Comment on attachment 8622684 [details] MozReview Request: Bug 1174766 - Append to gecko logfile after marionette restarts the browser rather than removing the old file every time.;r=ato Bug 1174766 - Append to gecko logfile after marionette restarts the browser rather than removing the old file every time.;r=ato
Assignee | ||
Comment 17•6 years ago
|
||
Bug 1174766 - Modify test_profile_management to use a clean profile to avoid contention on windows.;r=ato
Attachment #8625195 -
Flags: review?(ato)
Comment 18•6 years ago
|
||
Comment on attachment 8625195 [details] MozReview Request: Bug 1174766 - Modify test_profile_management to use a clean profile to avoid contention on windows.;r=ato https://reviewboard.mozilla.org/r/11807/#review10263 Ship It!
Attachment #8625195 -
Flags: review?(ato) → review+
Comment 19•6 years ago
|
||
Comment on attachment 8622684 [details] MozReview Request: Bug 1174766 - Append to gecko logfile after marionette restarts the browser rather than removing the old file every time.;r=ato https://reviewboard.mozilla.org/r/11427/#review10371 ::: testing/marionette/driver/marionette_driver/geckoinstance.py:63 (Diff revision 2) > + os.remove(self.gecko_log) This will raise if `self.gecko_log` is a directory. Is this what we want? ::: testing/marionette/driver/marionette_driver/geckoinstance.py:61 (Diff revision 2) > + gecko_log = os.path.realpath(gecko_log) To not mangle the user input I feel `os.path.normpath` is more appropriate here.
Attachment #8622684 -
Flags: review?(ato)
Comment 20•6 years ago
|
||
https://reviewboard.mozilla.org/r/11807/#review10373 Ship It!
Assignee | ||
Comment 21•6 years ago
|
||
https://reviewboard.mozilla.org/r/11427/#review10385 ::: testing/marionette/driver/marionette_driver/geckoinstance.py:63 (Diff revision 2) > + os.remove(self.gecko_log) This should be "gecko_log", it's never a directory because we convert it in the isdir branch above. ::: testing/marionette/driver/marionette_driver/geckoinstance.py:61 (Diff revision 2) > + gecko_log = os.path.realpath(gecko_log) This is a direct port of the old code.
Comment 22•6 years ago
|
||
Comment on attachment 8622684 [details] MozReview Request: Bug 1174766 - Append to gecko logfile after marionette restarts the browser rather than removing the old file every time.;r=ato https://reviewboard.mozilla.org/r/11427/#review10387 That’s good enough for me!
Attachment #8622684 -
Flags: review+
Comment 23•6 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/d2d535a3fdbf https://hg.mozilla.org/integration/mozilla-inbound/rev/585f52ceab5c
Comment 24•6 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d2d535a3fdbf https://hg.mozilla.org/mozilla-central/rev/585f52ceab5c
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in
before you can comment on or make changes to this bug.
Description
•