Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2732133002:
Adds proxy pingback chromedriver integration test (testPingback). (Closed)
|
Created:
3 years, 9 months ago by dougarnett Modified:
3 years, 9 months ago CC:
chromium-reviews, tbansal+watch-data-reduction-proxy_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdds proxy pingback chromedriver integration test (testPingback).
BUG=680577
Review-Url: https://codereview.chromium.org/2732133002
Cr-Commit-Position: refs/heads/master@{#455474}
Committed: https://chromium.googlesource.com/chromium/src/+/ce6e8b3d591faa42d1044ae562cde091166e3ccc
Patch Set 1 #
Total comments: 2
Patch Set 2 : Removed redundant stats flag #
Total comments: 2
Patch Set 3 : Added SleepUntilHistogramHasEntry() #Messages
Total messages: 24 (17 generated)
The CQ bit was checked by dougarnett@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
dougarnett@chromium.org changed reviewers: + robertogden@chromium.org, ryansturm@chromium.org
lgtm % nit https://codereview.chromium.org/2732133002/diff/1/tools/chrome_proxy/webdrive... File tools/chrome_proxy/webdriver/smoke.py (right): https://codereview.chromium.org/2732133002/diff/1/tools/chrome_proxy/webdrive... tools/chrome_proxy/webdriver/smoke.py:39: t.AddChromeArg('--enable-stats-collection-bindings') This one is added for you in common.py:173, no need to add it here
The CQ bit was checked by dougarnett@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2732133002/diff/1/tools/chrome_proxy/webdrive... File tools/chrome_proxy/webdriver/smoke.py (right): https://codereview.chromium.org/2732133002/diff/1/tools/chrome_proxy/webdrive... tools/chrome_proxy/webdriver/smoke.py:39: t.AddChromeArg('--enable-stats-collection-bindings') On 2017/03/06 23:15:16, Robert Ogden wrote: > This one is added for you in common.py:173, no need to add it here THanks - done
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm % adding a call to wait for the histogram (to avoid occasional flakiness) https://codereview.chromium.org/2732133002/diff/20001/tools/chrome_proxy/webd... File tools/chrome_proxy/webdriver/smoke.py (right): https://codereview.chromium.org/2732133002/diff/20001/tools/chrome_proxy/webd... tools/chrome_proxy/webdriver/smoke.py:40: t.LoadURL('http://check.googlezip.net/test.html') Can you add a t.SleepUntilHistogramHasEntry("DataReductionProxy.Pingback.Succeeded") after the second load? https://cs.chromium.org/chromium/src/tools/chrome_proxy/webdriver/common.py?l... Technically, there is a race here with the second loadURL triggering an out of band low-priority request.
The CQ bit was checked by dougarnett@chromium.org to run a CQ dry run
https://codereview.chromium.org/2732133002/diff/20001/tools/chrome_proxy/webd... File tools/chrome_proxy/webdriver/smoke.py (right): https://codereview.chromium.org/2732133002/diff/20001/tools/chrome_proxy/webd... tools/chrome_proxy/webdriver/smoke.py:40: t.LoadURL('http://check.googlezip.net/test.html') On 2017/03/07 18:32:56, Ryan Sturm wrote: > Can you add a > t.SleepUntilHistogramHasEntry("DataReductionProxy.Pingback.Succeeded") after the > second load? > > https://cs.chromium.org/chromium/src/tools/chrome_proxy/webdriver/common.py?l... > > Technically, there is a race here with the second loadURL triggering an out of > band low-priority request. Done.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by dougarnett@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from robertogden@chromium.org, ryansturm@chromium.org Link to the patchset: https://codereview.chromium.org/2732133002/#ps40001 (title: "Added SleepUntilHistogramHasEntry()")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1488991813595040,
"parent_rev": "5e6bda5447afa39cc476898bcdd2ee560b1620bc", "commit_rev":
"ce6e8b3d591faa42d1044ae562cde091166e3ccc"}
Message was sent while issue was closed.
Description was changed from ========== Adds proxy pingback chromedriver integration test (testPingback). BUG=680577 ========== to ========== Adds proxy pingback chromedriver integration test (testPingback). BUG=680577 Review-Url: https://codereview.chromium.org/2732133002 Cr-Commit-Position: refs/heads/master@{#455474} Committed: https://chromium.googlesource.com/chromium/src/+/ce6e8b3d591faa42d1044ae562cd... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/ce6e8b3d591faa42d1044ae562cd... |
