567274 - Talos should halt on download or unzip failure

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P3)

Description

[Phil Ringnalda (:philor)]

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1274399232.1274399865.15943.gz
Rev3 WINNT 6.1 mozilla-central talos on 2010/05/20 16:47:12  

  inflating: firefox/xul.dll          bad CRC ff3b34a5  (should be f3a57fd9)
program finished with exit code 2
...
Running test tdhtml: 
NOISE: __FAILbrowser non-zero return code (-1073741515)__FAIL

While http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1274399212.1274399358.13547.gz is an xpcshell test that got the same busted zip, it sensibly gave up when unzipping failed, rather than trying to run some partial, broken browser. Since there's no good that can come from running part of a browser, only red or worse (if it's possible to unzip only part of the browser, but then run it, the perf numbers from that would be... unreliable), it seems like it should bail when it doesn't unzip.

Comment 1

[John Ford [:jhford] CET/CEST Berlin Time]

Attachment: untested patch

This will force the step to halt the build if there is an error unpacking the file.  The build will still run the reboot step is it has alwaysRun=True.

Comment 2

[John Ford [:jhford] CET/CEST Berlin Time]

If we were to fix bug 557336 we could use the same base class for the setup and tear down of unit tests and talos runs.  This seems like a good thing as both are running in the same pool of slaves and avoids code duplication.

Comment 3

[[e-mail] (:bhearsum)]

We should probably add haltOnFailure for most, if not all, of the DownloadFile and UnpackFile steps in TalosFactory and RuntimeTalosFactory. The only exception, I think, is for the download/unpack symbols. Because these aren't a _crucial_ part of the test process, I think it'd be better to continue on even if we fail to download or unpack them. We need to go through all the DownloadFile/UnpackFile steps starting from http://hg.mozilla.org/build/buildbotcustom/file/default/process/factory.py#l7004, ending at http://hg.mozilla.org/build/buildbotcustom/file/default/process/factory.py#l7723, and see which ones need this change applied.

Comment 4

[[e-mail] (:bhearsum)]

Same goes for the UnittestPackagedBuildFactory: http://hg.mozilla.org/build/buildbotcustom/file/default/process/factory.py#l6576

Comment 5

[Erick Dransch [:edransch]]

Attachment: add haltOnFailure=True to Talos Factory

added haltOnFailure to UnpackFile and DownloadFile in Talos Factory (except for Download/Unpacks of symbols, as mentioned above).

Comment 6

[Erick Dransch [:edransch]]

Attachment: Revised patch

Comment 7

[Erick Dransch [:edransch]]

Comment on attachment 586487 [details] [diff] [review]
Revised patch

Revised patch to correctly handle post-failure reboot.

Comment 8

[[e-mail] (:bhearsum)]

This made it to production yesterday. Yay!