Closed Bug 679490 Opened 14 years ago Closed 14 years ago

Jetpack tinderbox reports success for test runs that fail tests

Categories

(Release Engineering :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: myk, Assigned: lsblakk)

References

Details

Attachments

(1 file, 2 obsolete files)

The Jetpack tinderbox reports success (i.e. green) for test runs that fail tests. Here's an example: http://tbpl.mozilla.org/?tree=Jetpack&rev=22a8e6fa5ff1 From what I can tell, right now it unconditionally reports success, no matter what actually happens during each run. It should be reporting failure (i.e. red) for test runs with any failing tests.
Isn't this bug 669520? I don't get why lsblakk resolved that, since it didn't seem to need a same-rev example so much as needing the code running the tests on the Jetpack tree to do the same scraping for "Traceback" that the code running the tests on mozilla-central does.
(In reply to Phil Ringnalda (:philor) from comment #1) > Isn't this bug 669520? I don't get why lsblakk resolved that, since it > didn't seem to need a same-rev example so much as needing the code running > the tests on the Jetpack tree to do the same scraping for "Traceback" that > the code running the tests on mozilla-central does. You're right, I'm not sure where I got confused on that last bug. So back to setting up error parsing on the ScriptFactory that matches what is run on m-c test suite runs.
Assignee: nobody → lsblakk
Does http://pastebin.mozilla.org/1324293 look like it gets what you need? http://tinderbox.mozilla.org/showbuilds.cgi?tree=MozillaTest is super backlogged so in about an hour the results for that push should show up there too and I can paste the full log. This is just a run that I purposely messed with on my local machine so that it would have at least one test failure.
screenshot of tinderbox when a test fails (local machine testing): http://cl.ly/3A2t2c3k122w311I1z1k
Attachment #558862 - Flags: feedback?(catlee)
filed bug 679490 since we'll need to move this away from Tinderbox eventually.
(In reply to Lukas Blakk [:lsblakk] from comment #3) > Does http://pastebin.mozilla.org/1324293 look like it gets what you need? It looks right to me, but Brian is the expert. Brian: does that output look right to you? (In reply to Lukas Blakk [:lsblakk] from comment #4) > screenshot of tinderbox when a test fails (local machine testing): > http://cl.ly/3A2t2c3k122w311I1z1k Here's a live URL of that view: http://tinderbox.mozilla.org/showbuilds.cgi?tree=MozillaTest&maxdate=1315395117&hours=24&legend=0&norules=1 (In reply to Lukas Blakk [:lsblakk] from comment #5) > filed bug 679490 since we'll need to move this away from Tinderbox > eventually. Did you mean to cite a different bug number here? In any case, yes, the tbpl view is what we really care about: https://tbpl.mozilla.org/?tree=Jetpack
> Did you mean to cite a different bug number here? In any case, yes, the > tbpl view is what we really care about: > > https://tbpl.mozilla.org/?tree=Jetpack Why yes I did: bug 685299 The tbpl you link there *should* work, although slowly and with suffering due to the tinderbox mail backlog that we are in right now. I believe most of the tbpl pages are using usebuildbot=1 now for more swift reporting and that's where the bug I filed comes into play - we need to be able to link those logs in the build database with an ftp storage of the logs in order to move away from reliance on tinderbox for results & error parsing.
(In reply to Myk Melez [:myk] from comment #6) > (In reply to Lukas Blakk [:lsblakk] from comment #3) > > Does http://pastebin.mozilla.org/1324293 look like it gets what you need? > > It looks right to me, but Brian is the expert. That certainly looks like the output of a jetpack test process, and if I were writing some buildbot code to parse it, it's what I'd need to write that code. Did I answer the right question? :) But, can't we just have the buildstep look at the exit status of the test process and declare failure if-and-only-if it's non-zero? That'd be a lot more reliable and wouldn't misfire when e.g. we have a test named test-traceback.js or test-failure.js . Scanning for those messages to produce a condensed logfile sounds reasonable, but scanning them to determine success-vs-failure feels fragile to me.
Comment on attachment 558862 [details] [diff] [review] v.1 run_jetpack with error parsing for addonsdk-poller runs Review of attachment 558862 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. As discussed, let's grab the exit code from the child process and return that.
Attachment #558862 - Flags: feedback?(catlee) → feedback+
tested on my local machine with a snowleopard jetpack-poller builder and a 10.6 mozilla-central debug jetpack builder to make sure nothing broke for branch runs while adding this to addonsdk-poller triggered runs.
Attachment #558862 - Attachment is obsolete: true
Attachment #559346 - Flags: review?(catlee)
Attachment #559346 - Flags: review?(catlee) → review+
Comment on attachment 559346 [details] [diff] [review] v.2 run_jetpack with error parsing for addonsdk-poller runs, now exits with subprocess.Popen returncode landed: http://hg.mozilla.org/build/tools/rev/8015140cb437 This should go into effect immediately as the run_jetpack script is downloaded per run. Please let me know if any undesired behaviour occurs.
Attachment #559346 - Flags: checked-in+
It actually happened but the switch from tinderbox to buildbot based tbpl bit us. The jetpack jobs were not showing anymore. See bug 687021 to see what happened for win64. This has been failing since then with ImportError: No module named sut_lib [1] I have back the change out: http://hg.mozilla.org/build/tools/rev/0c0665c0fbb9 The backout has fixed the problem. [1] Log: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1316184472.1316184557.9239.gz&fulltext=1
(In reply to Armen Zambrano G. [:armenzg] - Release Engineer from comment #12) > It actually happened but the switch from tinderbox to buildbot based tbpl > bit us. I'm pretty sure that time's arrow points the other way. If it landed and took immediate effect on the afternoon of the 9th, and we switched to usebuildbot by default on the late evening of the 12th, then there were 3.5 days before "buildbot based tbpl bit us" during which "someone hid it on tinderbox based tbpl, and nobody cared that it was gone, bit us."
Test runs in branch test suites use a different naming scheme/location for the tools dir - in the addonsdk poller triggered test runs it's called 'scripts'. Instead of casing the chasing around of sut_lib I have just added the little runCommand that is needed to the run_jetpack.py script and ran tests in both m-c jetpack test as well as jetpack-snowleopard on my local setup. Both have passed (the reason this wasn't caught in my previous testing was that my m-c test run was actually pulling the build/tools and not my user repo tools - separate path settings).
Attachment #559346 - Attachment is obsolete: true
Attachment #561499 - Flags: review?(armenzg)
Comment on attachment 561499 [details] [diff] [review] v.3 run_jetpack with error parsing for addonsdk-poller runs, now without importing sut_lib Review of attachment 561499 [details] [diff] [review]: ----------------------------------------------------------------- r+ with the addition of the note. The interdiff was pretty clear between the two patches. ::: buildfarm/utils/run_jetpack.py @@ +13,5 @@ > SDK_DIR="jetpack" > > +log = logging.getLogger() > + > +def runCommand(cmd, env=None, logEcho=True): Can you please add a note to point to the code you are forking off this code?
Attachment #561499 - Flags: review?(armenzg) → review+
Comment on attachment 561499 [details] [diff] [review] v.3 run_jetpack with error parsing for addonsdk-poller runs, now without importing sut_lib I added a note in both run_jetpack.py and sut_lib.py referencing the copying of runCommand. Landed: http://hg.mozilla.org/build/tools/rev/edca76df663e
Attachment #561499 - Flags: checked-in+
This is live and seems to be working as desired now.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: