Closed
Bug 794799
Opened 12 years ago
Closed 12 years ago
Fix Talos TinderboxPrint output so TBPL's MachineResult.js::_getTalosResults doesn't have to un-mangle it
Categories
(Testing :: Talos, defect)
Testing
Talos
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: emorley, Assigned: emorley)
References
Details
Attachments
(4 files, 1 obsolete file)
895 bytes,
patch
|
k0scist
:
review+
|
Details | Diff | Splinter Review |
2.75 KB,
patch
|
k0scist
:
review+
|
Details | Diff | Splinter Review |
1.03 KB,
patch
|
k0scist
:
review+
|
Details | Diff | Splinter Review |
1.42 KB,
patch
|
k0scist
:
review+
|
Details | Diff | Splinter Review |
(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> }
Assignee | ||
Comment 1•12 years ago
|
||
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> }
Assignee | ||
Comment 2•12 years ago
|
||
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 }
Assignee | ||
Comment 3•12 years ago
|
||
Is already in the buildbot log; TBPL just has to strip it out of the TinderboxPrint output.
Attachment #665345 -
Flags: review?(jhammel)
Assignee | ||
Comment 4•12 years ago
|
||
...just show it in the log. (Buildbot logs already show time per build step; TBPL just strips this entry out).
Attachment #665347 -
Flags: review?(jhammel)
Assignee | ||
Comment 5•12 years ago
|
||
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.
Attachment #665365 -
Flags: review?(jhammel)
Assignee | ||
Comment 6•12 years ago
|
||
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> }
Assignee | ||
Comment 7•12 years ago
|
||
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.
Attachment #665373 -
Flags: review?(jhammel)
Comment 8•12 years ago
|
||
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
Attachment #665345 -
Flags: review?(jmaher)
Attachment #665345 -
Flags: review?(jhammel)
Attachment #665345 -
Flags: review+
Comment 9•12 years ago
|
||
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
Attachment #665347 -
Flags: review?(jhammel) → review+
Comment 10•12 years ago
|
||
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
Attachment #665365 -
Flags: review?(jhammel) → review+
Comment 11•12 years ago
|
||
Comment on attachment 665373 [details] [diff] [review] Don't also output talosErrors as 'FAIL:' Yep, wfm
Attachment #665373 -
Flags: review?(jhammel) → review+
Comment 12•12 years ago
|
||
see also https://bugzilla.mozilla.org/show_bug.cgi?id=795006
Comment 13•12 years ago
|
||
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.
Attachment #665345 -
Flags: review?(jmaher) → review+
Assignee | ||
Comment 14•12 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=77dda7f4011e
Assignee | ||
Comment 15•12 years ago
|
||
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) ========= }
Assignee | ||
Comment 16•12 years ago
|
||
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•12 years ago
|
||
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.
Assignee | ||
Updated•12 years ago
|
Assignee | ||
Comment 18•12 years ago
|
||
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.
Assignee | ||
Comment 19•12 years ago
|
||
Fix parentheses.
Attachment #665345 -
Attachment is obsolete: true
Attachment #668008 -
Flags: review?(jhammel)
Comment 20•12 years ago
|
||
Comment on attachment 668008 [details] [diff] [review] Don't output source revision (v2) wfm
Attachment #668008 -
Flags: review?(jhammel) → review+
Assignee | ||
Comment 21•12 years ago
|
||
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
Assignee | ||
Comment 22•12 years ago
|
||
In production, looks fine: https://tbpl.mozilla.org/php/getLogExcerpt.php?id=16241384&type=tinderbox_print
You need to log in
before you can comment on or make changes to this bug.
Description
•