Closed Bug 1090931 Opened 5 years ago Closed 5 years ago

Reftest analyzer shows failures twice because of logcat

Categories

(Testing :: Reftest, defect)

x86
macOS
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla36

People

(Reporter: kats, Assigned: kats)

References

Details

Attachments

(1 file)

+++ 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.
Attached patch Update regexSplinter Review
Attached patch updates the regex to find "I/Gecko" anywhere in the line and fixes this issue.
Attachment #8513442 - Flags: review?(dbaron)
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.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) 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).
Status: NEW → ASSIGNED
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
Attachment #8513442 - Flags: review?(dbaron) → review+
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 :(
https://hg.mozilla.org/mozilla-central/rev/4cb91f2c872c
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.