882670 - AnnotatedSummaryGenerator should processLine() harder, better, faster, stronger

Status: RESOLVED FIXED

(Tree Management Graveyard :: TBPL, defect)

Description

[Ed Morley [:emorley]]

AnnotatedSummaryGenerator::processLine() currently doesn't handle a number of edge cases very well. In addition it could do with a fair amount of cleanup.

Rough overview of current algorithm:
* Check for the pipe symbol delimited format ("foo | bar | baz")
  ** If not delimited, fall back to searching for the whole line.
  ** If it is delimited, check if the test/step name is 'leakcheck'
     ** If so, and the format is valid, search for the objects listed.
     ** If so, but the format is not valid [1], then fall back to searching for the entire line.
     ** If not 'leakcheck', search for the test/step name (minus path).
* If no bugs were returned by the search, then check for a crash top frame, and if found, search again using that string.

Note when we call out to do the search, we give up if the search string is less than 5 characters long or if it's on a blacklist of terms known to not produce useful search results.

This causes problems, since for the cases that match the blacklist (eg step = 'Shutdown' or 'automation.py') we don't then fall back to searching for the entire line (which should be specific enough that we don't need to blacklist). This means we never suggest failures such as bug 630780, bug 820756, bug 848924, bug 667435, bug 797242 etc.

Proposed algorithm:
* Check for the pipe symbol delimited format ("foo | bar | baz")
  ** If it is delimited, check if a _valid_ leakcheck format is present.
     ** If so, search for the objects listed.
     ** If not, search for the test/step name (minus path).
* If no search term has been set so far, or if the chosen search term matches the blacklist or is less than 5 characters long - then set the search term to be the whole failure line.
* Run the search
* If no bugs were returned by the search, then check for a crash top frame, and if found, search again using that string.


[1] eg: http://hg.mozilla.org/mozilla-central/file/b197bed90a98/build/automationutils.py#l262

Comment 1

[Ed Morley [:emorley]]

Attachment: Part 1: Comment & variable/method name cleanup;

Comment 2

[Ed Morley [:emorley]]

Attachment: Part 2: Invert conditional

Swap the if and the else statements to make the diff clearer in part 6.

Comment 3

[Ed Morley [:emorley]]

Attachment: Part 3: Don't special-case leakcheck errors

leakcheck failures can either be of the form:
"TEST-UNEXPECTED-FAIL | leakcheck |.* leaked \d+ bytes (Object-1, Object-2, Object-3, ...)"
...or if an error occurred during processing:
"TEST-UNEXPECTED-FAIL | leakcheck |.* negative leaks caught!"

We currently special-case the latter - falling back to searching for the full
line (searching for 'leakcheck' would return too many results). However, with
part 6 we'll automatically fall back to searching for the whole line more
generically, so this special-casing is no longer required.

With the special-casing removed, we need to add 'leakcheck' to the blacklist
to prevent false positives and ensure the generic fallback in part 6 kicks in.
In addition, the 'automationutils.processLeakLog()' entry can be removed since
bug 850681 (where it was renamed to leakcheck) has landed on all active trees.

Comment 4

[Ed Morley [:emorley]]

Attachment: Part 4: trim() after truncation

If the search term is longer than 200 characters we truncate it, since bug
summaries can only hold 255 characters - and we need to leave room for the
"Intermittent ..." and alternative test names etc. This truncation can leave
whitespace at the end of the search term, so we should trim() afterwards.

Comment 5

[Ed Morley [:emorley]]

Attachment: Part 5: Move search-term sanity checking into a separate function

Part 6 needs to perform the search-term sanity checking independently of
getBugsForFailure(), so let's break it out into a separate function.

Comment 6

[Ed Morley [:emorley]]

Attachment: Part 6: Fall back to whole line search for more cases

Currently we only fall back to searching for the full failure line when unable
to find a search term (ie: when the line isn't in the pipe delimited format).
This patch makes us sanity check the search term earlier, so we can fall back
to full line searching for the "too short" and "matches blacklist" cases too.

Comment 7

[Ed Morley [:emorley]]

Attachment: Part 7: Logging tweaks

Avoids repetition of the search term, cleans up output indendation and state
number of bug results returned from the cache/bzapi.

Comment 8

[Ed Morley [:emorley]]

Example new log output:

{
Prefetching: Windows XP 32-bit try debug test mochitest-browser-chrome on 45ec4e12e4fa
php /var/www/tbpl/dataimport/../php/getLogExcerpt.php id=24105119&type=annotated
Starting general_error log generation
Starting raw log generation
Generating raw log: 5046ms
Generating general_error log: 9285ms
Starting annotatedsummary log generation
* Search term: "browser_capabilities.js"
  Found in cache
  Bugs returned: 1
* Search term: "browser_net_post-data.js"
  Found in cache
  Bugs returned: 0
* Search term: "browser_net_post-data.js"
  Found in cache
  Bugs returned: 0
* Search term: "browser_net_post-data.js"
  Found in cache
  Bugs returned: 0
Generating annotatedsummary log: 4ms
Completed after 9414ms
}

Comment 9

[Ed Morley [:emorley]]

Let me know if you'd like to talk through any of the patches here :-0

Comment 10

[Ed Morley [:emorley]]

:-) even

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 763244 [details] [diff] [review]
Part 7: Logging tweaks

Nice work!

Thanks for the small chunks and good explanations along the way. Was easy to follow what you were doing :)

Comment 12

[Ed Morley [:emorley]]

Thank you for the review :-)

https://hg.mozilla.org/webtools/tbpl/rev/75600f52f6ff
https://hg.mozilla.org/webtools/tbpl/rev/ce80686fb3a3
https://hg.mozilla.org/webtools/tbpl/rev/cbfbdfa6833c
https://hg.mozilla.org/webtools/tbpl/rev/612c22f85fd0
https://hg.mozilla.org/webtools/tbpl/rev/bcd877b5c151
https://hg.mozilla.org/webtools/tbpl/rev/0af2f5a0b555
https://hg.mozilla.org/webtools/tbpl/rev/209f59e809df

Comment 13

[Ed Morley [:emorley]]

(I'll leave this to simmer on tbpl-dev for a bit to be sure all is working as expected)

Comment 14

[Ed Morley [:emorley]]

In production :-)