Issue metadata
Sign in to add a comment
|
Add more tests for My Files on navigation tree. |
||||||||||||||||||||
Issue descriptionInitial patch for My Files has only basic coverage for showing My Files and the expected order. More tests should be added.
,
Jul 4
One observed regression that should be tested: 1) Assume that we don't have any folders in Downloads. 2) Open Files app and click "Downloads" in left pane. 3) Create "New folder" from the context menu on right pane. 4) Observe the "Downloads" on left pane. Expected: Downloads volumes should have '>' mark to expand the tree. Actual: Nothing happens on left pane.
,
Jul 4
,
Jul 4
Marking this issue as a regression for the issue on comment #2. Feel free to separate a bug for it, or use this one to fix it.
,
Jul 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f79c17b68e41c6a125b8798709431fd5363980fa commit f79c17b68e41c6a125b8798709431fd5363980fa Author: Luciano Pacheco <lucmult@chromium.org> Date: Wed Jul 11 03:41:18 2018 Fix My Files to display its children on right hand side Also fix to be able to open Downloads from RHS. Add |name| attribute for crostini FakeEntry so it can display it on RHS. Add |file-name| attribute on FileTable rows to make it easier to select a single row on tests. Fix crash due to DCHECK on getFileTasksInternal::FileManagerPrivateInternalGetFileTasksFunction where it would fail with entries being empty when we send an array with only FilesAppEntry instances, because the API binding discards objects that aren't native Entry. So we remove non-native entries before calling |FileManagerPrivate.getFileTasks|. Change DirectoryContentScanner to not check for FakeEntry but instead check for the availability of the desired method |createReader| since FilesAppEntry also implements it even though they're non-native (aka FakeEntry). This fixes My Files to display its children on RHS. Change FileFilter to check for attributes existence (filesystem and fullPath) before operating on it. This fixes crostini FakeEntry to be able to be displayed on RHS. Change DirectoryModel to check for non-null VolumeInfo. This fixes this function running for My Files and crostini entries. Bug: 846587 , 857335 Cq-Include-Trybots: luci.chromium.try:closure_compilation Change-Id: Id3cfd1e957db57114c97de1d34a10494dcaaeedb Reviewed-on: https://chromium-review.googlesource.com/1130559 Commit-Queue: Luciano Pacheco <lucmult@chromium.org> Reviewed-by: Sasha Morrissey <sashab@chromium.org> Cr-Commit-Position: refs/heads/master@{#574060} [modify] https://crrev.com/f79c17b68e41c6a125b8798709431fd5363980fa/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc [modify] https://crrev.com/f79c17b68e41c6a125b8798709431fd5363980fa/ui/file_manager/file_manager/background/js/launcher_search.js [modify] https://crrev.com/f79c17b68e41c6a125b8798709431fd5363980fa/ui/file_manager/file_manager/common/js/util.js [modify] https://crrev.com/f79c17b68e41c6a125b8798709431fd5363980fa/ui/file_manager/file_manager/foreground/js/directory_contents.js [modify] https://crrev.com/f79c17b68e41c6a125b8798709431fd5363980fa/ui/file_manager/file_manager/foreground/js/directory_model.js [modify] https://crrev.com/f79c17b68e41c6a125b8798709431fd5363980fa/ui/file_manager/file_manager/foreground/js/file_manager.js [modify] https://crrev.com/f79c17b68e41c6a125b8798709431fd5363980fa/ui/file_manager/file_manager/foreground/js/file_tasks.js [modify] https://crrev.com/f79c17b68e41c6a125b8798709431fd5363980fa/ui/file_manager/file_manager/foreground/js/ui/file_table.js [modify] https://crrev.com/f79c17b68e41c6a125b8798709431fd5363980fa/ui/file_manager/integration_tests/file_manager/my_files.js
,
Jul 12
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9a8460b3e49c7b9ee03dcf56af71a0d76c0f60e4 commit 9a8460b3e49c7b9ee03dcf56af71a0d76c0f60e4 Author: Luciano Pacheco <lucmult@chromium.org> Date: Thu Jul 12 07:12:07 2018 Change test to use full-path-for-testing attribute selector This makes this test compatible with directory tree having My Files or not, and makes the test more readable too. This is a preparation to enable new navigation (with My Files) by default. Bug: 857335 No-presubmit: true Cq-Include-Trybots: luci.chromium.try:closure_compilation Change-Id: Ia35bc2cc7c91e0492f0158990bfc397a6b57da21 Reviewed-on: https://chromium-review.googlesource.com/1133610 Commit-Queue: Noel Gordon <noel@chromium.org> Reviewed-by: Noel Gordon <noel@chromium.org> Cr-Commit-Position: refs/heads/master@{#574500} [modify] https://crrev.com/9a8460b3e49c7b9ee03dcf56af71a0d76c0f60e4/ui/file_manager/integration_tests/file_manager/context_menu.js [modify] https://crrev.com/9a8460b3e49c7b9ee03dcf56af71a0d76c0f60e4/ui/file_manager/integration_tests/file_manager/create_new_folder.js [modify] https://crrev.com/9a8460b3e49c7b9ee03dcf56af71a0d76c0f60e4/ui/file_manager/integration_tests/file_manager/folder_shortcuts.js
,
Jul 12
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/76bc398de31290a2d97ee6b7ea804a55d1d5451d commit 76bc398de31290a2d97ee6b7ea804a55d1d5451d Author: Luciano Pacheco <lucmult@chromium.org> Date: Thu Jul 12 08:27:24 2018 Add new methods to EntryList and tests Change the logic to add entries to My Files to account for volume being mounted/unmounted. This fixes some broken tests when running with the flag enabled by default, so test to cover this is on crrev.com/c/1131025. Fix test getMetadata function name. Bug: 857335 Cq-Include-Trybots: luci.chromium.try:closure_compilation Change-Id: Id056b0e0d5c77a07c2561f6566bfc99111864917 Reviewed-on: https://chromium-review.googlesource.com/1133613 Commit-Queue: Luciano Pacheco <lucmult@chromium.org> Reviewed-by: Noel Gordon <noel@chromium.org> Cr-Commit-Position: refs/heads/master@{#574505} [modify] https://crrev.com/76bc398de31290a2d97ee6b7ea804a55d1d5451d/ui/file_manager/file_manager/common/js/files_app_entry_types.js [modify] https://crrev.com/76bc398de31290a2d97ee6b7ea804a55d1d5451d/ui/file_manager/file_manager/common/js/files_app_entry_types_unittest.html [modify] https://crrev.com/76bc398de31290a2d97ee6b7ea804a55d1d5451d/ui/file_manager/file_manager/common/js/files_app_entry_types_unittest.js [modify] https://crrev.com/76bc398de31290a2d97ee6b7ea804a55d1d5451d/ui/file_manager/file_manager/foreground/js/navigation_list_model.js [modify] https://crrev.com/76bc398de31290a2d97ee6b7ea804a55d1d5451d/ui/file_manager/file_manager/foreground/js/navigation_list_model_unittest.js
,
Jul 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f129fcdbf7d556bdb09279b27915ff0856bc6f42 commit f129fcdbf7d556bdb09279b27915ff0856bc6f42 Author: Luciano Pacheco <lucmult@chromium.org> Date: Fri Jul 13 01:46:28 2018 Fix crostini DirectoryItem type when using My Files DirectoryItem created without a specific NavigationModelItem defaults to SubDirectoryItem. Change DirectoryItem to check if entry has a NavigationModelItem to generate the appropriate derived "item" type from DirectoryItem. In particular, crostini will generate FakeItem. This fixes issue where crostini wouldn't mount because the trigger to mount is on FakeItem. Change FakeItem to have updateSharedStatusIcon as no-op, since it doesn't have a shared state. This fixes an error where DirectoryItem calls updateSharedStatusIcon on its sub-items, before FakeItem was used only as top-level so nobody was calling updateSharedStatusIcon. Change DirectoryTree to call EntryList to update its sub-directories since those sub-directories are usually volumes that need to update themselves. Add test createFolderNestedDownloads to check that EntryListItem call to updateSubDirectories doesn't have to be recursive. Change SubDirectoryItem to add "volume-type-for-testing" in test to fix tests using this selector when running with My Files enabled if the item is a volume. Bug: 857335 Cq-Include-Trybots: luci.chromium.try:closure_compilation Change-Id: I8a579b9c754143d9ba8322aba9014e82a3ac1757 Reviewed-on: https://chromium-review.googlesource.com/1133616 Commit-Queue: Luciano Pacheco <lucmult@chromium.org> Reviewed-by: Noel Gordon <noel@chromium.org> Cr-Commit-Position: refs/heads/master@{#574811} [modify] https://crrev.com/f129fcdbf7d556bdb09279b27915ff0856bc6f42/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc [modify] https://crrev.com/f129fcdbf7d556bdb09279b27915ff0856bc6f42/ui/file_manager/file_manager/foreground/js/ui/directory_tree.js [modify] https://crrev.com/f129fcdbf7d556bdb09279b27915ff0856bc6f42/ui/file_manager/integration_tests/file_manager/create_new_folder.js [modify] https://crrev.com/f129fcdbf7d556bdb09279b27915ff0856bc6f42/ui/file_manager/integration_tests/file_manager/directory_tree_context_menu.js [modify] https://crrev.com/f129fcdbf7d556bdb09279b27915ff0856bc6f42/ui/file_manager/integration_tests/file_manager/my_files.js
,
Jul 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a84752d7a1dfa557b92f380e91be254f0c27f73d commit a84752d7a1dfa557b92f380e91be254f0c27f73d Author: Luciano Pacheco <lucmult@chromium.org> Date: Fri Jul 13 13:25:08 2018 Changes to make the tests work with and without My Files Change DirectoryTree.updateTreeByEntry_ to call updateItemByEntry on its sub-items, to fix folder updates within EntryListItem/My Files. Add MY_FILES_ROOT_LABEL to static translation strings.js for UI tests. Also change getBreadcrumbsPath result to accept the expectedPath with or without My Files prefix so tests can pass with/without My Files. Bug: 857335 Cq-Include-Trybots: luci.chromium.try:closure_compilation Change-Id: I76b85c75374190403ed1301f4021b5ec725d9d00 Reviewed-on: https://chromium-review.googlesource.com/1136340 Commit-Queue: Luciano Pacheco <lucmult@chromium.org> Reviewed-by: Noel Gordon <noel@chromium.org> Cr-Commit-Position: refs/heads/master@{#574896} [modify] https://crrev.com/a84752d7a1dfa557b92f380e91be254f0c27f73d/ui/file_manager/file_manager/foreground/js/ui/directory_tree.js [modify] https://crrev.com/a84752d7a1dfa557b92f380e91be254f0c27f73d/ui/file_manager/file_manager/test/js/strings.js [modify] https://crrev.com/a84752d7a1dfa557b92f380e91be254f0c27f73d/ui/file_manager/integration_tests/remote_call.js
,
Jul 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/294f19fc3c16bf13983f85e8e5e1c7c14c4be554 commit 294f19fc3c16bf13983f85e8e5e1c7c14c4be554 Author: Luciano Pacheco <lucmult@chromium.org> Date: Fri Jul 13 16:19:06 2018 Enable new-files-app-navigation by default This CL reverses the value of the feature flag to mean enabled, and fixes all tests to pass with the new My Files on navigation menu. Change MyFiles tests to not append the flag anymore, because when the flag is provided it now disables "My Files" :) Fix navigation_list_model_unittest.js to pass flag true/false on NavigationListModel with the desired flag state. Stop calling "notifyEntriesChanged" on DirectoryTree.updateSharedStatusIcon, because this causes the cache invalidation, which was causing some metadata model to be invalidated before being displayed on file list, in particular the modificationTime column on file list. crbug.com/857343 tracks further investigation if this call was really still needed. Change folder_shortcuts.js to account to new My Files on directory tree, because it changes the keyboard shortcut to select top-level directories. Also fix and improve some comments. Change tab_index.js to account to My Files on breadcrumbs being focused during tab cycle. This requires adding an id on breadcrumbs. Bug: 857335 , 862897 Cq-Include-Trybots: luci.chromium.try:closure_compilation Change-Id: Iddb2545772e62c0e8adebd8b278bd3a5765f5040 Reviewed-on: https://chromium-review.googlesource.com/1131025 Reviewed-by: Noel Gordon <noel@chromium.org> Commit-Queue: Luciano Pacheco <lucmult@chromium.org> Cr-Commit-Position: refs/heads/master@{#574939} [modify] https://crrev.com/294f19fc3c16bf13983f85e8e5e1c7c14c4be554/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc [modify] https://crrev.com/294f19fc3c16bf13983f85e8e5e1c7c14c4be554/ui/file_manager/file_manager/foreground/js/navigation_list_model.js [modify] https://crrev.com/294f19fc3c16bf13983f85e8e5e1c7c14c4be554/ui/file_manager/file_manager/foreground/js/navigation_list_model_unittest.js [modify] https://crrev.com/294f19fc3c16bf13983f85e8e5e1c7c14c4be554/ui/file_manager/file_manager/foreground/js/ui/directory_tree.js [modify] https://crrev.com/294f19fc3c16bf13983f85e8e5e1c7c14c4be554/ui/file_manager/file_manager/foreground/js/ui/location_line.js [modify] https://crrev.com/294f19fc3c16bf13983f85e8e5e1c7c14c4be554/ui/file_manager/integration_tests/file_manager/folder_shortcuts.js [modify] https://crrev.com/294f19fc3c16bf13983f85e8e5e1c7c14c4be554/ui/file_manager/integration_tests/file_manager/tab_index.js
,
Jul 16
Since commit above all tests are running with My Files. The condition on comment #2 is covered by tests: ui/file_manager/integration_tests/file_manager/create_new_folder.js Which I added 1 more tests to double check that it would work with nested folders, as we weren't 100% sure during code review. |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by noel@chromium.org
, Jun 28 2018