Use Non-Autocomplete textarea in ImeTest

Use Non-Autocomplete textarea in ImeTest

On some devices, the textarea will be busy executing
autocomplete code while clicking on the textarea in
ImeTest#testKeyboardAppFinishesCompositionOnUnexpectedSelectionChange
By using the textarea without autocomplete, this issue is
avoided while still testing the full intent of this test.
(The autocomplete is not essential to what the test tries
to accomplish)

BUG=704131
Review-Url: https://codereview.chromium.org/2768463005
Cr-Commit-Position: refs/heads/master@{#460411}
Committed: https://chromium.googlesource.com/chromium/src/+/b92ae006f9fcafbd31fea15459c9b704b506aee6

[kraush, 2017-03-22 15:12:49]

kraush@amazon.com changed reviewers:
+ changwan@chromium.org

[kraush, 2017-03-22 15:12:50]

Hi Changwan,

Can you take a look at this change to make a test more stable on some devices?

Thanks!
Holger

[Changwan Ryu, 2017-03-29 15:40:31]

On 2017/03/22 15:12:50, kraush wrote:
> Hi Changwan,
> 
> Can you take a look at this change to make a test more stable on some devices?
> 
> Thanks!
> Holger

Hi Holger,

Sorry I couldn't review this earlier.

There are chances that the test run order can be changed in the future, or when
test runner mandates it.
Could you try to make your fix contained in the test that caused the failure,
instead of fixing it in other places?

Also, would it be possible to turn off autocomplete for most tests? (and add one
to specifically test autocomplete + IME)

[kraush, 2017-03-29 15:49:29]

On 2017/03/29 15:40:31, Changwan Ryu wrote:
> On 2017/03/22 15:12:50, kraush wrote:
> > Hi Changwan,
> > 
> > Can you take a look at this change to make a test more stable on some
devices?
> > 
> > Thanks!
> > Holger
> 
> Hi Holger,
> 
> Sorry I couldn't review this earlier.
> 
> There are chances that the test run order can be changed in the future, or
when
> test runner mandates it.
> Could you try to make your fix contained in the test that caused the failure,
> instead of fixing it in other places?
> 
> Also, would it be possible to turn off autocomplete for most tests? (and add
one
> to specifically test autocomplete + IME)

Hi Changwan,

Thanks for your feedback!
I'm not sure I completely follow the first part of your comment.
This change is completely limited to altering code within
testKeyboardAppFinishesCompositionOnUnexpectedSelectionChange (which is the
failing test)
It is not relying on test execution order or anything else.

I can definitely alter other tests to also use non-autocomplete!
Would it make sense to rename the textareas to textarea_autocomplete and
textarea_no_autocomplete and use the respective ones in each test? (Depending on
whether or not they expect autocomplete to be used)
--> I'm actually not sure there are any tests that use autocomplete so far. If
not, I'd be happy to add one.

Let me know if that works for you.

[Changwan Ryu, 2017-03-29 15:58:43]

On 2017/03/29 15:49:29, kraush wrote:
> On 2017/03/29 15:40:31, Changwan Ryu wrote:
> > On 2017/03/22 15:12:50, kraush wrote:
> > > Hi Changwan,
> > > 
> > > Can you take a look at this change to make a test more stable on some
> devices?
> > > 
> > > Thanks!
> > > Holger
> > 
> > Hi Holger,
> > 
> > Sorry I couldn't review this earlier.
> > 
> > There are chances that the test run order can be changed in the future, or
> when
> > test runner mandates it.
> > Could you try to make your fix contained in the test that caused the
failure,
> > instead of fixing it in other places?
> > 
> > Also, would it be possible to turn off autocomplete for most tests? (and add
> one
> > to specifically test autocomplete + IME)
> 
> Hi Changwan,
> 
> Thanks for your feedback!
> I'm not sure I completely follow the first part of your comment.
> This change is completely limited to altering code within
> testKeyboardAppFinishesCompositionOnUnexpectedSelectionChange (which is the
> failing test)
> It is not relying on test execution order or anything else.
> 
> I can definitely alter other tests to also use non-autocomplete!
> Would it make sense to rename the textareas to textarea_autocomplete and
> textarea_no_autocomplete and use the respective ones in each test? (Depending
on
> whether or not they expect autocomplete to be used)
> --> I'm actually not sure there are any tests that use autocomplete so far. If
> not, I'd be happy to add one.
> 
> Let me know if that works for you.

lgtm

My apologies, I was in a rush, and read it wrong.

[kraush, 2017-03-29 15:59:50]

The CQ bit was checked by kraush@amazon.com

[commit-bot: I haz the power, 2017-03-29 16:00:40]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-29 16:20:07]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1490803190486520, "parent_rev":
"75820eb165cb62f61da28517bb90a44bc432aec6", "commit_rev":
"b92ae006f9fcafbd31fea15459c9b704b506aee6"}

[commit-bot: I haz the power, 2017-03-29 16:20:56]

Description was changed from

==========
Use Non-Autocomplete textarea in ImeTest

On some devices, the textarea will be busy executing
autocomplete code while clicking on the textarea in
ImeTest#testKeyboardAppFinishesCompositionOnUnexpectedSelectionChange
By using the textarea without autocomplete, this issue is
avoided while still testing the full intent of this test.
(The autocomplete is not essential to what the test tries
to accomplish)

BUG=704131
==========

to

==========
Use Non-Autocomplete textarea in ImeTest

On some devices, the textarea will be busy executing
autocomplete code while clicking on the textarea in
ImeTest#testKeyboardAppFinishesCompositionOnUnexpectedSelectionChange
By using the textarea without autocomplete, this issue is
avoided while still testing the full intent of this test.
(The autocomplete is not essential to what the test tries
to accomplish)

BUG=704131

Review-Url: https://codereview.chromium.org/2768463005
Cr-Commit-Position: refs/heads/master@{#460411}
Committed:
https://chromium.googlesource.com/chromium/src/+/b92ae006f9fcafbd31fea15459c9...
==========

[commit-bot: I haz the power, 2017-03-29 16:21:03]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/b92ae006f9fcafbd31fea15459c9...