794799 - Fix Talos TinderboxPrint output so TBPL's MachineResult.js::_getTalosResults doesn't have to un-mangle it

Status: RESOLVED FIXED

(Testing :: Talos, defect)

Description

[Ed Morley [:emorley]]

(Phil Ringnalda (:philor) from bug 790963 comment #6)
> I really think the right thing to do is to fix Talos (have you looked at the
> graphserver URLs we get for the suites where we get both a link for the
> number, and a link for "(details)"? and the reason that we have this special
> FAIL: processing, to strip out a TinderboxPrint:<br>? and the way the
> useless repeated details links come in a <p style="font-size:smaller;">?),
> but I don't think that we'll lose anything by not using the generic
> _getScrapeResults even when there's no graphserver URL, because I don't
> think there's anything interesting TinderboxPrinted in those cases.

eg:

{
Completed test tresize: 
		Stopped Thu, 27 Sep 2012 01:05:51
RETURN: cycle time: 00:18:15<br>
talos-r3-xp-051: 
		Stopped Thu, 27 Sep 2012 01:05:51
NOISE: Outputting talos results => {'results_urls': ['http://graphs.mozilla.org/server/collect.cgi'], 'datazilla_urls': ['https://datazilla.mozilla.org/talos']}
Generating results file: tdhtmlr: 
		Started Thu, 27 Sep 2012 01:05:51
Generating results file: tdhtmlr: 
		Stopped Thu, 27 Sep 2012 01:05:51
Generating results file: tresize: 
		Started Thu, 27 Sep 2012 01:05:51
Generating results file: tresize: 
		Stopped Thu, 27 Sep 2012 01:05:51
NOISE: Posting result 0 of 2 to http://graphs.mozilla.org/server/collect.cgi, attempt 0
NOISE: Posting result 1 of 2 to http://graphs.mozilla.org/server/collect.cgi, attempt 0
RETURN:<br>
RETURN:<a href='http://graphs.mozilla.org/graph.html#tests=[[220,131,1]]'>tdhtmlr_paint: 619.03</a><br>
RETURN:<a href='http://graphs.mozilla.org/graph.html#tests=[[254,131,1]]'>tresize: 13.0</a><br>
RETURN:<p style="font-size:smaller;">Details:<br>| <a href='http://graphs.mozilla.org/graph.html#tests=[[220,131,1]]'>tdhtmlr_paint</a> | <a href='http://graphs.mozilla.org/graph.html#tests=[[254,131,1]]'>tresize</a> |</p>
NOISE: Outputting datazilla results to https://datazilla.mozilla.org/talos
NOISE: datazilla: https//datazilla.mozilla.org/talos; oauth=False
program finished with exit code 0
elapsedTime=1099.938000
TinderboxPrint:<a href = "http://hg.mozilla.org/integration/mozilla-inbound/rev/157f3efdee37">rev:157f3efdee37</a>

TinderboxPrint: cycle time: 00:18:15<br>

TinderboxPrint:<br>

TinderboxPrint:<a href='http://graphs.mozilla.org/graph.html#tests=[[220,131,1]]'>tdhtmlr_paint: 619.03</a><br>

TinderboxPrint:<a href='http://graphs.mozilla.org/graph.html#tests=[[254,131,1]]'>tresize: 13.0</a><br>

TinderboxPrint:<p style="font-size:smaller;">Details:<br>| <a href='http://graphs.mozilla.org/graph.html#tests=[[220,131,1]]'>tdhtmlr_paint</a> | <a href='http://graphs.mozilla.org/graph.html#tests=[[254,131,1]]'>tresize</a> |</p>
}

Comment 1

[Ed Morley [:emorley]]

Output of https://tbpl.mozilla.org/php/getLogExcerpt.php?id=15584544&type=tinderbox_print:
{
 s: talos-r3-xp-051
<a href = "http://hg.mozilla.org/integration/mozilla-inbound/rev/157f3efdee37">rev:157f3efdee37</a>
 cycle time: 00:18:15<br>
<br>
<a href='http://graphs.mozilla.org/graph.html#tests=[[220,131,1]]'>tdhtmlr_paint: 619.03</a><br>
<a href='http://graphs.mozilla.org/graph.html#tests=[[254,131,1]]'>tresize: 13.0</a><br>
<p style="font-size:smaller;">Details:<br>| <a href='http://graphs.mozilla.org/graph.html#tests=[[220,131,1]]'>tdhtmlr_paint</a> | <a href='http://graphs.mozilla.org/graph.html#tests=[[254,131,1]]'>tresize</a> |</p>
}

Comment 2

[Ed Morley [:emorley]]

Example failure case:
{
Traceback (most recent call last):
  File "run_tests.py", line 250, in run_tests
getting files in '/mnt/sdcard/tests/profile/minidumps/'
Failed tp4m: 
		Stopped Thu, 27 Sep 2012 01:09:59
    talos_results.add(mytest.runTest(browser_config, test))
  File "/builds/tegra-259/talos-data/talos/ttest.py", line 378, in runTest
    test_results.add(browser_log_filename, counter_results=counter_results)
  File "/builds/tegra-259/talos-data/talos/results.py", line 120, in add
    browserLog = BrowserLogResults(filename=results, counter_results=counter_results, global_counters=self.global_counters)
  File "/builds/tegra-259/talos-data/talos/results.py", line 319, in __init__
    self.parse()
  File "/builds/tegra-259/talos-data/talos/results.py", line 356, in parse
    self.error("Could not find %s in browser output: (tokens: %s)" % (attr, tokens))
  File "/builds/tegra-259/talos-data/talos/results.py", line 331, in error
    raise utils.talosError(message)
talosError: "Could not find beforeLaunchTime in browser output: (tokens: ('__startBeforeLaunchTimestamp', '__endBeforeLaunchTimestamp')) [browser_output.txt]"
Traceback (most recent call last):
  File "run_tests.py", line 298, in <module>
FAIL: Busted: tp4m
FAIL: Could not find beforeLaunchTime in browser output: (tokens: ('__startBeforeLaunchTimestamp', '__endBeforeLaunchTimestamp')) [browser_output.txt]
    main()
  File "run_tests.py", line 295, in main
    run_tests(parser)
  File "run_tests.py", line 259, in run_tests
    raise e
utils.talosError: "Could not find beforeLaunchTime in browser output: (tokens: ('__startBeforeLaunchTimestamp', '__endBeforeLaunchTimestamp')) [browser_output.txt]"
program finished with exit code 1
}

Comment 3

[Ed Morley [:emorley]]

Attachment: Don't output source revision

Is already in the buildbot log; TBPL just has to strip it out of the TinderboxPrint output.

Comment 4

[Ed Morley [:emorley]]

Attachment: Don't TinderboxPrint cycle time

...just show it in the log. (Buildbot logs already show time per build step; TBPL just strips this entry out).

Comment 5

[Ed Morley [:emorley]]

Attachment: Clean up graphserver links

The output we're getting at the moment is of the form:
{
RETURN:<br>
RETURN:<a href='http://graphs.mozilla.org/graph.html#tests=[[220,131,1]]'>tdhtmlr_paint: 619.03</a><br>
RETURN:<a href='http://graphs.mozilla.org/graph.html#tests=[[254,131,1]]'>tresize: 13.0</a><br>
RETURN:<p style="font-size:smaller;">Details:<br>| <a href='http://graphs.mozilla.org/graph.html#tests=[[220,131,1]]'>tdhtmlr_paint</a> | <a href='http://graphs.mozilla.org/graph.html#tests=[[254,131,1]]'>tresize</a> |</p>
}

Which after a lot of head scratching reading results_from_graph() - which is sadly less than clear - boils down to:

{
'RETURN:<br>' +
first_results + 
'\nRETURN:<p style="font-size:smaller;">Details:<br>' + 
last_results + 
'|</p>'
}

...where first_results is a newline separated list of lines for which we were able to find a linkvalue (ie: test score used as the link text for the URL).
...and last_results is a pipe separated list of lines for which we were not able to find a linkvalue.

At the moment last_results is just duplicating first_results, but without being able to see the graphserver response, I've no idea if this is expected/intended.

Therefore this patch just ditches the idea of first_results/last_results (as well as stopping adding random '<br/>'s all over the place), and just shows results that have a linkvalue. It also (IMO) makes results_from_graph()a lot more grokable.

Comment 6

[Ed Morley [:emorley]]

Comment on attachment 665365 [details] [diff] [review]
Clean up graphserver links

After this patch, the output in comment 5, becomes:

{
RETURN:<a href='http://graphs.mozilla.org/graph.html#tests=[[220,131,1]]'>tdhtmlr_paint: 619.03</a>
RETURN:<a href='http://graphs.mozilla.org/graph.html#tests=[[254,131,1]]'>tresize: 13.0</a>
}

Comment 7

[Ed Morley [:emorley]]

Attachment: Don't also output talosErrors as 'FAIL:'

TBPL now recognises talosError, so the "FAIL:" output here is redundant, and is one of the contributing factors in the mess that is comment 2.

Comment 8

[Jeff Hammel]

Comment on attachment 665345 [details] [diff] [review]
Don't output source revision

+  if not ('repository' in browser_config) and ('sourcestamp' in browser_config):

Since I'm one of those fools that never remembers order of operations, I'd prefer

if not ('repository' in browser_config and 'sourcestamp' in browser_config):

or if you want to be even more verbose:

if not (('repository' in browser_config) and ('sourcestamp' in browser_config)):

I'm going to give this to jmaher to review.  I have no objections, but I also have no idea why this code is there o_O

Comment 9

[Jeff Hammel]

Comment on attachment 665347 [details] [diff] [review]
Don't TinderboxPrint cycle time

As long as we're cleaning up, can we format this in a more consistent way?

+  print "cycle time: %s" % elapsed

Comment 10

[Jeff Hammel]

Comment on attachment 665365 [details] [diff] [review]
Clean up graphserver links

This is fine by me.  As a standard disclaimer, this is integration code that I'm not overly fond of being in talos anyway.  So if it doesn't cause any problem with TBPL, I'm all for it

Comment 11

[Jeff Hammel]

Comment on attachment 665373 [details] [diff] [review]
Don't also output talosErrors as 'FAIL:'

Yep, wfm

Comment 12

[Jeff Hammel]

see also https://bugzilla.mozilla.org/show_bug.cgi?id=795006

Comment 13

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 665345 [details] [diff] [review]
Don't output source revision

Review of attachment 665345 [details] [diff] [review]:
-----------------------------------------------------------------

as far as I know this is good.  we should really run this on try server.

Comment 14

[Ed Morley [:emorley]]

https://tbpl.mozilla.org/?tree=Try&rev=77dda7f4011e

Comment 15

[Ed Morley [:emorley]]

I don't understand this failure:

{
Traceback (most recent call last):
  File "remotePerfConfigurator.py", line 243, in <module>
    sys.exit(main())
  File "remotePerfConfigurator.py", line 239, in main
    conf.parse_args(args)
  File "/builds/tegra-095/talos-data/talos/PerfConfigurator.py", line 398, in parse_args
    options, args = Configuration.parse_args(self, *args, **kwargs)
  File "/builds/tegra-095/talos-data/talos/configuration.py", line 466, in parse_args
    self(*config)
  File "/builds/tegra-095/talos-data/talos/configuration.py", line 372, in __call__
    self.validate()
  File "remotePerfConfigurator.py", line 120, in validate
    self._setupRemote(deviceip, deviceport)
  File "remotePerfConfigurator.py", line 197, in _setupRemote
    raise pc.ConfigurationError("Unable to connect to remote device '%s'" % deviceip)
PerfConfigurator.ConfigurationError: Unable to connect to remote device '10.250.49.83'
program finished with exit code 1
elapsedTime=0.419459
========= Finished 'python remotePerfConfigurator.py ...' failed (results: 2, elapsed: 0 secs) (at 2012-09-27 12:47:46.626564) =========
}

Comment 16

[Ed Morley [:emorley]]

Or more: This failure doesn't seem like it could be related, but yet it is occurring on every android talos run on my push and none others I can see on Try; so must be due to my push somehow.

Comment 17

[Joel Maher ( :jmaher ) (UTC -8)]

This error is during confuguration when we connect to the device and setup it up.  We also need to pull information like the deviceroot from the device.

Comment 18

[Ed Morley [:emorley]]

Comment on attachment 665345 [details] [diff] [review]
Don't output source revision

Review of attachment 665345 [details] [diff] [review]:
-----------------------------------------------------------------

::: talos/run_tests.py
@@ +51,5 @@
>      if not browser_config.get('browser_name'):
>        browser_config['browser_name'] = config.get('App', 'Name')
>      if not browser_config.get('browser_version'):
>        browser_config['browser_version'] = config.get('App', 'Version')
> +  if not ('repository' in browser_config) and ('sourcestamp' in browser_config):

Bah, this should be:
if not ((foo) and (bar)):

...missing some parentheses.

Comment 19

[Ed Morley [:emorley]]

Attachment: Don't output source revision (v2)

Fix parentheses.

Comment 20

[Jeff Hammel]

Comment on attachment 668008 [details] [diff] [review]
Don't output source revision (v2)

wfm

Comment 21

[Ed Morley [:emorley]]

https://hg.mozilla.org/build/talos/rev/babd4748ce0b
https://hg.mozilla.org/build/talos/rev/10c2387c497d
https://hg.mozilla.org/build/talos/rev/bb96110403fc
https://hg.mozilla.org/build/talos/rev/6f0fb4ab3a0c

Comment 22

[Ed Morley [:emorley]]

In production, looks fine:
https://tbpl.mozilla.org/php/getLogExcerpt.php?id=16241384&type=tinderbox_print