Closed
Bug 734320
Opened 13 years ago
Closed 13 years ago
Cut down on false positives in Jetpack Tinderbox log parsing by using the unittest parser
Categories
(Release Engineering :: General, defect)
Release Engineering
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: philor, Assigned: philor)
Details
Attachments
(1 file)
778 bytes,
patch
|
catlee
:
review+
philor
:
checked-in+
|
Details | Diff | Splinter Review |
There's not a lot of short-range hope for Jetpack, because their test failures look like "error: TEST FAILED: testname (failure)" and "error: fail: error message" for which we have no Tinderbox errorparser (and creating one means Perl, and CVS, and Tinderbox, and nobody's going to be that crazy all at once).
Medium-range, bug 713846 will give them logs uploaded to ftp.m.o instead of mailed to Tinderbox, at which point it's just a tbpl bug to add an errorparser, since that bit of tbpl was designed exactly to let people say "no, I like having my log mark errors with a phrase just ever so slightly different than the phrase in every other log."
But there is one short-range thing we can do: instead of using the ep_unix.pl errorparser, which was the default because it was the default for builds back when builds were the only thing that we actually did, and tests were just 10 things run by make check, we can switch to the ep_unittest.pl errorparser. While it won't actually see "TEST FAILED" because it wants to see "TEST-UNEXPECTED-FAIL," it also won't see every single message that was posted to the error console, because it doesn't have the /Error: / which picks up every "console: [JavaScript Error:"
http://tinderbox.mozilla.org/showlog.cgi?log=Jetpack/1331235243.1331235983.4284.gz is the brief log for a Jetpack run with one test failure. It does not actually include the test failure thanks to trimming out that part of the log, so it's just some junk from the console in other tests.
http://tinderbox.mozilla.org/showlog.cgi?tree=Jetpack&errorparser=unittest&logfile=1331235243.1331235983.4284.gz is the same log with the unittest errorparser, not summarizing any error, real or fake, but actually including the error in the log. Both parts of that, not summarizing fake errors, and actually including the error in the log, strike me as improvements.
Attachment #604304 -
Flags: review?(catlee)
Attachment #604304 -
Flags: feedback?(poirot.alex)
Comment 1•13 years ago
|
||
Hum the test is still reported as a failing one, right?
So instead of having many useless messages in error summary, we now have nothing.
I think it will still provide a WTF effect for non-jetpackers.
"I broke jetpack, the test is orange, why?"
I'm wondering if we should change our unit test output in order to match m-c conventions?
So that:
error: TEST FAILED: test-functional.test delay (failure)
Becomes:
error: TEST-UNEXPECTED-FAIL, test-functional.test delay (failure)
(or whatever is needed)
Assignee | ||
Comment 2•13 years ago
|
||
That would be exactly what I would do, though suggestions along those lines ("why don't you just change everything about how you work so it matches what we do?") have been rejected before.
But in this particular case, I'm not looking at changing things non-jetpackers will see: this only affects Tinderbox, which is only used on the Jetpack tree itself. When you load https://tbpl.mozilla.org/?tree=Jetpack&usetinderbox=1 I want to change two things: right now, clicking on an orange letter gives you a popup "summary" of things unrelated to the failure which I want to change to "Summary is empty" (I'd be perfectly happy if you change to TEST-UNEXPECTED-FAIL and fill the summary up with that, but having that work also requires this change), and, when you click on the "View brief log" link, I want to make it open a log which actually includes the test failure, rather than the current brief log which is trimmed down to only include the bits of log around things which are not the failure.
Updated•13 years ago
|
Attachment #604304 -
Flags: review?(catlee) → review+
Assignee | ||
Comment 3•13 years ago
|
||
Comment on attachment 604304 [details] [diff] [review]
buildbotcustom
But, either way this is the necessary first step.
http://hg.mozilla.org/build/buildbotcustom/rev/dfac39b33a7c
Attachment #604304 -
Flags: feedback?(poirot.alex) → checked-in+
Comment 4•13 years ago
|
||
deployed as part of this morning's reconfig
Assignee | ||
Comment 5•13 years ago
|
||
And fixed, though it might be my second shortest fix ever (after the one that only lasted one Firefox nightly), since you also fixed log uploading so there's no reason for them to use Tinderbox anymore and we should shut off Tinderbox mail for this branch :)
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Updated•12 years ago
|
Product: mozilla.org → Release Engineering
Updated•7 years ago
|
Component: General Automation → General
You need to log in
before you can comment on or make changes to this bug.
Description
•