New issue
Advanced search Search tips

Issue 876267 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 11
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocking:
issue 836254



Sign in to add a comment

CreateNewFolder/FilesAppBrowserTest selectCreateFolderDownloads flaky

Project Member Reported by noel@chromium.org, Aug 21

Issue description

CreateNewFolder/FilesAppBrowserTest has recently gone intermittently flakey on the slower bots MSAN/ASAN.

[23708:23708:0820/115051.830637:INFO:CONSOLE(0)] "Styling master document from stylesheets defined in HTML Imports is deprecated. Please refer to https://goo.gl/EGXzpw for possible migration paths.", source:  (0)
[23708:23708:0820/115054.580598:INFO:CONSOLE(156)] "at /file_manager/background.js:373:27: The number of file is 0. Not changed.", source: chrome-extension://oobinhbdbiehknkpbpejbbpdbkdjmoco/test_util.js (156)
[23708:23708:0820/115058.005944:INFO:CONSOLE(156)] "at /file_manager/background.js:373:27: The number of file is 0. Not changed.", source: chrome-extension://oobinhbdbiehknkpbpejbbpdbkdjmoco/test_util.js (156)
[23708:23708:0820/115107.658045:INFO:CONSOLE(156)] "at /file_manager/background.js:373:27: The number of file is 0. Not changed.", source: chrome-extension://oobinhbdbiehknkpbpejbbpdbkdjmoco/test_util.js (156)
[23708:23708:0820/115132.467308:INFO:CONSOLE(34422)] "getMetadata failure for: /New Folder", source: chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj/foreground/js/main_scripts.js (34422)
[23708:23708:0820/115136.453183:INFO:CONSOLE(156)] "at /file_manager/create_new_folder.js:172:12: Selected item is wrong. (actual: photos)", source: chrome-extension://oobinhbdbiehknkpbpejbbpdbkdjmoco/test_util.js (156)
[23708:23708:0820/115140.465118:INFO:CONSOLE(156)] "at /file_manager/create_new_folder.js:172:12: Selected item is wrong. (actual: photos)", source: chrome-extension://oobinhbdbiehknkpbpejbbpdbkdjmoco/test_util.js (156)
...

Seems the selection is sometimes wrong. There is a getMetaData failure on "New Folder" which might ore might nit be relevant.

We should modernize the CreateNewFolder tests, speed them and up etc, to help track down this intermittent selection fault.
 
KeyboardOperations/FilesAppBrowserTest.Test/renameNewFolderDownloads has recently been reported with selection failure during New Folder rename,  issue 874954 

The current issue has selection failure during New Folder rename too, the bots are trying to tell us something ...

The test system style recalculation avoidance change [1] was also landed in the last 7 days, FTR.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/1174087

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 22

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

commit 03c9916c378aa15631478ebe7e6537e351452bef
Author: Noel Gordon <noel@chromium.org>
Date: Wed Aug 22 06:55:08 2018

Speed up and modernize CreateNewFolder tests

 - Speed up setAndWaitUntilReady setup
   Only load the required basic entry set for the test, Downloads or
   Drive (not both), for speed.

 - Remove remoteCall.waitForElement(windowId, '#detail-table') lines
   Redundant because setAndWaitUntilReady() does that.

 - Remove #directory-tree [volume-type-for-testing="downloads"]
   Use the same CSS selector from used for Drive (for consistency).

 - Change selectCreateFolderDownloads steps
   Expand the directory tree before selecting the first item in the
   file-list, a change in behavior, but the order should not matter
   as discussed off-line.

Test: browser_tests --gtest_filter="CreateNewFolder/FilesApp*"
Bug:  876267 
Change-Id: I6de13b0dca83e5eb0372fde4680d8740606c4d84
Reviewed-on: https://chromium-review.googlesource.com/1183266
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584969}
[modify] https://crrev.com/03c9916c378aa15631478ebe7e6537e351452bef/ui/file_manager/integration_tests/file_manager/create_new_folder.js

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 22

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

commit 14121a40c83d7f7a746417ba72efee9a4e7d00e1
Author: Noel Gordon <noel@chromium.org>
Date: Wed Aug 22 11:35:56 2018

Modernize CreateNewFolder test expandRoot helper

Document all steps, Also verify the subtree contains the child name we
expect: 'photos' since the tests use BASIC_*_ENTRY_SETS. Move the vars
EXPAND_ICON and EXPANDED_SUBTREE internal to the helper where they are
used.

