Add support for the n parameter in the revision_range handler |
|||||||||
Issue descriptionGo to https://chromeperf.appspot.com/report?sid=8ddd98d5d45b23097d12af8e6a96b3afe7cb5353efcec8f7f64197e7f9b2fde0&start_rev=474705&end_rev=492304 Click the dot at point 492215 (this was a progression near the end of the graph). The link says: "492050 - 492215" The link goes to: https://chromium.googlesource.com/chromium/src/+log/9d3c07877e454d4540255b48f458b0c0d59f16fc%5E..2440912f23c01e661b47341bb322f50f67cb153b?pretty=fuller (Which is revisions r492116 to r492215.)
,
Aug 8 2017
Wow, thanks for reporting! Simon, do you have time to take a look today?
,
Aug 8 2017
,
Aug 8 2017
,
Aug 8 2017
There's a "Next" link at the bottom of the page that continues, the results are paginated.
,
Aug 8 2017
Ah! You're absolutely right. Would it be possible to append "&n=10000" to the url to reduce pagination? In this case both Vlad and I mistakenly thought our patch was not in the regression range because we used the browser find function to look for our names in the range.
,
Aug 8 2017
Yep can do! I'll see where that's done and put up a cl.
,
Aug 8 2017
Hey so test-results.appspot.com doesn't propagate that param: http://test-results.appspot.com/revision_range?start=492050&end=492215&n=1000 Not clear what params test-results supports, any idea where those are documented?
,
Aug 8 2017
Could that be handled in this code? https://cs.chromium.org/chromium/infra/go/src/infra/appengine/test-results/frontend/revision.go?type=cs&sq=package:chromium&l=51 +cc sergiyb
,
Aug 9 2017
Yes, that is correct place in the code. CL:
,
Aug 9 2017
,
Aug 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/infra/infra/+/f2ebff47959d8916ad7b2bfc2e75d1162b2b0610 commit f2ebff47959d8916ad7b2bfc2e75d1162b2b0610 Author: Sergiy Byelozyorov <sergiyb@chromium.org> Date: Thu Aug 17 09:04:04 2017 Pass param `n` through and add tests for revisionHandler R=pdr@chromium.org Bug: 753029 Change-Id: I6a35cdc2f561075a30cd26d6de7608c5bd4249b4 Reviewed-on: https://chromium-review.googlesource.com/608240 Commit-Queue: Sergiy Byelozyorov <sergiyb@chromium.org> Reviewed-by: Erik Staab <estaab@chromium.org> [modify] https://crrev.com/f2ebff47959d8916ad7b2bfc2e75d1162b2b0610/go/src/infra/appengine/test-results/frontend/revision.go [modify] https://crrev.com/f2ebff47959d8916ad7b2bfc2e75d1162b2b0610/go/src/infra/appengine/test-results/frontend/frontend.infra_testing [modify] https://crrev.com/f2ebff47959d8916ad7b2bfc2e75d1162b2b0610/go/src/infra/appengine/test-results/frontend/revision_test.go
,
Aug 17 2017
Deployed the change to the app.
,
Aug 17 2017
P.S. Someone still needs to update chromeperf app. Filed issue 756440 for this.
,
Aug 17 2017
Thank you for the quick fix! This will reduce confusion and increase our chances of catching regressions.
,
Sep 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4fd5e709632cb3ed441697f6463538df70818b1d commit 4fd5e709632cb3ed441697f6463538df70818b1d Author: catapult-deps-roller@chromium.org <catapult-deps-roller@chromium.org> Date: Tue Sep 19 03:29:44 2017 Roll src/third_party/catapult/ c173be04c..d6ef217af (2 commits) https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/c173be04cac2..d6ef217affb3 $ git log c173be04c..d6ef217af --date=short --no-merges --format='%ad %ae %s' 2017-09-18 simonhatch Dashboard - Append n=1000 to show large chromium ranges. 2017-09-18 dtu [pinpoint] Bold Commit subject in bug comment. Created with: roll-dep src/third_party/catapult BUG= 753029 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, see: http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel TBR=sullivan@chromium.org Change-Id: Ief0bdee37bfbb217170b2bb5fd7bf568649f46b9 Reviewed-on: https://chromium-review.googlesource.com/672150 Reviewed-by: <catapult-deps-roller@chromium.org> Commit-Queue: <catapult-deps-roller@chromium.org> Cr-Commit-Position: refs/heads/master@{#502773} [modify] https://crrev.com/4fd5e709632cb3ed441697f6463538df70818b1d/DEPS |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by vmp...@chromium.org
, Aug 7 2017