Closed
Bug 813577
Opened 8 years ago
Closed 8 years ago
Use try/except/finally in remoteautomation.py and b2gautomation.py to ensure temp directories do not linger
Categories
(Testing :: General, defect)
Testing
General
Tracking
(firefox20 fixed)
RESOLVED
FIXED
mozilla21
Tracking | Status | |
---|---|---|
firefox20 | --- | fixed |
People
(Reporter: emorley, Assigned: emorley)
References
Details
Attachments
(1 file)
6.46 KB,
patch
|
gps
:
review+
|
Details | Diff | Splinter Review |
Broken out from bug 808410. (In reply to Gregory Szorc [:gps] from comment #21) > Comment on attachment 681926 [details] [diff] [review] > Part 5: Callers of checkForCrashes should use its return value > > ::: build/mobile/b2gautomation.py > @@ +103,5 @@ > > try: > > shutil.rmtree(dumpDir) > > except: > > print "WARNING: unable to remove directory: %s" % (dumpDir) > > + return crashed > > See comment in remoteautomation.py regarding lingering temp directories and > try..finally. > ... > ::: build/mobile/remoteautomation.py > @@ +100,5 @@ > > + try: > > + shutil.rmtree(dumpDir) > > + except: > > + print "WARNING: unable to remove directory: %s" % dumpDir > > + return crashed > > If getDirectory() or checkForCrashes() raises, the temporary directory will > linger. I'd use a try..finally to ensure dumpDir is deleted. (In reply to Joel Maher (:jmaher) from comment #27) > ::: build/mobile/remoteautomation.py > @@ +100,5 @@ > > + try: > > + shutil.rmtree(dumpDir) > > + except: > > + print "WARNING: unable to remove directory: %s" % dumpDir > > + return crashed > > to that note, we should have a try/except/finall around the entire > getDirectory/checkForCrashes code. Right now we only have it around > shutil.rmtree.
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → bmo
Assignee | ||
Updated•8 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•8 years ago
|
||
This is what you had in mind? :-)
Attachment #693170 -
Flags: review?(gps)
Comment 2•8 years ago
|
||
Comment on attachment 693170 [details] [diff] [review] Patch v1 Review of attachment 693170 [details] [diff] [review]: ----------------------------------------------------------------- This looks good to me. Sorry it disappeared in my review queue for over a month. ::: build/automationutils.py @@ +147,5 @@ > return False > > + try: > + removeSymbolsPath = False > + Nit: leading whitespace
Attachment #693170 -
Flags: review?(gps) → review+
Assignee | ||
Comment 3•8 years ago
|
||
Fixed nit: https://hg.mozilla.org/integration/mozilla-inbound/rev/3067bf264c5f Will need to port the automationutils.py change to mozcrash too.
Comment 4•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/3067bf264c5f
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Assignee | ||
Comment 5•8 years ago
|
||
https://hg.mozilla.org/releases/mozilla-beta/rev/654850067995
status-firefox20:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•