Test: browser_tests --gtest_filter="CreateNewFolder/FilesApp*"
Bug:  876267 
Change-Id: I43548e39dc0be408a503ebacc19765c3db424afe
Reviewed-on: https://chromium-review.googlesource.com/1183270
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584985}
[modify] https://crrev.com/14121a40c83d7f7a746417ba72efee9a4e7d00e1/ui/file_manager/integration_tests/file_manager/create_new_folder.js

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 22

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

commit 964d4e7d7d49a4e46ee7a2a53292aea904f8d0d0
Author: Noel Gordon <noel@chromium.org>
Date: Wed Aug 22 13:36:02 2018

Modernize CreateNewFolder selectFirstListItem helper

Document all steps. Use ArrowDown key to select a file list item (Ctrl
is not needed) and check its fakeKeyDown return value. Also rename the
helper to selectFirstFileListItem, and use class and id selectors when
querying for file list row item selection.

Test: browser_tests --gtest_filter="CreateNewFolder/FilesApp*"
Bug:  876267 
Change-Id: I230ca4a54069c79169eac7a44003d81cd57d3c64
Reviewed-on: https://chromium-review.googlesource.com/1183271
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584999}
[modify] https://crrev.com/964d4e7d7d49a4e46ee7a2a53292aea904f8d0d0/ui/file_manager/integration_tests/file_manager/create_new_folder.js

Project Member

Comment 6 by bugdroid1@chromium.org, Aug 23

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

commit 4c745fa22ba89556785b1861aa710aefd0a1b765
Author: Noel Gordon <noel@chromium.org>
Date: Thu Aug 23 04:07:07 2018

CreateNewFolder test: improve createNewFolder helper and related

Stop using navigateWithDirectoryTree in createNewFolder helper (return
value unchecked, generally false if checked).

All tests navigate the directory tree during test setup by clicking on
subtree's expand icon. However, the createFolderNestedDownloads() test
needs to do an extra step to navigate into sub-tree 'photos'. Since it
is the only test that needs this extra step, navigateWithDirectoryTree
can be used to do it, with a post-verify step to check that #file-list
updated to the expected visual state (no files shown).

Change createNewFolder API: navigateWithDirectoryTree gone so path etc
can go. Add a tree |selector| argument so createNewFolder can check if
that folders appear in the directory tree.

Update the createNewFolder internals: focus the file list, use precise
#file-list selectors when checking for [selected] [renaming] row, more
checks to confirm that only one file-list row is [selected] [renaming]
namely the New Folder entry, and verify the New Folder is "present" in
directory tree HTML content.

After the new folder is renamed, verify that its name is shown in file
list and also in the directory tree. Finally, fully test the file-list
selection state at the end of the test (the long way).

Bug:  876267 
Change-Id: I8693bdc43b95a4c3b510633697fa4b3d85e08051
Reviewed-on: https://chromium-review.googlesource.com/1186008
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585391}
[modify] https://crrev.com/4c745fa22ba89556785b1861aa710aefd0a1b765/ui/file_manager/integration_tests/file_manager/create_new_folder.js

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 23

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

commit 091271adb1d504d13958b42abab25b514aea893a
Author: Noel Gordon <noel@chromium.org>
Date: Thu Aug 23 08:54:23 2018

CreateNewFolder test: change windowId->appId

Most tests in the itegration system use |appId| to refer to the Files
app window and it hellps little to be different on that point.

Bug:  876267 
Change-Id: Ifc9f5065f2526dd0c7a14eee12a96ba304ec984a
Reviewed-on: https://chromium-review.googlesource.com/1186285
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585432}
[modify] https://crrev.com/091271adb1d504d13958b42abab25b514aea893a/ui/file_manager/integration_tests/file_manager/create_new_folder.js

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 24

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

commit cf502b5a2f7d78534095a5f03520b99276e4e647
Author: Noel Gordon <noel@chromium.org>
Date: Fri Aug 24 05:17:04 2018

Make the test system directory tree selectors const

TREEITEM_DRIVE and TREEITEM_DOWNLOADS are shared between various tests
but are var-defined in multiple places.

Multiple definitions are error-prone: choose one place and define them
as const therein (CreateNewFolder for now). Update folder_shortcuts.js
test to use this new TREEITEM_DRIVE definition, keyboard_operations.js
already uses the TREEITEM_DRIVE/DOWNLOADS definitions.

