Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2717573002:
Adding origin timeout integration test (Closed)
DescriptionAdding origin timeout integration test
Adds a test to verify that when an origin times out through Data Saver,
the request is bypassed.
Also fixes another test.
BUG=680560
Review-Url: https://codereview.chromium.org/2717573002
Cr-Commit-Position: refs/heads/master@{#452655}
Committed: https://chromium.googlesource.com/chromium/src/+/0c4e3966f07968e88ef07fc563935e8792f5ec0f
Patch Set 1 #
Total comments: 7
Patch Set 2 : tbansal comments #Patch Set 3 : fixing asserts #Messages
Total messages: 23 (14 generated)
ryansturm@chromium.org changed reviewers: + robertogden@chromium.org, tbansal@chromium.org
robertgoden, tbansal: PTAL
The CQ bit was checked by ryansturm@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...
lgtm % comments below. https://codereview.chromium.org/2717573002/diff/1/tools/chrome_proxy/webdrive... File tools/chrome_proxy/webdriver/bypass.py (right): https://codereview.chromium.org/2717573002/diff/1/tools/chrome_proxy/webdrive... tools/chrome_proxy/webdriver/bypass.py:85: # Verify that when an origin times out using Data Saver, the request bypassed. s/request/request is fetched directly and data saver is bypassed only for one request/ https://codereview.chromium.org/2717573002/diff/1/tools/chrome_proxy/webdrive... tools/chrome_proxy/webdriver/bypass.py:86: def testOriginTimeoutBypass(self): may be change test name to testOriginTimeoutBlockOnce since the test is really testing for block-once rather than bypass (which could be longer).
The CQ bit was checked by ryansturm@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/2717573002/diff/1/tools/chrome_proxy/webdrive... File tools/chrome_proxy/webdriver/bypass.py (right): https://codereview.chromium.org/2717573002/diff/1/tools/chrome_proxy/webdrive... tools/chrome_proxy/webdriver/bypass.py:85: # Verify that when an origin times out using Data Saver, the request bypassed. On 2017/02/23 21:41:13, tbansal1 wrote: > s/request/request is fetched directly and data saver is bypassed only for one > request/ Done. https://codereview.chromium.org/2717573002/diff/1/tools/chrome_proxy/webdrive... tools/chrome_proxy/webdriver/bypass.py:86: def testOriginTimeoutBypass(self): On 2017/02/23 21:41:13, tbansal1 wrote: > may be change test name to testOriginTimeoutBlockOnce since the test is really > testing for block-once rather than bypass (which could be longer). Done.
https://codereview.chromium.org/2717573002/diff/1/tools/chrome_proxy/webdrive... File tools/chrome_proxy/webdriver/bypass.py (right): https://codereview.chromium.org/2717573002/diff/1/tools/chrome_proxy/webdrive... tools/chrome_proxy/webdriver/bypass.py:93: self.assertNotEqual(1, len(responses)) Pretty sure this should be assert equals. This assertion fails on my machine.
lgtm https://codereview.chromium.org/2717573002/diff/1/tools/chrome_proxy/webdrive... File tools/chrome_proxy/webdriver/bypass.py (right): https://codereview.chromium.org/2717573002/diff/1/tools/chrome_proxy/webdrive... tools/chrome_proxy/webdriver/bypass.py:93: self.assertNotEqual(1, len(responses)) On 2017/02/23 21:50:46, Robert Ogden wrote: > Pretty sure this should be assert equals. This assertion fails on my machine. A safer check might be assertNotEqual(0, ...) so the test does not depend too much on the structure of the webpage.
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 ryansturm@chromium.org to run a CQ dry run
https://codereview.chromium.org/2717573002/diff/1/tools/chrome_proxy/webdrive... File tools/chrome_proxy/webdriver/bypass.py (right): https://codereview.chromium.org/2717573002/diff/1/tools/chrome_proxy/webdrive... tools/chrome_proxy/webdriver/bypass.py:93: self.assertNotEqual(1, len(responses)) On 2017/02/23 21:56:14, tbansal1 wrote: > On 2017/02/23 21:50:46, Robert Ogden wrote: > > Pretty sure this should be assert equals. This assertion fails on my machine. > > A safer check might be assertNotEqual(0, ...) > so the test does not depend too much on the structure of the webpage. It was supposed to be a 0, changed it somehow. Good catch.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm
The CQ bit was unchecked by ryansturm@chromium.org
The CQ bit was checked by ryansturm@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from tbansal@chromium.org Link to the patchset: https://codereview.chromium.org/2717573002/#ps40001 (title: "fixing asserts")
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": 1487887675789580,
"parent_rev": "6fd1dea30b0d03302f67800622219c763489bf79", "commit_rev":
"0c4e3966f07968e88ef07fc563935e8792f5ec0f"}
Message was sent while issue was closed.
Description was changed from ========== Adding origin timeout integration test Adds a test to verify that when an origin times out through Data Saver, the request is bypassed. Also fixes another test. BUG=680560 ========== to ========== Adding origin timeout integration test Adds a test to verify that when an origin times out through Data Saver, the request is bypassed. Also fixes another test. BUG=680560 Review-Url: https://codereview.chromium.org/2717573002 Cr-Commit-Position: refs/heads/master@{#452655} Committed: https://chromium.googlesource.com/chromium/src/+/0c4e3966f07968e88ef07fc56393... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/0c4e3966f07968e88ef07fc56393... |
