Closed Bug 729361 Opened 13 years ago Closed 13 years ago

buildRemoteManifest silently succeeds on failure; it should fail

Categories

(Testing :: Talos, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: k0scist, Unassigned)

Details

Attachments

(2 files)

If buildRemoteManifest is called with a file that doesn't exist or other baddities, we silently return the name of the origin manifest: http://hg.mozilla.org/build/talos/file/caf24bd3f566/talos/PerfConfigurator.py#l325 Instead, we should probably fail. Also, we use `manifestData = fHandle.read() ... for line in manifestData.split('\n'):` vs manifestLines = fHandle.readlines(): ... for line in lines
Attached patch err out harderSplinter Review
Attachment #599417 - Flags: review?(jmaher)
Comment on attachment 599417 [details] [diff] [review] err out harder Review of attachment 599417 [details] [diff] [review]: ----------------------------------------------------------------- r=me with a better message for raise. ::: talos/PerfConfigurator.py @@ +324,5 @@ > fHandle.close() > except: > if fHandle: > fHandle.close() > + raise raise what, your hands? Some kind of message is always nice.
Attachment #599417 - Flags: review?(jmaher) → review+
Whiteboard: [talos-checkin-needed]
Attached patch add a commentSplinter Review
Not sure what level of verbosity you want here. Re-raising the current exception will print the error to screen, such as IOError: [Errno 2] No such file or directory: 'dfjasdfk' for a missing file
Attachment #606623 - Flags: review?(jmaher)
Comment on attachment 606623 [details] [diff] [review] add a comment Review of attachment 606623 [details] [diff] [review]: ----------------------------------------------------------------- well, there is no perfect method for error handling, so this will do.
Attachment #606623 - Flags: review?(jmaher) → review+
pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=33e5d14f7a1f tested on android locally with ts and tsvg; works fine
looks pretty green, pushing
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Try run for 33e5d14f7a1f is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=33e5d14f7a1f Results (out of 75 total builds): success: 68 failure: 7 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jhammel@mozilla.com-33e5d14f7a1f
Whiteboard: [talos-checkin-needed]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: