If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Jetpack tinderbox reports success for test runs that fail tests

RESOLVED FIXED

Status

Release Engineering
General
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: myk, Assigned: lsblakk)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

6 years ago
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.
(Assignee)

Comment 2

6 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

6 years ago
Assignee: nobody → lsblakk
(Assignee)

Comment 3

6 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

6 years ago
Created attachment 558862 [details] [diff] [review]
v.1 run_jetpack with error parsing for addonsdk-poller runs

screenshot of tinderbox when a test fails (local machine testing): http://cl.ly/3A2t2c3k122w311I1z1k
Attachment #558862 - Flags: feedback?(catlee)
(Assignee)

Comment 5

6 years ago
filed bug 679490 since we'll need to move this away from Tinderbox eventually.
(Reporter)

Comment 6

6 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

6 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.
(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+
Created attachment 559346 [details] [diff] [review]
v.2 run_jetpack with error parsing for addonsdk-poller runs, now exits with subprocess.Popen returncode

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

6 years ago
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+

Comment 12

6 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
(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."
Created attachment 561499 [details] [diff] [review]
v.3 run_jetpack with error parsing for addonsdk-poller runs, now without importing sut_lib

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

6 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+
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
Last Resolved: 6 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.