Test: browser_tests --gtest_filter="*/FilesApp*"
No-presubmit: true
Bug:  876267 
Change-Id: I7b86719a29a25e9d649c33edbe8af6cddae529bc
Reviewed-on: https://chromium-review.googlesource.com/1186390
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585696}
[modify] https://crrev.com/cf502b5a2f7d78534095a5f03520b99276e4e647/ui/file_manager/integration_tests/file_manager/create_new_folder.js
[modify] https://crrev.com/cf502b5a2f7d78534095a5f03520b99276e4e647/ui/file_manager/integration_tests/file_manager/folder_shortcuts.js

Project Member

Comment 9 by bugdroid1@chromium.org, Aug 24

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

commit 6b6a65f4dc6c2404fb18cf07a21f4c0ea244fa19
Author: Noel Gordon <noel@chromium.org>
Date: Fri Aug 24 05:29:28 2018

folder_shortcuts.js test: use const

More variables in this file are said to be const but are declared var.
Make the directory tree selectors const.

ENTRY_SET is too generic (not really helpful for code searching): make
it FOLDER_ENTRY_SET, also make it const. Ahhh DIRECTORY: be const till
I think of a better name.

Bug:  876267 
Change-Id: Ic0c1a25c4d019088cf73e45119081b4582c640c0
Reviewed-on: https://chromium-review.googlesource.com/1186521
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585700}
[modify] https://crrev.com/6b6a65f4dc6c2404fb18cf07a21f4c0ea244fa19/ui/file_manager/integration_tests/file_manager/folder_shortcuts.js

Project Member

Comment 10 by bugdroid1@chromium.org, Aug 27

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

commit 5e80db603e28244f76b95fdd788ee22ceb34657b
Author: Noel Gordon <noel@chromium.org>
Date: Mon Aug 27 00:58:28 2018

CreateNewFolder: use windowId in JsDoc comments

Comment only change, no change in behavior.

Bug:  876267 
Change-Id: Ia5d46aafb094ea8e01ba5ec40397742b4a2d0dc9
Reviewed-on: https://chromium-review.googlesource.com/1189243
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586176}
[modify] https://crrev.com/5e80db603e28244f76b95fdd788ee22ceb34657b/ui/file_manager/integration_tests/file_manager/create_new_folder.js

Blocking: 836254
Status: Fixed (was: Started)
Status: Started (was: Fixed)
Oh, one more thing to land here ...
Project Member

Comment 15 by bugdroid1@chromium.org, Aug 28

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

commit 54526fcdc5e94dd3ade1b9207e2845a5840b103a
Author: Noel Gordon <noel@chromium.org>
Date: Tue Aug 28 00:34:07 2018

CreateNewFolder tests: use string type for element query parameter

waitForElement() and waitForElementLost() expect a string type for the
element query parameter (according to their documentation).

Test: browser_tests --gtest_filter="CreateNewFolder/FilesApp*"
Bug:  876267 
Change-Id: Ica157ca4daed79eeaa65722ed98db7673d14065e
Reviewed-on: https://chromium-review.googlesource.com/1190043
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586522}
[modify] https://crrev.com/54526fcdc5e94dd3ade1b9207e2845a5840b103a/ui/file_manager/integration_tests/file_manager/create_new_folder.js

selectCreateFolderDownloads is not flaking anymore ...
selectCreateFolderDownloads.png
194 KB View Download
Status: Fixed (was: Started)
Status: Started (was: Fixed)
Adding a extra logging in case of future failure.  Patch coming ...
Project Member

Comment 20 by bugdroid1@chromium.org, Sep 11

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

commit ce4fa1f7c097d2e90a942555f1deeb65a94030c2
Author: Luciano Pacheco <lucmult@chromium.org>
Date: Tue Sep 11 06:33:19 2018

Add more info if create_new_folder.js test fails

In case of failure, debug/report the folder name in the log.

Test: browser_tests --gtest_filter="CreateNewFolder/FilesApp*"
Bug:  876267 
Change-Id: I1341c3f746506521a172829fdcf121cbe2030920
Reviewed-on: https://chromium-review.googlesource.com/1207787
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590215}
[modify] https://crrev.com/ce4fa1f7c097d2e90a942555f1deeb65a94030c2/ui/file_manager/integration_tests/file_manager/create_new_folder.js

Status: Fixed (was: Started)
Again link for reference: looking good to me, pointer out failures elsewhere in the chrome-os system ... see  issue 882293  for example.

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

Sign in to add a comment