New issue
Advanced search Search tips

Issue 869240 link

Starred by 1 user

Issue metadata

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

Blocking:
issue 836254



Sign in to add a comment

FilesApp FSP providers.js integration test code needs some work

Project Member Reported by noel@chromium.org, Jul 31

Issue description

providers.js mounts mock FSP for integration tests.  Some work for M70 includes moving the providers menu in FIle app to a new location, and we should update the provider.js test code during that work:

- use consistent names in the "Test Provider" test extensions

- fix the test commentary

- improve test robustness, eg., the clicking/click testing could be improved to less dependent on the actual location or HTML menu structure (the current code looks like it could produce false positives during test)
 
Owner: noel@chromium.org
Status: Started (was: Available)
Blocking: 836254
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 31

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

commit d89d12df107d12edf88f6799336bab7d8db053d7
Author: Noel Gordon <noel@chromium.org>
Date: Tue Jul 31 05:05:39 2018

Make integration test extension manifest names consistent

Name various test extensions used in integration test consistently, in
particular, the test extensions used in the providers.js tests [1].

[1] these are the only integration testcases that explicitly depend on
test extension manifest name. Add a helper to return the name.

Bug:  869240 
Change-Id: I9d5ab74af9083201ab2c0173c2e505ec6f6e05be
Reviewed-on: https://chromium-review.googlesource.com/1154784
Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579316}
[modify] https://crrev.com/d89d12df107d12edf88f6799336bab7d8db053d7/ui/file_manager/integration_tests/audio_player_test_manifest.json
[modify] https://crrev.com/d89d12df107d12edf88f6799336bab7d8db053d7/ui/file_manager/integration_tests/file_manager/providers.js
[modify] https://crrev.com/d89d12df107d12edf88f6799336bab7d8db053d7/ui/file_manager/integration_tests/file_manager_test_manifest.json
[modify] https://crrev.com/d89d12df107d12edf88f6799336bab7d8db053d7/ui/file_manager/integration_tests/gallery_test_manifest.json
[modify] https://crrev.com/d89d12df107d12edf88f6799336bab7d8db053d7/ui/file_manager/integration_tests/testing_provider/background.js
[modify] https://crrev.com/d89d12df107d12edf88f6799336bab7d8db053d7/ui/file_manager/integration_tests/testing_provider/manifest.json
[modify] https://crrev.com/d89d12df107d12edf88f6799336bab7d8db053d7/ui/file_manager/integration_tests/testing_provider/manifest_multiple_mounts.json
[modify] https://crrev.com/d89d12df107d12edf88f6799336bab7d8db053d7/ui/file_manager/integration_tests/testing_provider/manifest_source_device.json
[modify] https://crrev.com/d89d12df107d12edf88f6799336bab7d8db053d7/ui/file_manager/integration_tests/testing_provider/manifest_source_file.json
[modify] https://crrev.com/d89d12df107d12edf88f6799336bab7d8db053d7/ui/file_manager/integration_tests/video_player_test_manifest.json

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 1

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

commit 65de167b0a621f73d4bb14b596af67b892afe33b
Author: Luciano Pacheco <lucmult@chromium.org>
Date: Wed Aug 01 00:32:13 2018

Move "Add new services" button from directory tree

Add "Add new services" menu item to Files app "gear menu" and change
the behavior to:

1. Go directly to Webstore in case there isn't any provider/FSP
installed.
2. Open "providers menu" when there is at least one provider/FSP
installed, which is the old behaviour.

Add a new command "new-service" to display the providers menu to be
able open it from the gear menu.

Change the gear menu item label to be "Add new service".

Fix My files tests because "Add new services" isn't displayed on
directory tree anymore.

