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

Issue 845087 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug

Blocking:
issue 836254



Sign in to add a comment

CopyBetweenWindows FilesAppBrowserTest (nee FileManagerBrowserTest) is disabled all bots

Project Member Reported by noel@chromium.org, May 21 2018

Issue description

We should get the CopyBetweenWindows working again.  The test simply failed to click on the removable volume (mtp, for example), so the expected file-list of that removable volume was never displayed by Files.App doh.
 

Comment 1 by noel@chromium.org, May 21 2018

Blocking: 836254
Components: Platform>Apps>FileManager Tests>Flaky

Comment 2 by noel@chromium.org, May 21 2018

Cc: loyso@chromium.org sashab@chromium.org joelhockey@chromium.org lucmult@chromium.org

Comment 3 by sashab@chromium.org, May 21 2018

Labels: CrOSFilesCategory-Testing
Project Member

Comment 4 by bugdroid1@chromium.org, May 21 2018

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

commit bc6d0c2423f0cff0e84a22934d3dd8001cd3151f
Author: Noel Gordon <noel@chromium.org>
Date: Mon May 21 08:26:50 2018

Add ability to fill test USB or MTP volumes with entries, or not

When mounting a fake removable volume (MTP or USB), allow the test case
to control filling the volume with entries or not.

Add a new file display test for USB. Comment file_display.js some more,
plus minor code and git cl format tweaks.

FileDisplay is not supported on Mash: add Mash filter bot exclusions.

Test=browser_test --gtest_filter="FileDisplay/FilesApp*"

Tbr: fukino-san
Bug:  845087 ,836254
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I841f21d6b737813fc8a59574bfe10abfdf0b6e0d
Reviewed-on: https://chromium-review.googlesource.com/1067219
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560247}
[modify] https://crrev.com/bc6d0c2423f0cff0e84a22934d3dd8001cd3151f/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc
[modify] https://crrev.com/bc6d0c2423f0cff0e84a22934d3dd8001cd3151f/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc
[modify] https://crrev.com/bc6d0c2423f0cff0e84a22934d3dd8001cd3151f/testing/buildbot/filters/mash.browser_tests.filter
[modify] https://crrev.com/bc6d0c2423f0cff0e84a22934d3dd8001cd3151f/ui/file_manager/integration_tests/file_manager/file_display.js

Comment 5 by noel@chromium.org, May 21 2018

Labels: -Pri-3 Pri-1
Status: Started (was: Untriaged)
Watching bots again [1] waiting new data.  The DEBUG version test times were in 35-40 secs range before #4.  After #4, they're in the 20 secs range.

https://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=FileDialog%2FFilesApp

Project Member

Comment 6 by bugdroid1@chromium.org, May 22 2018

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

commit fee73bc36d89b65eb6cf73e5af68dd5cf916d1a2
Author: Noel Gordon <noel@chromium.org>
Date: Tue May 22 04:21:00 2018

Re-enable CopyBetweenWindows browser test in RELEASE and DEBUG

Test failed since it clicked the USB volume but did not check that the
correct entries were displayed (often the Drive entries were) [1].

Use the new "mountFakeUsbEmpty" command to better control the USB file
volume entries during test. Verify the expected entries are shown when
the USB volume is clicked, prior to a "copy between windows".

In the C++, add LOG(FATAL) to detect if a USB addEntries request would
be dropped on the floor (JS test called AddEntries before Mount).

Also, spin the run loop when creating or mounting removable entries to
mitigate (not fix) the potential for bot TIMEOUT under high load.

Start running these tests in RELEASE/DEBUG again to gather new data on
their speed and robustness on these bots.

[1] So these test got disabled even in RELEASE, sadly.

Test: browser_test --gtest_filter="CopyBetweenWindows/FilesApp*"
Bug:  845087 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I6aea094c597a5595477b8801bc9641f5b4744694
Reviewed-on: https://chromium-review.googlesource.com/1066255
Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560479}
[modify] https://crrev.com/fee73bc36d89b65eb6cf73e5af68dd5cf916d1a2/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc
[modify] https://crrev.com/fee73bc36d89b65eb6cf73e5af68dd5cf916d1a2/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc
[modify] https://crrev.com/fee73bc36d89b65eb6cf73e5af68dd5cf916d1a2/ui/file_manager/integration_tests/file_manager/copy_between_windows.js

Comment 8 by noel@chromium.org, May 22 2018

Owner: noel@chromium.org
Project Member

Comment 9 by bugdroid1@chromium.org, May 22 2018

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

commit 5faad695bed83ab761b8bbcfb1ba551184190613
Author: Noel Gordon <noel@chromium.org>
Date: Tue May 22 07:25:16 2018

Update browser test USB and MTP command detector

Follow-on from crrev.com/560247. No change in behavior, no new tests,
covered by existing tests.

Bug:  845087 
Change-Id: Id2e0876bd96d7e01576d286f5331a44caba01e57
Reviewed-on: https://chromium-review.googlesource.com/1068638
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560504}
[modify] https://crrev.com/5faad695bed83ab761b8bbcfb1ba551184190613/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc

Project Member

Comment 10 by bugdroid1@chromium.org, May 23 2018

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

commit 38d6629de4a0abd09127066f99eaccb1f08d0b96
Author: Noel Gordon <noel@chromium.org>
Date: Wed May 23 23:08:03 2018

Re-enable CopyBetweenWindows browser test in MSAN

Bug:  845087 
Change-Id: Ib454c78465ac7d76b93f7d66307dac74ddbbb2e3
Reviewed-on: https://chromium-review.googlesource.com/1067573
Reviewed-by: Stuart Langley <slangley@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561300}
[modify] https://crrev.com/38d6629de4a0abd09127066f99eaccb1f08d0b96/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc

Project Member

Comment 11 by bugdroid1@chromium.org, May 24 2018

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

commit afba31e06a9b634888fe0285c7ddc8c64c55e845
Author: Noel Gordon <noel@chromium.org>
Date: Thu May 24 10:30:18 2018

Re-enable FilesApp CopyBetweenWindows browser test all bots

Test working well in RELEASE/DEBUG/MSAN: now re-enable on ASAN

Bug:  845087 
Change-Id: I5d7d82c9eca3d0b3fbbfb58bd2d34050d089f8d7
Reviewed-on: https://chromium-review.googlesource.com/1071413
Reviewed-by: Stuart Langley <slangley@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561453}
[modify] https://crrev.com/afba31e06a9b634888fe0285c7ddc8c64c55e845/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc

Comment 12 Deleted

Comment 13 Deleted

Comment 14 by noel@chromium.org, May 25 2018

There is residual TIMEOUT-PASS on ASAN chrome-os.  Those bots are slow, the tests can't be broken up into smaller tests, but the flake rate is not large enough to concern me at this time, since ...

The flake cause is bot-load bumping into the fact that these tests doing enough at run-time to flirt with the bot's time-out.  We can either increase the bot time-out, or else find more test speed another way: see crbug.com/836254#c75

Sign in to add a comment