Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2440353002:
Check whether screen is on when it fails to click node in ImeTest. (Closed)
DescriptionCheck whether screen is on when it fails to click node in ImeTest.
Sometimes ImeTest failed because it failed to click node. We suspect it's
because the screen is locked. So we add assertion to check whether that's
the case.
BUG=628964
Committed: https://crrev.com/d57d3f344c2526792bd5aa74e925e4c16a3bff9c
Cr-Commit-Position: refs/heads/master@{#427023}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Address changwan@'s review #Messages
Total messages: 20 (13 generated)
The CQ bit was checked by yabinh@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 ========== Check whether screen is on when it fails to click node in ImeTest. Sometimes ImeTest failed because it failed to click node. We suspect it's because the screen is locked. So we add assertion to check whether that's the case. BUG=628964 ========== to ========== Check whether screen is on when it fails to click node in ImeTest. Sometimes ImeTest failed because it failed to click node. We suspect it's because the screen is locked. So we add assertion to check whether that's the case. BUG=628964 ==========
yabinh@chromium.org changed reviewers: + aelias@chromium.org, changwan@chromium.org
Sometimes ImeTest failed on trybot in setUp() in the line:
assertEquals("Failed to dispatch touch event.", true, result);
I suspect it's because the screen is not on for some reasons:
1. The only way to repro it locally is locking the screen.
2. The failures were rare on trybot, but several ImeTest, though not all, failed
at the same time.
3. The failures were often, though not always, accompanied with another test
failure: ContentShellPreconditionsTest#testScreenIsOn. Here are 2 builds :
https://build.chromium.org/p/chromium.android/builders/Lollipop%20Tablet%20Te...
https://build.chromium.org/p/chromium.android/builders/Jelly%20Bean%20Tester/...
lgtm w/ nits. Thanks for looking into this. also cc'ing jburodick@ in case he's aware of this issue. https://codereview.chromium.org/2440353002/diff/1/content/public/android/java... File content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java (right): https://codereview.chromium.org/2440353002/diff/1/content/public/android/java... content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java:125: @SuppressWarnings("deprecation") Could you add @TargetApi(Build.VERSION_CODES.KITKAT_WATCH) ?
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 yabinh@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/2440353002/diff/1/content/public/android/java... File content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java (right): https://codereview.chromium.org/2440353002/diff/1/content/public/android/java... content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java:125: @SuppressWarnings("deprecation") On 2016/10/24 06:19:24, Changwan Ryu wrote: > Could you add @TargetApi(Build.VERSION_CODES.KITKAT_WATCH) ? Done.
The CQ bit was unchecked by yabinh@chromium.org
The CQ bit was checked by yabinh@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from changwan@chromium.org Link to the patchset: https://codereview.chromium.org/2440353002/#ps20001 (title: "Address changwan@'s review")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Check whether screen is on when it fails to click node in ImeTest. Sometimes ImeTest failed because it failed to click node. We suspect it's because the screen is locked. So we add assertion to check whether that's the case. BUG=628964 ========== to ========== Check whether screen is on when it fails to click node in ImeTest. Sometimes ImeTest failed because it failed to click node. We suspect it's because the screen is locked. So we add assertion to check whether that's the case. BUG=628964 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Check whether screen is on when it fails to click node in ImeTest. Sometimes ImeTest failed because it failed to click node. We suspect it's because the screen is locked. So we add assertion to check whether that's the case. BUG=628964 ========== to ========== Check whether screen is on when it fails to click node in ImeTest. Sometimes ImeTest failed because it failed to click node. We suspect it's because the screen is locked. So we add assertion to check whether that's the case. BUG=628964 Committed: https://crrev.com/d57d3f344c2526792bd5aa74e925e4c16a3bff9c Cr-Commit-Position: refs/heads/master@{#427023} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/d57d3f344c2526792bd5aa74e925e4c16a3bff9c Cr-Commit-Position: refs/heads/master@{#427023}
Message was sent while issue was closed.
On 2016/10/24 06:19:24, Changwan Ryu wrote: > lgtm w/ nits. Thanks for looking into this. > > also cc'ing jburodick@ in case he's aware of this issue. No, I haven't seen this. We disable the lock screen and force the screen to stay on while plugged in during device provisioning. If you see a test that fails on a bot in assertScreenIsOn, please file a bug w/ component Infra>Client>Android. > > https://codereview.chromium.org/2440353002/diff/1/content/public/android/java... > File > content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java > (right): > > https://codereview.chromium.org/2440353002/diff/1/content/public/android/java... > content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java:125: > @SuppressWarnings("deprecation") > Could you add @TargetApi(Build.VERSION_CODES.KITKAT_WATCH) ? |
