New issue
Advanced search Search tips

Issue 905934 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 22
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Task

Blocking:
issue 860355



Sign in to add a comment

Closure compile import_history_unittest

Project Member Reported by slangley@chromium.org, Nov 16

Issue description

Remove html unittest file.
Closure compile test file and deps.
 
Owner: noel@chromium.org
 Issue 906891  has been merged into this issue.
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 21

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

commit 1db80a3602a14700a1d6f1f1c192ac45569f11fa
Author: Noel Gordon <noel@chromium.org>
Date: Wed Nov 21 00:33:16 2018

BUILD fix: file_manager/common/js/BUILD.gn unittest_util

unittest_util is a js_library: it is not js_unittest, best I can tell.
No change in behavior, no new tests.

Bug:  905934 
No-try: true
Change-Id: Ib508c7380ca7babf970895ce7d69545d219daaa6
Reviewed-on: https://chromium-review.googlesource.com/c/1345370
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Stuart Langley <slangley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609867}
[modify] https://crrev.com/1db80a3602a14700a1d6f1f1c192ac45569f11fa/ui/file_manager/file_manager/common/js/BUILD.gn

Project Member

Comment 4 by bugdroid1@chromium.org, Nov 21

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

commit 4f984f4e13ac498176597639a09adf43faca3ea4
Author: Noel Gordon <noel@chromium.org>
Date: Wed Nov 21 01:16:22 2018

Remove analytics tracking from import.RuntimeHistoryLoader

No change in test behavior, no new tests.

Bug:  905934 ,  847729 
No-try: true
Change-Id: Ic1a9b352260670b643bb60cae6cfb5483f80ea70
Reviewed-on: https://chromium-review.googlesource.com/c/1345374
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Stuart Langley <slangley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609879}
[modify] https://crrev.com/4f984f4e13ac498176597639a09adf43faca3ea4/ui/file_manager/file_manager/background/js/background.js
[modify] https://crrev.com/4f984f4e13ac498176597639a09adf43faca3ea4/ui/file_manager/file_manager/background/js/import_history.js
[modify] https://crrev.com/4f984f4e13ac498176597639a09adf43faca3ea4/ui/file_manager/file_manager/background/js/import_history_unittest.js

Cc: fukino@chromium.org
After #4, metrics.ImportEvents.HISTORY_LOAD is no longer used. We should removed it. +fukino-san
Status: Started (was: Available)
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 21

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

commit 6e171745ac433bb66f457118d372929f5c819148
Author: Noel Gordon <noel@chromium.org>
Date: Wed Nov 21 06:51:06 2018

Remove file_manager/extern import_history_enum.js

Move this enum into its namespace file (extern import_history): avoids
mistakes in our FilesApp BUILD file where developer forgot to add both
these dependent extern files.

These externs are used to closure compile parts of Files app including
our JS unittests (this change is covered by two unittests for example)
so they are useful (see bug).

No change in test behavior, no new tests.

Bug:  905934 
Change-Id: I126d8486425fa16098eb16d52376a1622e656d14
Reviewed-on: https://chromium-review.googlesource.com/c/1345697
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: calamity <calamity@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609928}
[modify] https://crrev.com/6e171745ac433bb66f457118d372929f5c819148/ui/file_manager/externs/background/import_history.js
[delete] https://crrev.com/dd5bccafb152e96b4a7c26f86e7aac3a0e398845/ui/file_manager/externs/background/import_history_enum.js
[modify] https://crrev.com/6e171745ac433bb66f457118d372929f5c819148/ui/file_manager/file_manager/background/js/import_history.js
[modify] https://crrev.com/6e171745ac433bb66f457118d372929f5c819148/ui/file_manager/file_manager/foreground/js/ui/BUILD.gn

Project Member

Comment 8 by bugdroid1@chromium.org, Nov 21

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

commit b970401f02ce0fccbea96a392aa4a298b9618419
Author: Noel Gordon <noel@chromium.org>
Date: Wed Nov 21 10:20:39 2018

Clean-up import_history_unittest.js

To prepare for closure compilation of this unit_test file, clean-up to
make upcoming closure changes easier to understand.

  - update and fix obvious closure @type errors, etc
  - remove '|undefined' from variables new-ed in Setup()
  - define undefined variables, remove unused variables
  - importer.SyncFileEntryProvider is not defined in Files
    app, nor in the code-base or the web: remove it.

No change in test behavior, no new tests.

