New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 623442 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 3
Type: Bug



Sign in to add a comment

views_unittests Flaky textfield tests involving system clipboard (all platforms)

Project Member Reported by tapted@chromium.org, Jun 27 2016

Issue description

Chrome Version       : 53.0.2774.3

I'm scrutinizing views_unittests on Mac for https://codereview.chromium.org/1925943002/ and I've noticed a few tests that tend to fail once, but "always" succeed on retries. The same thing seems to happen on Linux/Win, but aren't causing havoc.

I think we should move the tests that involve the system clipboard to an interactive_ui_test target, or somehow mock out the system clipboard so that these tests are more robust when run in parallel. (Any alternative suggestions?)

The (possibly incomplete) list is:

1) ViewTest.TextfieldCutCopyPaste
2) TextfieldTest.ReadOnlyTest
3) TextfieldTest.PasswordTest
4) TextfieldTest.CutCopyPaste
5) TextfieldModelTest.Clipboard


* 1,2,3,4,5 - flaked on win_64 at https://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_rel_ng/builds/235724 -> https://chromium-swarm.appspot.com/user/task/2fa8502477e12b10
* 1,2,4 - flaked on win at https://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/245620 -> https://chromium-swarm.appspot.com/user/task/2fa84e6781a8c810
* 4 - flaked on linux at https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/252728 -> https://chromium-swarm.appspot.com/user/task/2fa84ae0a6b13510
* 3 - flaked on mac 10.10 at https://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_10.10_rel_ng/builds/103783 -> https://chromium-swarm.appspot.com/user/task/2fa84a0bd1434110

I scanned a few CrOS bots, but didn't spot any flakes there.


Output like the following (just showing first failure):

1) ViewTest.TextfieldCutCopyPaste
[ RUN      ] ViewTest.TextfieldCutCopyPaste
e:\b\build\slave\win\build\src\ui\views\view_unittest.cc(2035): error: Value of: result
  Actual: L"read only"
Expected: kNormalText
Which is: L"Normal"
[more]


2) TextfieldTest.ReadOnlyTest
e:\b\build\slave\win\build\src\ui\views\controls\textfield\textfield_unittest.cc(1557): error: Value of: GetClipboardText(ui::CLIPBOARD_TYPE_COPY_PASTE)
  Actual: L""
Expected: ASCIIToUTF16("Test")
Which is: L"Test"


3) TextfieldTest.PasswordTest
e:\b\build\slave\win\build\src\ui\views\controls\textfield\textfield_unittest.cc(927): error: Value of: GetClipboardText(ui::CLIPBOARD_TYPE_COPY_PASTE)
  Actual: L""
Expected: ASCIIToUTF16("foo")
Which is: L"foo"
[more]


4) TextfieldTest.CutCopyPaste
[ RUN      ] TextfieldTest.CutCopyPaste
e:\b\build\slave\win\build\src\ui\views\controls\textfield\textfield_unittest.cc(1841): error: Value of: GetClipboardText(ui::CLIPBOARD_TYPE_COPY_PASTE)
  Actual: L""
Expected: ASCIIToUTF16("123")
Which is: L"123"
[more]


5) TextfieldModelTest.Clipboard
e:\b\build\slave\win\build\src\ui\views\controls\textfield\textfield_model_unittest.cc(498): error: Value of: clipboard_text
  Actual: L""
Expected: initial_clipboard_text
Which is: L"initial text"
[lots more]
 

Comment 1 by msw@chromium.org, Jun 27 2016

Cc: e...@chromium.org
Mocking the clipboard might be nice for unit tests, but we'll want at least one interactive_ui_test end-to-end test with the real clipboard.

I'm not sure if mocking the clipboard at a lower level is easy (don't know ScopedClipboardWriter/ClipboardTest well):
ScopedClipboardWriter::WriteTextOrURL
ScopedClipboardWriter::WriteText
TextfieldModel::Copy
Textfield::Copy

You could potentially set a custom TextfieldController in tests that acts as a clipboard itself; you'd just make OnAfterCutOrCopy and OnAfterPaste persist and apply the text for that one Textfield instance; but those aren't called on failed actions (cutting from a read-only textfield), and it doesn't support cross-object clipboard use.

Maybe the best answer is to make them all interactive_ui_tests...

Comment 2 by e...@chromium.org, Jun 28 2016

Mocking the clipboard actually became possible just recently. Because mus has to override the OS's clipboard, you can supply your own clipboard implementation with Clipboard::SetClipboardForCurrentThread().

Comment 3 by tapted@chromium.org, Jun 30 2016

Owner: tapted@chromium.org
Status: Assigned (was: Available)
Another for the collection:

[ RUN      ] TextfieldTest.ContextMenuDisplayTest
../../ui/views/controls/textfield/textfield_unittest.cc:639: Failure
Expected: (GetClipboardText(ui::CLIPBOARD_TYPE_COPY_PASTE).empty()) != (menu->IsEnabledAt(4 )), actual: true vs true
[  FAILED  ] TextfieldTest.ContextMenuDisplayTest (33 ms)


I'll look into mocking with Clipboard::SetClipboardForCurrentThread() - thanks for the pointers!
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 16 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/80f0522d703aaf59ccbe0cb53f1eb8574187a265

commit 80f0522d703aaf59ccbe0cb53f1eb8574187a265
Author: karandeepb <karandeepb@chromium.org>
Date: Wed Nov 16 23:12:23 2016

Views:: Use TestClipboard in ScopedViewsTestHelper.

Currently the behavior of any views_unittest which depends on clipboard is
flaky. This is because the Clipboard is a global/shared resource. Hence when
multiple tests are run in parallel, one test can modify the clipboard state,
causing another test to flake. This CL sets up a TestClipboard instance in the
ScopedViewsTestHelper. This should fix various flaky views_unittests, for
example:
- ViewTest.TextfieldCutCopyPaste
- TextfieldTest.ReadOnlyTest
- TextfieldTest.CutCopyPaste
- TextfieldTest.PasswordTest
etc.

The following tests (disabled due to flaky selection clipboard behavior) are re-
enabled:
- LabelSelectionTest.SelectionClipboard
- TextfieldTest.SelectionClipboard
- TextfieldTest.SelectionClipboard_Password

BUG= 623442 ,  396477 

Review-Url: https://codereview.chromium.org/2499303003
Cr-Commit-Position: refs/heads/master@{#432659}

[modify] https://crrev.com/80f0522d703aaf59ccbe0cb53f1eb8574187a265/ui/views/controls/label_unittest.cc
[modify] https://crrev.com/80f0522d703aaf59ccbe0cb53f1eb8574187a265/ui/views/controls/textfield/textfield_unittest.cc
[modify] https://crrev.com/80f0522d703aaf59ccbe0cb53f1eb8574187a265/ui/views/test/scoped_views_test_helper.cc

Components: Tests>Disabled
Labels: Test-Disabled

Sign in to add a comment