Bug:  682356 ,  869240 
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: Ie9bf211c42a18b3717a72f40e57ceaade5958281
Reviewed-on: https://chromium-review.googlesource.com/1139944
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579639}
[modify] https://crrev.com/65de167b0a621f73d4bb14b596af67b892afe33b/chrome/app/chromeos_strings.grdp
[modify] https://crrev.com/65de167b0a621f73d4bb14b596af67b892afe33b/ui/file_manager/file_manager/foreground/js/file_manager.js
[modify] https://crrev.com/65de167b0a621f73d4bb14b596af67b892afe33b/ui/file_manager/file_manager/foreground/js/file_manager_commands.js
[modify] https://crrev.com/65de167b0a621f73d4bb14b596af67b892afe33b/ui/file_manager/file_manager/foreground/js/gear_menu_controller.js
[modify] https://crrev.com/65de167b0a621f73d4bb14b596af67b892afe33b/ui/file_manager/file_manager/foreground/js/ui/file_manager_ui.js
[modify] https://crrev.com/65de167b0a621f73d4bb14b596af67b892afe33b/ui/file_manager/file_manager/foreground/js/ui/gear_menu.js
[modify] https://crrev.com/65de167b0a621f73d4bb14b596af67b892afe33b/ui/file_manager/file_manager/main.html
[modify] https://crrev.com/65de167b0a621f73d4bb14b596af67b892afe33b/ui/file_manager/file_manager/test/js/strings.js
[modify] https://crrev.com/65de167b0a621f73d4bb14b596af67b892afe33b/ui/file_manager/integration_tests/file_manager/my_files.js
[modify] https://crrev.com/65de167b0a621f73d4bb14b596af67b892afe33b/ui/file_manager/integration_tests/file_manager/providers.js

Mostly done here, minor follow-up to #3, where Trent mentioned moving the appId into the "class" section.  Patch up ...
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 1

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

commit 7d376a29b4aaf33f2254d987302bea0f40fef56b
Author: Noel Gordon <noel@chromium.org>
Date: Wed Aug 01 02:35:42 2018

Move appId into the "class" section: aka the front

Bug:  869240 
Change-Id: Ie5350b73b21bf94912fea292261b4dea7382c511
Reviewed-on: https://chromium-review.googlesource.com/1157728
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579676}
[modify] https://crrev.com/7d376a29b4aaf33f2254d987302bea0f40fef56b/ui/file_manager/integration_tests/file_manager/providers.js

Labels: OS-Chrome
Status: Fixed (was: Started)
Project Member

Comment 8 by bugdroid1@chromium.org, Aug 8

Labels: merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a0b7e14a19bfb170a3acceb29fc0ea3157eb6cd2

commit a0b7e14a19bfb170a3acceb29fc0ea3157eb6cd2
Author: Luciano Pacheco <lucmult@chromium.org>
Date: Wed Aug 08 02:40:02 2018

Move "Add new services" button from directory tree

Add "Add new services" menu item to Files app "gear menu" and change
the behavior to:

1. Go directly to Webstore in case there isn't any provider/FSP
installed.
2. Open "providers menu" when there is at least one provider/FSP
installed, which is the old behaviour.

Add a new command "new-service" to display the providers menu to be
able open it from the gear menu.

Change the gear menu item label to be "Add new service".

Fix My files tests because "Add new services" isn't displayed on
directory tree anymore.

Bug:  682356 ,  869240 
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: Ie9bf211c42a18b3717a72f40e57ceaade5958281
Reviewed-on: https://chromium-review.googlesource.com/1139944
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#579639}(cherry picked from commit 65de167b0a621f73d4bb14b596af67b892afe33b)
Reviewed-on: https://chromium-review.googlesource.com/1166482
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#485}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/a0b7e14a19bfb170a3acceb29fc0ea3157eb6cd2/ui/file_manager/file_manager/foreground/js/file_manager.js
[modify] https://crrev.com/a0b7e14a19bfb170a3acceb29fc0ea3157eb6cd2/ui/file_manager/file_manager/foreground/js/file_manager_commands.js
[modify] https://crrev.com/a0b7e14a19bfb170a3acceb29fc0ea3157eb6cd2/ui/file_manager/file_manager/foreground/js/gear_menu_controller.js
[modify] https://crrev.com/a0b7e14a19bfb170a3acceb29fc0ea3157eb6cd2/ui/file_manager/file_manager/foreground/js/ui/file_manager_ui.js
[modify] https://crrev.com/a0b7e14a19bfb170a3acceb29fc0ea3157eb6cd2/ui/file_manager/file_manager/foreground/js/ui/gear_menu.js
[modify] https://crrev.com/a0b7e14a19bfb170a3acceb29fc0ea3157eb6cd2/ui/file_manager/file_manager/main.html
[modify] https://crrev.com/a0b7e14a19bfb170a3acceb29fc0ea3157eb6cd2/ui/file_manager/file_manager/test/js/strings.js
[modify] https://crrev.com/a0b7e14a19bfb170a3acceb29fc0ea3157eb6cd2/ui/file_manager/integration_tests/file_manager/my_files.js
[modify] https://crrev.com/a0b7e14a19bfb170a3acceb29fc0ea3157eb6cd2/ui/file_manager/integration_tests/file_manager/providers.js

Sign in to add a comment