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)
Release Engineering
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: myk, Assigned: lsblakk)
References
Details
Attachments
(1 file, 2 obsolete files)
3.02 KB,
patch
|
armenzg
:
review+
lsblakk
:
checked-in+
|
Details | Diff | Splinter Review |
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.
Comment 1•14 years ago
|
||
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.
Assignee | ||
Comment 2•14 years ago
|
||
(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 | ||
Updated•14 years ago
|
Assignee: nobody → lsblakk
Assignee | ||
Comment 3•14 years ago
|
||
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.
Assignee | ||
Comment 4•14 years ago
|
||
screenshot of tinderbox when a test fails (local machine testing): http://cl.ly/3A2t2c3k122w311I1z1k
Attachment #558862 -
Flags: feedback?(catlee)
Assignee | ||
Comment 5•14 years ago
|
||
filed bug 679490 since we'll need to move this away from Tinderbox eventually.
Reporter | ||
Comment 6•14 years ago
|
||
(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
Assignee | ||
Comment 7•14 years ago
|
||
> 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.
Comment 8•14 years ago
|
||
(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 9•14 years ago
|
||
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+
Assignee | ||
Comment 10•14 years ago
|
||
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)
Updated•14 years ago
|
Attachment #559346 -
Flags: review?(catlee) → review+
Assignee | ||
Comment 11•14 years ago
|
||
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+
Comment 12•14 years ago
|
||
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
Comment 13•14 years ago
|
||
(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."
Assignee | ||
Comment 14•14 years ago
|
||
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 15•14 years ago
|
||
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+
Assignee | ||
Comment 16•14 years ago
|
||
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+
Assignee | ||
Comment 17•14 years ago
|
||
This is live and seems to be working as desired now.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•12 years ago
|
Product: mozilla.org → Release Engineering
You need to log in
before you can comment on or make changes to this bug.
Description
•