1090931 - Reftest analyzer shows failures twice because of logcat

Status: RESOLVED FIXED

(Testing :: Reftest, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

+++ This bug was initially created as a clone of Bug #1016251 +++

Using the try push at https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=46e40619ef4e as an example. Click on the first failed R14, and open the reftest analyzer. The http://10.0.2.2:8888/tests/layout/reftests/position-dynamic-changes/horizontal/leftA-widthN-rightA.html?margin_abspos test is listed twice.

This happens because the logcat output duplicates some of the reftest output. I had previously fixed this in bug 1016251 by skipping over "logcat" lines in the reftest analyzer, but the regex I used then doesn't work now because the output format appears to have changed. Specifically, the "I/Gecko" doesn't appear at the start of the line any more, it appears after some timestamp stuff.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Attachment: Update regex

Attached patch updates the regex to find "I/Gecko" anywhere in the line and fixes this issue.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Actually this might be specific to treeherder since using tbpl to view the same reftest failure works fine. Not sure why, since the log excerpt on tbpl also has some timestamp junk at the start of the line.

Comment 3

[Ed Morley [:emorley]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #2)
> Actually this might be specific to treeherder since using tbpl to view the
> same reftest failure works fine. Not sure why, since the log excerpt on tbpl
> also has some timestamp junk at the start of the line.

Treeherder invokes the reftest analyser with less hackery than TBPL, by passing the log URL directly to the reftest analyser. The reftest analyser therefore gets the full log contents as opposed to the parsed excerpt from TBPL - which isn't ideal (due to increased page load time), but was seen as the cleanest short-to-medium-term solution in bug 1059331.

The longer term plan is to replace the in-repo reftest analyser completely (bug 1063640).

Comment 4

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 8513442 [details] [diff] [review]
Update regex

I'd be a little more worried about this showing up in a filename.  (data URLs can't have / in them, can they?)

Maybe do a match for I/Gecko that precedes REFTEST (i.e., /I\/Gecko.*?REFTEST/)?

r=dbaron with that

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Good idea; updated the regex as requested and verified it still works.

https://hg.mozilla.org/integration/mozilla-inbound/rev/4cb91f2c872c

I realized I should have put a DONTBUILD on that right after I hit enter... but ctrl-c'ing pushes is even worse :(

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4cb91f2c872c