Closed Bug 1398069 Opened 2 years ago Closed 2 years ago
Ensure talos account for 'cycles' and coalesce results
59 bytes, text/x-review-board-request
See bug 1394804 comment 30: > I see that we run this 5 times, but oddly it seems as though we have a reporting > problem- the resulting PERFHERDER_JSON had 5 duplicate copies of the data (25 > replicates/subtest - 5 identical copies) where as normally we just publish a > single copy of the data. > Please file a bug to work on that, specifically ensure that we account for > 'cycles' and coalesce results, here is the code area: > http://searchfox.org/mozilla-central/source/testing/talos/talos/output.py#58
This code is very hard to follow with all these result/results/tresults/tpresults, but this patch got things to work locally for my DAMP needs. I have *absolutely no idea* if that break any other test involving this codepath. https://treeherder.mozilla.org/#/jobs?repo=try&revision=9eefd4df86b3cd71731d06cc1682c1f18f392f70
Should I ask someone else to review this patch? It prevents me from proceeding with bug 1394804.
Comment on attachment 8906695 [details] Bug 1398069 - Merge results when using cycles > 1. I had looked at this and was not sure if it was right/wrong or what could be missing. :rwood, can you pick this up tomorrow and figure it out. This is not an area of the talos code that we are both very familiar with
Attachment #8906695 - Flags: review?(jmaher) → review?(rwood)
(In reply to Joel Maher ( :jmaher) (UTC-5) from comment #4) ... Ok I'll have a look also
FYI, I added the rest of the talos jobs to the try run in comment 2
Update: Spent some time looking into tppagecycles vs tpcycles vs cycles and I now understand the issue. For example, if you run the existing DAMP with --tppagecycles 2 and -cycles 1, you get 2 replicates per test case/subtest as expected. If you run --tppagecycles 2 and -cycles 2 the entire suite is repeated as expected however for the talos.json output I had expected to see the first set of results/subtests each with 2 replicates, and then all the same subtests listed again with 2 more replicates. However what you see is the first set of subtests output with each subtest listing all 4 replicates, and then all of the subtests output repeated again and listing the exact same 4 replicate values over again.
Update: I applied the patch locally and ran damp, and confirmed with --tppagecycles 2 and -cycles 2, the output now just contains one set of all the subtests, each subtest having 4 replicates listed, as expected. Note: ts_paint, sessionrestore, and tresize tests are the other tests that use cycles = N where N is not the default of 1; so I am checking to see how this patch will effect the output of those tests.
Comment on attachment 8906695 [details] Bug 1398069 - Merge results when using cycles > 1. Yep looks good! Also verified this patch has no effect on the other talos tests that use cycles > 1 by default; they still report the same way.
r+ wasn't forwarded to mozreview, so requesting checkin. I'm not sure this keyword is still watched??
Attachment #8906695 - Flags: review+ → review?(rwood)
This doesn't meet the review requirements in MozReview for autoland to push it. http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview/autoland.html#landing-commits
:rwood and :ochameau are both consistent contributors to the repo- are they not l3? who is not l3?
Will it not land in mozreview maybe because the commit text is 'r=jmaher' however I am the one who reviewed it? Mozreview won't allow me to 'ship it'.
Thanks Alexandre, even with r=rwood, in mozreview no 'ship it' option appears for me even when I'm logged in. Not sure what is going on...
:jmaher, are you able to mark this review as 'ship it'? https://reviewboard.mozilla.org/r/178428/
Comment on attachment 8906695 [details] Bug 1398069 - Merge results when using cycles > 1. https://reviewboard.mozilla.org/r/178430/#review186510
Attachment #8906695 - Flags: review+
it appears so
Not a good day to land patches: "A server error occurred: Autoland returned an error message during communications."
Attachment #8906695 - Flags: review?(rwood)
I assume this is because autoland is closed right now
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/31376e1efa8f Merge results when using cycles > 1. r=jmaher
You need to log in before you can comment on or make changes to this bug.