Closure compile import_controller_unittest |
||
Issue description
- fix dependent FilesApp foreground BUILD rules
- they lack backend externs_list= dependencies rules but they use
backend classes: Crosini, ProgressCenter, and so on
- some BUILD rules include the world, aka ui:file_manager_ui, but
they neither need nor use the world, so prune the dependencies
down to the actual things needed
- fix lack of externs file defines & decls for Importer components
- externs for MediaImportHandler are half-baked, wrong, missing
- fix brokeness, and at least make them right
- adding full externs for the various components is interesting,
but is a better subject of future work
- background mock_media_scanner foreground test visible
- allow this class to be used in the foreground unittests, and
only one test needs it (the subject of this bug)
- make the import_controller_unittest.js Closure-sane
- this file is woefully out-of-date and a reason why we need
to enusre our code is continously Closure checked
- it is full of @types for an "impoter." class and similar
- spelling mistakes in the @type defines "ImportControler"
- it is full of undefined @types annotations
- the @type class / type referred to no longer exist
- defines @return on functions that have no return value
- finally, remove the current html unittest
- closure compile the code and auto-generate the unittest
,
Dec 12
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/620164346f4488cdb1604b19e82ae65a09d268e3 commit 620164346f4488cdb1604b19e82ae65a09d268e3 Author: Noel Gordon <noel@chromium.org> Date: Wed Dec 12 04:06:13 2018 Fix foreground file_manager_commands and import_controller BUILD rules - ImportController @types refer to {CommandHandlerDeps} - add externs_list deps UI externs/command_handler_deps.js - And the same for FileManagerCommands No change in test behavior. Bug: 914197 Change-Id: I9f71b677f22961d0e14ac55834ca0a9b0cea180a Reviewed-on: https://chromium-review.googlesource.com/c/1373051 Reviewed-by: Sam McNally <sammc@chromium.org> Commit-Queue: Noel Gordon <noel@chromium.org> Cr-Commit-Position: refs/heads/master@{#615810} [modify] https://crrev.com/620164346f4488cdb1604b19e82ae65a09d268e3/ui/file_manager/file_manager/foreground/js/BUILD.gn
,
Dec 12
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4a5a88b153d150440d511f513f8cbf25e34114c8 commit 4a5a88b153d150440d511f513f8cbf25e34114c8 Author: Noel Gordon <noel@chromium.org> Date: Wed Dec 12 05:06:21 2018 Fix foreground file_transfer_controller BUILD rule - FileTransferController uses ProgressCenter in its contructor - add externs_list deps to externs backend ProgressCenter No change in test behavior. Bug: 914197 Change-Id: I12d91732edd9fec52c44af54d9c88a6e9f98719a Reviewed-on: https://chromium-review.googlesource.com/c/1373050 Reviewed-by: Sam McNally <sammc@chromium.org> Reviewed-by: Stuart Langley <slangley@chromium.org> Commit-Queue: Noel Gordon <noel@chromium.org> Cr-Commit-Position: refs/heads/master@{#615815} [modify] https://crrev.com/4a5a88b153d150440d511f513f8cbf25e34114c8/ui/file_manager/file_manager/foreground/js/BUILD.gn
,
Dec 12
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/aa221f1b03c3bd2d01957f12f2922cf73a0a6cda commit aa221f1b03c3bd2d01957f12f2922cf73a0a6cda Author: Noel Gordon <noel@chromium.org> Date: Wed Dec 12 13:17:14 2018 Make mock_media_scanner available to foreground tests Foreground import_controller_unittest needs a mock MediaScanner. Allow background {MockMediaScanner} to be used by foreground tests. No change in test behavior. Tbr: slangley@ Bug: 914197 Change-Id: I146bff5e4ef5f12df5a060b3efedb1c2e1ea378e Reviewed-on: https://chromium-review.googlesource.com/c/1373589 Reviewed-by: Noel Gordon <noel@chromium.org> Commit-Queue: Noel Gordon <noel@chromium.org> Cr-Commit-Position: refs/heads/master@{#615870} [modify] https://crrev.com/aa221f1b03c3bd2d01957f12f2922cf73a0a6cda/ui/file_manager/file_manager/background/js/BUILD.gn
,
Dec 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/98b9f1efa71855c361059ecbf9bb8e598bc6fda5 commit 98b9f1efa71855c361059ecbf9bb8e598bc6fda5 Author: Noel Gordon <noel@chromium.org> Date: Thu Dec 13 01:37:11 2018 Add background externs media_import_handler.js to the Closure BUILD This change adds media_import_handler.js to the Closure BUILD and type checks it and also the externs it uses. Immediate result: many Closure compile errors since the types it uses do not have an externs file, or the constructor signatures are incomplete or just plain wrong [1]. [1] importer.MediaImportHandler.ImportTask @extends TaskQueue.BaseTask but the constructor signature in the externs file does not (Closure is not silly, and spots that error). Next issue: TaskQueue.* doesn't have an externs file, which one might use to fix the first error. Typical fix would be to use Impl pattern, add three new externs files, etc. The result would be a large, intrusive change. Simpler, type-safe methods are sought and are readily available. Closure externs can forward declare dependent class types, even define their constructors, to allow the constructor signature of the class of interest to be fully defined, complete with the dependent @param types that Closure can enforce. Use this fact in externs media_import_handler.js: forward declare, and define the constructors / interfaces as needed, of the dependent types namely, TaskQueue.*, DispositionChecker.*, that do not have an externs file. Define importer.MediaImportHandler with @params for Closure type checking. What about the implementations? Simply update the implementation JSDoc with one additional annotation line: @return undefined With that, Closure type-checks the constructor definition by comparing it with the signatures defined in the externs file, and concludes that it does match, that it does not "re-define" any externs file types. No change in test behavior. Bug: 914197 Change-Id: I15c02cd4ab83c19bef1882e77702e975fa1fdc2e Reviewed-on: https://chromium-review.googlesource.com/c/1373893 Reviewed-by: Stuart Langley <slangley@chromium.org> Commit-Queue: Noel Gordon <noel@chromium.org> Cr-Commit-Position: refs/heads/master@{#616151} [modify] https://crrev.com/98b9f1efa71855c361059ecbf9bb8e598bc6fda5/ui/file_manager/externs/background/import_runner.js [modify] https://crrev.com/98b9f1efa71855c361059ecbf9bb8e598bc6fda5/ui/file_manager/externs/background/media_import_handler.js [modify] https://crrev.com/98b9f1efa71855c361059ecbf9bb8e598bc6fda5/ui/file_manager/file_manager/background/js/BUILD.gn [modify] https://crrev.com/98b9f1efa71855c361059ecbf9bb8e598bc6fda5/ui/file_manager/file_manager/background/js/duplicate_finder.js [modify] https://crrev.com/98b9f1efa71855c361059ecbf9bb8e598bc6fda5/ui/file_manager/file_manager/background/js/media_import_handler.js [modify] https://crrev.com/98b9f1efa71855c361059ecbf9bb8e598bc6fda5/ui/file_manager/file_manager/background/js/task_queue.js
,
Dec 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d54599dd95e2ed173adacd3bea194ba0cf9503e1 commit d54599dd95e2ed173adacd3bea194ba0cf9503e1 Author: Noel Gordon <noel@chromium.org> Date: Thu Dec 13 03:31:56 2018 import_controller_unittest: add a default metadata helper Add a default metadata helper routine, and use it at call sites. Bug: 914197 Change-Id: Ib1de53438db145e5f266db92cce35bef211c83f9 Reviewed-on: https://chromium-review.googlesource.com/c/1375170 Reviewed-by: Sam McNally <sammc@chromium.org> Commit-Queue: Noel Gordon <noel@chromium.org> Cr-Commit-Position: refs/heads/master@{#616193} [modify] https://crrev.com/d54599dd95e2ed173adacd3bea194ba0cf9503e1/ui/file_manager/file_manager/foreground/js/import_controller_unittest.js
,
Dec 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9fdbed2fa2e6b03ce7f34a8a9bcdbe9fbec30ceb commit 9fdbed2fa2e6b03ce7f34a8a9bcdbe9fbec30ceb Author: Noel Gordon <noel@chromium.org> Date: Thu Dec 13 10:28:57 2018 import_controller_unittest: clean-up preamble and SetUp - add standard JSDoc used elsewhere for window.metrics - use local variable testFileSystem, to exchange 2 lines for 3 - volumeManager.getCurrentProfileVolumeInfo can return null so add an assert that it's never null in these tests No change in test behavior. Tbr: sammc@ Bug: 914197 Change-Id: I2ea6314b57f55224f7c26050591bd551b6f56bb8 Reviewed-on: https://chromium-review.googlesource.com/c/1375199 Reviewed-by: Noel Gordon <noel@chromium.org> Commit-Queue: Noel Gordon <noel@chromium.org> Cr-Commit-Position: refs/heads/master@{#616262} [modify] https://crrev.com/9fdbed2fa2e6b03ce7f34a8a9bcdbe9fbec30ceb/ui/file_manager/file_manager/foreground/js/import_controller_unittest.js
,
Dec 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6ca6800c1b1886228c03b465823e5f88444a7f86 commit 6ca6800c1b1886228c03b465823e5f88444a7f86 Author: Noel Gordon <noel@chromium.org> Date: Thu Dec 13 14:03:53 2018 import_controller_unittest: spell correct, remove, update @types - spell correct impoter => importer - remove @param {!DirectoryEntry} destination - the affected functions don't have parameters - fix obvious @type errors - event listener functions receive an {!Event} - arrays are @type {!Array<whatever>} for sure - remove importer.CommandUpdate - this @type does not exist in the code base - importer.ImportControler => importer.ImportController - add TODO to determine class @override types - auto-format the code Bug: 914197 Change-Id: Ib225ed6030e70c0676f04f4f730e72e6b48eec4b Reviewed-on: https://chromium-review.googlesource.com/c/1375201 Commit-Queue: Noel Gordon <noel@chromium.org> Reviewed-by: Sam McNally <sammc@chromium.org> Cr-Commit-Position: refs/heads/master@{#616299} [modify] https://crrev.com/6ca6800c1b1886228c03b465823e5f88444a7f86/ui/file_manager/file_manager/foreground/js/import_controller_unittest.js
,
Dec 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7935ad7fcaaed7392a7c0efff7625ade0354d045 commit 7935ad7fcaaed7392a7c0efff7625ade0354d045 Author: Noel Gordon <noel@chromium.org> Date: Thu Dec 13 23:46:43 2018 Closure compile import_controller_unittest - remove import_controller_unittest.html - add BUILD rules to auto-generate and compile this unittest - update comments, remove unused functions, code re-format - fix all Closure compile errors No change in test behavior. Bug: 914197 Change-Id: Ida12573fc640dcc97325b1240bedbbf970ff1f35 Reviewed-on: https://chromium-review.googlesource.com/c/1375301 Reviewed-by: Stuart Langley <slangley@chromium.org> Commit-Queue: Noel Gordon <noel@chromium.org> Cr-Commit-Position: refs/heads/master@{#616501} [modify] https://crrev.com/7935ad7fcaaed7392a7c0efff7625ade0354d045/chrome/browser/chromeos/file_manager/file_manager_jstest.cc [modify] https://crrev.com/7935ad7fcaaed7392a7c0efff7625ade0354d045/ui/file_manager/file_manager/foreground/js/BUILD.gn [delete] https://crrev.com/125a005b04c3a68f5c763df5abfa661b3ae70292/ui/file_manager/file_manager/foreground/js/import_controller_unittest.html [modify] https://crrev.com/7935ad7fcaaed7392a7c0efff7625ade0354d045/ui/file_manager/file_manager/foreground/js/import_controller_unittest.js
,
Dec 14
|
||
►
Sign in to add a comment |
||
Comment 1 by bugdroid1@chromium.org
, Dec 12