Bug:  905934 
Change-Id: Idab68cc2257286e2278cdce361a84fc2176c51cc
Reviewed-on: https://chromium-review.googlesource.com/c/1345690
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609983}
[modify] https://crrev.com/b970401f02ce0fccbea96a392aa4a298b9618419/ui/file_manager/file_manager/background/js/import_history_unittest.js

Project Member

Comment 9 by bugdroid1@chromium.org, Nov 21

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

commit 3e0d94809a3f1748ce96fecc8aa1b5ebca49ca02
Author: Noel Gordon <noel@chromium.org>
Date: Wed Nov 21 11:38:59 2018

Remove metrics and analytics from FilesApp import_history_unittest

CL:1345374 removed metrics / analytics use from ImportHistory and with
those gone, we can now 1) remove them from import_history_unittest and
2) remove metrics from the import history js_library BUILD deps.

No change in test behavior, no new tests.

Bug:  905934 
Change-Id: I706beb0fa5c61fdb55b46a8d2072a9de1016fa40
Reviewed-on: https://chromium-review.googlesource.com/c/1346189
Reviewed-by: Sam McNally <sammc@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610003}
[modify] https://crrev.com/3e0d94809a3f1748ce96fecc8aa1b5ebca49ca02/ui/file_manager/file_manager/background/js/BUILD.gn
[modify] https://crrev.com/3e0d94809a3f1748ce96fecc8aa1b5ebca49ca02/ui/file_manager/file_manager/background/js/import_history_unittest.html

Project Member

Comment 10 by bugdroid1@chromium.org, Nov 21

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

commit 4aa1743483c6efb140c26f0dc5d447b0161b7a27
Author: Noel Gordon <noel@chromium.org>
Date: Wed Nov 21 13:14:41 2018

After CL:1345374 metrics.ImportEvents are unused: remove

 - remove metrics.ImportEvents
 - remove metrics.timing.Variables.HISTORY_LOAD

No change in behavior, no new tests.

Bug:  905934 ,  847729 
Change-Id: I2f5ab9659dc5e134bdb47d8c770a1f918b4a0126
Reviewed-on: https://chromium-review.googlesource.com/c/1345381
Reviewed-by: Stuart Langley <slangley@chromium.org>
Reviewed-by: Naoki Fukino <fukino@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610025}
[modify] https://crrev.com/4aa1743483c6efb140c26f0dc5d447b0161b7a27/ui/file_manager/file_manager/common/js/metrics_events.js

^^^ addresses #4, #5.
Project Member

Comment 12 by bugdroid1@chromium.org, Nov 22

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

commit 6d3e5f486486d1046831287b67df6723887430bb
Author: Noel Gordon <noel@chromium.org>
Date: Thu Nov 22 00:52:33 2018

Closure compile import_history_unittest

Closure JS compiler type annotations for this unit test were added in
CL:1345690: enable them and auto-generate the unit test code.

 Remove import_history_unittest.html
  - make file_manager_jstest.cc auto-generate the unit test code

 BUILD file: enable Closure compiler
  - update js_library(import_history) deps for Closure compiler
  - add js_unittest(import_history_unittest) auto-generate rule

 import_history_unittest.js: remove unused Drive sync leftovers
  - Drive sync moved to its own unittest rule in the past so ...
  - chrome.fileManagerPrivate.onFileTransfersUpdated > /dev/null
  - chrome.syncFileSystem, TestSyncFileEntryProvider > /dev/null

Bug:  905934 
Change-Id: I16b93e755c3a4a00378b426616cf030afc6a325f
Reviewed-on: https://chromium-review.googlesource.com/c/1346430
Reviewed-by: Stuart Langley <slangley@chromium.org>
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610274}
[modify] https://crrev.com/6d3e5f486486d1046831287b67df6723887430bb/chrome/browser/chromeos/file_manager/file_manager_jstest.cc
[modify] https://crrev.com/6d3e5f486486d1046831287b67df6723887430bb/ui/file_manager/file_manager/background/js/BUILD.gn
[delete] https://crrev.com/ed2b4ba7455a1a193cc24879ceef0c2762e7892f/ui/file_manager/file_manager/background/js/import_history_unittest.html
[modify] https://crrev.com/6d3e5f486486d1046831287b67df6723887430bb/ui/file_manager/file_manager/background/js/import_history_unittest.js

#4 removed analytics tracking of time taken to load history and the number of files loaded. Filed issue 907702 to determine if need to add an UMA for that.

Status: Fixed (was: Started)

Sign in to add a comment