New issue
Advanced search Search tips

Issue 911024 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 4
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug

Blocking:
issue 860355



Sign in to add a comment

Prepare Crostini and its unittest for Closure compilation

Project Member Reported by noel@chromium.org, Dec 3

Issue description

 - add Closure @types to the unittest vars

 - make Crostini externs file a @interface
  - bring it up-to-date with the current implementation methods
 - define CrostiniImpl that @implements the extern file definition

 - add MockCrostini for foreground unittests


 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 3

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

commit 4ca8b95867881758c342c18d5b306fe77104f564
Author: Noel Gordon <noel@chromium.org>
Date: Mon Dec 03 12:42:22 2018

Prepare to Closure compile Crostini unittest

 - add Closure @type annotations to unittest JS types
 - add setUp(), add setDriveFsEnabled() helpers
 - prefix test cases with a descriptive comment

No change in test behavior, no new tests.

Bug:  911024 
Change-Id: Iaa1bcf5b4ab00d7e2c89875361ff14b1dee03a00
Reviewed-on: https://chromium-review.googlesource.com/c/1358316
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613058}
[modify] https://crrev.com/4ca8b95867881758c342c18d5b306fe77104f564/ui/file_manager/file_manager/background/js/crostini_unittest.js

Project Member

Comment 2 by bugdroid1@chromium.org, Dec 4

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

commit 49c3b5ddad77fcf536f081a146b842323e3e7137
Author: Noel Gordon <noel@chromium.org>
Date: Tue Dec 04 00:12:24 2018

Make Crostini externs file an @interface

 - change the Crostini externs file to be an @interface class
   - add missing methods from the current implementation
   - add background BUILD rules to Closure compile {Crostini}
 - rename the background page Crostini to CrostiniImpl and
   make it @implements the {Crostini} class interface
 - use CrostiniImpl as the implementation for Files App: it is
   only instantiated on the Files app background page.
 - use CrostiniImpl for foreground unittests for now, until a
   mock (test-only) implementation is available.

No change in test behavior, no new tests.

Tbr: lucmult@
Bug:  911024 
Change-Id: I580c9701374d6d8c1842a96040625c6be719ed64
Reviewed-on: https://chromium-review.googlesource.com/c/1358319
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613341}
[modify] https://crrev.com/49c3b5ddad77fcf536f081a146b842323e3e7137/ui/file_manager/externs/background/crostini.js
[modify] https://crrev.com/49c3b5ddad77fcf536f081a146b842323e3e7137/ui/file_manager/file_manager/background/js/BUILD.gn
[modify] https://crrev.com/49c3b5ddad77fcf536f081a146b842323e3e7137/ui/file_manager/file_manager/background/js/background.js
[modify] https://crrev.com/49c3b5ddad77fcf536f081a146b842323e3e7137/ui/file_manager/file_manager/background/js/crostini.js
[modify] https://crrev.com/49c3b5ddad77fcf536f081a146b842323e3e7137/ui/file_manager/file_manager/background/js/crostini_unittest.js
[modify] https://crrev.com/49c3b5ddad77fcf536f081a146b842323e3e7137/ui/file_manager/file_manager/foreground/js/file_tasks_unittest.js
[modify] https://crrev.com/49c3b5ddad77fcf536f081a146b842323e3e7137/ui/file_manager/file_manager/foreground/js/task_controller_unittest.js

Project Member

Comment 3 by bugdroid1@chromium.org, Dec 4

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

commit 8ceeede9ece7c3a29d2f4a99dea9e9595dbfa348
Author: Noel Gordon <noel@chromium.org>
Date: Tue Dec 04 05:00:59 2018

Add mock_crostini rule for FilesApp foreground unittests

 - add background BUILD rule for mock_crostini
   - make mock_crostini visible to foreground code
 - include mock code in foreground unittests.html
 - change background / foreground tests to use it

No change in test behavior.

Bug:  911024 
Change-Id: I15e3b243cea5937111da66ac504e143116bcbf0d
Reviewed-on: https://chromium-review.googlesource.com/c/1358321
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613415}
[modify] https://crrev.com/8ceeede9ece7c3a29d2f4a99dea9e9595dbfa348/ui/file_manager/file_manager/background/js/BUILD.gn
[modify] https://crrev.com/8ceeede9ece7c3a29d2f4a99dea9e9595dbfa348/ui/file_manager/file_manager/background/js/crostini_unittest.js
[add] https://crrev.com/8ceeede9ece7c3a29d2f4a99dea9e9595dbfa348/ui/file_manager/file_manager/background/js/mock_crostini.js
[modify] https://crrev.com/8ceeede9ece7c3a29d2f4a99dea9e9595dbfa348/ui/file_manager/file_manager/foreground/js/file_tasks_unittest.html
[modify] https://crrev.com/8ceeede9ece7c3a29d2f4a99dea9e9595dbfa348/ui/file_manager/file_manager/foreground/js/file_tasks_unittest.js
[modify] https://crrev.com/8ceeede9ece7c3a29d2f4a99dea9e9595dbfa348/ui/file_manager/file_manager/foreground/js/task_controller_unittest.html
[modify] https://crrev.com/8ceeede9ece7c3a29d2f4a99dea9e9595dbfa348/ui/file_manager/file_manager/foreground/js/task_controller_unittest.js

Status: Fixed (was: Started)
Status: Started (was: Fixed)
Need to add mock_crostini to the js_type_check("test_support_type_check") rule ...

Project Member

Comment 6 by bugdroid1@chromium.org, Dec 4

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

commit 068db4e520fd526f29fad53309e9f6465ef53b70
Author: Noel Gordon <noel@chromium.org>
Date: Tue Dec 04 11:03:35 2018

Add mock_crostini to test_support_type_check rule

All background BUILD.gn test mocks rules should be in this list - add
mock_crostini to the list. No change in behavior, no new tests.

Tbr: lucmult@
Bug:  911024 
Change-Id: I624160591f11b3ca2d068edc147c99dab9de7198
Reviewed-on: https://chromium-review.googlesource.com/c/1360454
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613504}
[modify] https://crrev.com/068db4e520fd526f29fad53309e9f6465ef53b70/ui/file_manager/file_manager/background/js/BUILD.gn

Status: Fixed (was: Started)

Sign in to add a comment