Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2691033004:
[Local NTP] Add an integration test for the fakebox (Closed)
|
Created:
3 years, 10 months ago by Marc Treib Modified:
3 years, 10 months ago Reviewers:
sfiera CC:
chromium-reviews, skanuj+watch_chromium.org, melevin+watch_chromium.org, donnd+watch_chromium.org, jfweitz+watch_chromium.org, David Black, samarth+watch_chromium.org, kmadhusu+watch_chromium.org, Jered Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[Local NTP] Add an integration test for the fakebox
BUG=692002
Review-Url: https://codereview.chromium.org/2691033004
Cr-Commit-Position: refs/heads/master@{#450706}
Committed: https://chromium.googlesource.com/chromium/src/+/af71c6b09dc5bd882f2f2018050c2902a92e024b
Patch Set 1 #Patch Set 2 : . #
Total comments: 2
Patch Set 3 : real mouse events #Patch Set 4 : s/int/double/ #Patch Set 5 : fix Mac #
Messages
Total messages: 39 (28 generated)
The CQ bit was checked by treib@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: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
Description was changed from ========== [Local NTP] Add some integration tests In particular, this adds an end-to-end test for the fakebox behavior. BUG=692002 ========== to ========== [Local NTP] Add an integration test for the fakebox BUG=692002 ==========
The CQ bit was checked by treib@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.
treib@chromium.org changed reviewers: + sfiera@chromium.org
PTAL!
LGTM https://codereview.chromium.org/2691033004/diff/20001/chrome/browser/ui/searc... File chrome/browser/ui/search/local_ntp_browsertest.cc (right): https://codereview.chromium.org/2691033004/diff/20001/chrome/browser/ui/searc... chrome/browser/ui/search/local_ntp_browsertest.cc:82: ASSERT_TRUE(GetBoolFromJS(active_tab, "!!clickOnFakebox()", &result)); Is it not also possible to send a "real" mouse event from native code, like the key event below?
https://codereview.chromium.org/2691033004/diff/20001/chrome/browser/ui/searc... File chrome/browser/ui/search/local_ntp_browsertest.cc (right): https://codereview.chromium.org/2691033004/diff/20001/chrome/browser/ui/searc... chrome/browser/ui/search/local_ntp_browsertest.cc:82: ASSERT_TRUE(GetBoolFromJS(active_tab, "!!clickOnFakebox()", &result)); On 2017/02/14 14:05:44, sfiera wrote: > Is it not also possible to send a "real" mouse event from native code, like the > key event below? Artifact of the order in which I wrote this stuff :) Yes, should be possible, though it'll be a bit tricky to find the correct position... I'll investigate.
The CQ bit was checked by treib@chromium.org to run a CQ dry run
Now with real mouse events - PTAL again! (Here's hoping this won't be super-flaky...)
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
LBTM
The CQ bit was unchecked by treib@chromium.org
The CQ bit was checked by treib@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sfiera@chromium.org Link to the patchset: https://codereview.chromium.org/2691033004/#ps40001 (title: "real mouse events")
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
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 treib@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...
On 2017/02/14 19:10:41, commit-bot: I haz the power wrote: > 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_...) So, turns out getBoundingClientRect returns floats rather than ints on Linux and Mac, but apparently not on Windows. That's what you get for working on Win :D
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 treib@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...
On 2017/02/15 10:14:04, Marc Treib wrote: > On 2017/02/14 19:10:41, commit-bot: I haz the power wrote: > > 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_...) > > So, turns out getBoundingClientRect returns floats rather than ints on Linux and > Mac, but apparently not on Windows. That's what you get for working on Win :D aaand the latest patch set fixes Mac too. This was a fun one to track down.
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 treib@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sfiera@chromium.org Link to the patchset: https://codereview.chromium.org/2691033004/#ps80001 (title: "fix Mac")
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": 80001, "attempt_start_ts": 1487172460894890,
"parent_rev": "ff2bfbfd76f3c43e5618addd34c82fed42d2a6f5", "commit_rev":
"af71c6b09dc5bd882f2f2018050c2902a92e024b"}
Message was sent while issue was closed.
Description was changed from ========== [Local NTP] Add an integration test for the fakebox BUG=692002 ========== to ========== [Local NTP] Add an integration test for the fakebox BUG=692002 Review-Url: https://codereview.chromium.org/2691033004 Cr-Commit-Position: refs/heads/master@{#450706} Committed: https://chromium.googlesource.com/chromium/src/+/af71c6b09dc5bd882f2f2018050c... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://chromium.googlesource.com/chromium/src/+/af71c6b09dc5bd882f2f2018050c... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
