Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2838393002:
Assert the location is correct when processing a tap request. (Closed)
|
Created:
3 years, 8 months ago by dtapuska Modified:
3 years, 7 months ago Reviewers:
Ken Russell (switch to Gerrit) CC:
chromium-reviews, mlamouri+watch-content_chromium.org, extensions-reviews_chromium.org, jam, darin-cc_chromium.org, chromium-apps-reviews_chromium.org, piman+watch_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionAssert the location is correct when processing a tap request.
Check that the position is correct when executing the GPU benchmark
Tap request if it isn't throw an exception.
BUG=683503
Review-Url: https://codereview.chromium.org/2838393002
Cr-Commit-Position: refs/heads/master@{#469032}
Committed: https://chromium.googlesource.com/chromium/src/+/eb843b0448b95be3dbbb5b398237a86c3861b078
Patch Set 1 #Patch Set 2 : Fix origin translation problem #Patch Set 3 : Add layout test #
Messages
Total messages: 26 (18 generated)
The CQ bit was checked by dtapuska@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: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by dtapuska@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...
Description was changed from ========== Assert the location is correct when processing a tap request. Check that the position is correct when executing the GPU benchmark Tap request. BUG=683503 ========== to ========== Assert the location is correct when processing a tap request. Check that the position is correct when executing the GPU benchmark Tap request if it isn't throw an exception. BUG=683503 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
dtapuska@chromium.org changed reviewers: + kbr@chromium.org
lgtm, but think you should add some tests that trigger the failure mode and assert that the exception is thrown.
On 2017/04/28 22:31:27, Ken Russell wrote: > lgtm, but think you should add some tests that trigger the failure mode and > assert that the exception is thrown. I can't find any test cases for any of the gpuBenchmarking methods. Can you direct me to where these are actually tested?
On 2017/05/01 15:37:35, dtapuska wrote: > On 2017/04/28 22:31:27, Ken Russell wrote: > > lgtm, but think you should add some tests that trigger the failure mode and > > assert that the exception is thrown. > > I can't find any test cases for any of the gpuBenchmarking methods. Can you > direct me to where these are actually tested? A quick code search turned up this layout test: https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/fast/even... So presumably another one could be placed alongside it.
On 2017/05/01 17:45:11, Ken Russell wrote: > On 2017/05/01 15:37:35, dtapuska wrote: > > On 2017/04/28 22:31:27, Ken Russell wrote: > > > lgtm, but think you should add some tests that trigger the failure mode and > > > assert that the exception is thrown. > > > > I can't find any test cases for any of the gpuBenchmarking methods. Can you > > direct me to where these are actually tested? > > A quick code search turned up this layout test: > https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/fast/even... > > So presumably another one could be placed alongside it. Oh you wanted a layout test.. it is a little far disconnected to the implementation but I guess I could add it that way.. I was looking more in content/tests/gpu/*
On 2017/05/01 17:47:47, dtapuska wrote: > On 2017/05/01 17:45:11, Ken Russell wrote: > > On 2017/05/01 15:37:35, dtapuska wrote: > > > On 2017/04/28 22:31:27, Ken Russell wrote: > > > > lgtm, but think you should add some tests that trigger the failure mode > and > > > > assert that the exception is thrown. > > > > > > I can't find any test cases for any of the gpuBenchmarking methods. Can you > > > direct me to where these are actually tested? > > > > A quick code search turned up this layout test: > > > https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/fast/even... > > > > So presumably another one could be placed alongside it. > > Oh you wanted a layout test.. it is a little far disconnected to the > implementation but I guess I could add it that way.. I was looking more in > content/tests/gpu/* Yeah, sorry, I don't think these methods are easily unit testable. Some of these methods are used by the tests in content/tests/gpu, but not really the mouse- and scroll-related stuff. Those are mainly used by the performance tests to the best of my knowledge.
The CQ bit was checked by dtapuska@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.
The CQ bit was checked by dtapuska@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from kbr@chromium.org Link to the patchset: https://codereview.chromium.org/2838393002/#ps40001 (title: "Add layout test")
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": 1493834021852980,
"parent_rev": "a290b0d17c13bc5c506f7bce0e0cbbeb3e8afcd6", "commit_rev":
"eb843b0448b95be3dbbb5b398237a86c3861b078"}
Message was sent while issue was closed.
Description was changed from ========== Assert the location is correct when processing a tap request. Check that the position is correct when executing the GPU benchmark Tap request if it isn't throw an exception. BUG=683503 ========== to ========== Assert the location is correct when processing a tap request. Check that the position is correct when executing the GPU benchmark Tap request if it isn't throw an exception. BUG=683503 Review-Url: https://codereview.chromium.org/2838393002 Cr-Commit-Position: refs/heads/master@{#469032} Committed: https://chromium.googlesource.com/chromium/src/+/eb843b0448b95be3dbbb5b398237... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/eb843b0448b95be3dbbb5b398237... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
