New issue
Advanced search Search tips

Issue 753029 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Add support for the n parameter in the revision_range handler

Project Member Reported by pdr@chromium.org, Aug 7 2017

Issue description

Go 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.)
 
perfgraph.png
272 KB View Download
I can confirm that this is what I'm seeing as well.
Cc: simonhatch@chromium.org
Wow, thanks for reporting! Simon, do you have time to take a look today?
Components: -Infra>Client>Perf Speed>Dashboard
Owner: simonhatch@chromium.org
Owner: ----
There's a "Next" link at the bottom of the page that continues, the results are paginated.

Comment 6 by pdr@chromium.org, 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.
Yep can do! I'll see where that's done and put up a cl.
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?

Comment 9 by pdr@chromium.org, Aug 8 2017

Cc: serg...@chromium.org
Labels: -Pri-1 Pri-2
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
Cc: -serg...@chromium.org
Owner: serg...@chromium.org
Status: Started (was: Untriaged)
Yes, that is correct place in the code. CL: 
Status: Fixed (was: Started)
Deployed the change to the app.
Summary: Add support for the n parameter in the revision_range handler (was: Chrome perf regression range links are wrong)
P.S. Someone still needs to update chromeperf app. Filed  issue 756440  for this.

Comment 15 by pdr@chromium.org, Aug 17 2017

Thank you for the quick fix! This will reduce confusion and increase our chances of catching regressions.
Project Member

Comment 16 by bugdroid1@chromium.org, 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