394250 - add ep_unittest.pl to handle unit test output

Status: RESOLVED FIXED

(Webtools Graveyard :: Tinderbox, defect)

Description

[(not currently active) Ted Mielczarek]

The unit test boxes currently spit out a lot of expected errors, which get caught by tinderbox's log scraping.  They look like this:
*** 11532 INFO TODO | Text threw exception: TypeError: attr has no properties | 

ep_unix.pl has, as one of its regexes, /Error: /, which is catching these lines.  That regex used to be / Error: /, it was changed in rev 1.5 without much description as to why.  It looks like this regex doesn't even catch most of our compile errors nowadays, the gmake regex above is all we seem to trigger, since gcc's error messages contain "error:" (with a lowercase e), for example:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1188338460.1261.gz

I think we should consider changing this back to get rid of these false positives.  We would miss cases like this, however, but maybe we can change that to fit:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1188375660.1188378805.4780.gz

Comment 1

[cls]

The comment changed along with the regex.  It added 'or smoketest' so I'm taking it that the leading space was preventing smoketest errors from being caught.  That will definitely be the case for us locally if you change that back.  I'd rather catch a few false positives than miss errors that should be caught.  

It's going to hard to add a regex that will catch generic errors but ignore the js exceptions.  I'm not convinced that we even should make that change since the desire to ignore those errors are limited to a specific set of tests which is not necessarily the general case.  It may be difficult to do securely, but a better alternative would be to add a way to add warning regexes via the admin page so that different trees can catch different errors.

Comment 2

[(not currently active) Ted Mielczarek]

Ok, I can buy that.  What would you think about adding a new error parser specifically for these unit test boxes?  If we added an ep_unittest.pl, we could modify the regexes to not catch these false positives, and additionally we could add regexes to catch the existing failure output of the various unit tests.  You're probably right that some sort of "per-tree error filter" that was configurable via the admin interface would be nice, but that sounds like a whole lot of work for what we really need.

Comment 3

[Jeff Walden [:Waldo]]

It's not like it's hard to add some error string to test output; we could easily prefix with "Error: " or something without much trouble, so long as we know what the string is and don't need to change it frequently or anything.  Pick something and I'm sure we can deal.

Comment 4

[(not currently active) Ted Mielczarek]

Right, that's easy, and I figured we'd handle that in bug 379327.  I wanted to deal with the false positives specifically in here.

Comment 5

[cls]

In lieu of a per tree error filter, I think a new error parser for unit tests is fine.

Comment 6

[(not currently active) Ted Mielczarek]

Attachment: first cut at ep_unittest.pl

This adds error handling for mochitest, reftest, and browser chrome failure output.  TUnit is kinda hard, we may need to look at the output.  Tihs also linkifies the testcase to bonsai for reftests, since the test is listed on the FAIL line.  We can probably modify MochiTest/browser chrome tests to output the test file name on a FAIL line, and get the same nice behavior.  I've tested this manually by pasting log input into a test script, but I'd like to get this installed on tinderbox-stage so I could throw some real logs at it.

Comment 7

[(not currently active) Ted Mielczarek]

Attachment: updated to link to test filenames

Ok, updated ep_unittest.pl, using the patches from bug 379327 to link to the failed test file for mochitest and browser chrome tests.  Still need to figure out xpcshell tests.

Comment 8

[(not currently active) Ted Mielczarek]

justdave said he could apply this locally to tinderbox-stage.

Comment 9

[(not currently active) Ted Mielczarek]

This is an old log, so it doesn't have my patches from bug 379327, but the error output looks a lot better anyway:
http://tinderbox-stage.mozilla.org/showlog.cgi?tree=Firefox&logfile=1188675840.1188677653.320.gz&buildname=Linux%20qm-centos5-01%20dep%20unit%20test&buildtime=1188675840&errorparser=unittest

I'm going to setup my own unit test buildbot to test the buildbot patch on the other bug, and to test some error output.

Comment 10

[(not currently active) Ted Mielczarek]

Log from a run I just did:
http://tinderbox-stage.mozilla.org/showlog.cgi?log=MozillaTest/1188960032.1189002561.26869.gz

Looks like I need to handle linkifying mochichrome tests, but otherwise it looks pretty good!

Comment 11

[(not currently active) Ted Mielczarek]

Attachment: add ep_unittest.pl

Ok, I'm happy with this for the time being, at least until I get bug 394875 fixed.

Comment 12

[cls]

Comment on attachment 279811 [details] [diff] [review]
add ep_unittest.pl

I forgot to mention that you need the corresponding Makefile change to make sure that the new parser is actually installed.

Comment 13

[(not currently active) Ted Mielczarek]

Checked in with the Makefile change.  Thanks!

This still might need a tweak to handle the xpcshell-based unit tests properly, but that shouldn't be a big deal.

Comment 14

[(not currently active) Ted Mielczarek]

Attachment: add xpcshell unit test support

Should have added this in the first pass.  This adds support for the xpcshell unit test harness.  The source file linking is not really correct yet, I need to fix bug 394875 to get that, but it might take me a bit to get there, and I'd rather just get this in first.

Comment 15

[cls]

Comment on attachment 280383 [details] [diff] [review]
add xpcshell unit test support

Since it's already checked in, can we just get a patch against the existing version?

Comment 16

[(not currently active) Ted Mielczarek]

Attachment: add xpcshell unit test support

Whoops, that was actually the old patch.  Sorry.

Comment 17

[(not currently active) Ted Mielczarek]

Comment on attachment 280411 [details] [diff] [review]
add xpcshell unit test support

Checked in.