New issue
Advanced search Search tips

Issue 752319 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

MD Downloads: Use browser proxy pattern and add more tests.

Project Member Reported by dpa...@chromium.org, Aug 3 2017

Issue description

MD Downloads UI currently has only two unit tests (not browser tests), for action_service.js.

[1] https://cs.chromium.org/chromium/src/chrome/browser/resources/md_downloads/action_service_unittest.gtestjs

 
Components: UI>Browser>Downloads
Status: Started (was: Available)
So there are actually some UI test that I did not locate initially at https://cs.chromium.org/chromium/src/chrome/test/data/webui/md_downloads/. I'll audit them and make additions as necessary. The "proxy" pattern would benefit existing and potential new tests either way.
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 5 2017

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

commit 527668c43dcc5c735bce1d0472db1a021ab43f41
Author: dpapad <dpapad@chromium.org>
Date: Sat Aug 05 00:39:49 2017

Downloads WebUI: Convert to ES6 class syntax.

This is in preparation of moving all chrome.send calls behind
a "browser proxy" and adding UI tests.

Bug:  752319 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I874eef672f463be8edcae2137b731d301eeebda7
Reviewed-on: https://chromium-review.googlesource.com/600497
Reviewed-by: Dave Schuyler <dschuyler@chromium.org>
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492198}
[modify] https://crrev.com/527668c43dcc5c735bce1d0472db1a021ab43f41/chrome/browser/resources/md_downloads/action_service.js
[modify] https://crrev.com/527668c43dcc5c735bce1d0472db1a021ab43f41/chrome/browser/resources/md_downloads/action_service_unittest.gtestjs
[modify] https://crrev.com/527668c43dcc5c735bce1d0472db1a021ab43f41/chrome/test/data/webui/md_downloads/toolbar_tests.js

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 9 2017

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

commit fd7dc9d92b1d29319061ca3d60e4497da8d16525
Author: dpapad <dpapad@chromium.org>
Date: Wed Aug 09 06:51:54 2017

Downloads WebUI: Use a browser proxy in preparation for more tests.

ActionService was responsible for search related logic and state, as well as
forwording chrome.send() calls. Splitting it to two classes,

 - BrowserProxy, responsible only for chromes.send() calls.
 - ActionService responsible only for search related stuff (will be renamed
   to SearchService in follow up CL, to make reviewing this CL easier)

This is in preparation of adding a TestBrowserProxy such that during testing
no chrome.send() calls are invoked.

Bug:  752319 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I5dc3c6a05655b2ccc412b91a368d60d46f385d30
Reviewed-on: https://chromium-review.googlesource.com/601194
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Dan Beam (no longer on Chrome) <dbeam@chromium.org>
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492869}
[modify] https://crrev.com/fd7dc9d92b1d29319061ca3d60e4497da8d16525/chrome/browser/browser_resources.grd
[modify] https://crrev.com/fd7dc9d92b1d29319061ca3d60e4497da8d16525/chrome/browser/resources/md_downloads/action_service.html
[modify] https://crrev.com/fd7dc9d92b1d29319061ca3d60e4497da8d16525/chrome/browser/resources/md_downloads/action_service.js
[modify] https://crrev.com/fd7dc9d92b1d29319061ca3d60e4497da8d16525/chrome/browser/resources/md_downloads/action_service_unittest.gtestjs
[add] https://crrev.com/fd7dc9d92b1d29319061ca3d60e4497da8d16525/chrome/browser/resources/md_downloads/browser_proxy.html
[add] https://crrev.com/fd7dc9d92b1d29319061ca3d60e4497da8d16525/chrome/browser/resources/md_downloads/browser_proxy.js
[modify] https://crrev.com/fd7dc9d92b1d29319061ca3d60e4497da8d16525/chrome/browser/resources/md_downloads/compiled_resources2.gyp
[modify] https://crrev.com/fd7dc9d92b1d29319061ca3d60e4497da8d16525/chrome/browser/resources/md_downloads/item.html
[modify] https://crrev.com/fd7dc9d92b1d29319061ca3d60e4497da8d16525/chrome/browser/resources/md_downloads/item.js
[modify] https://crrev.com/fd7dc9d92b1d29319061ca3d60e4497da8d16525/chrome/browser/resources/md_downloads/manager.html
[modify] https://crrev.com/fd7dc9d92b1d29319061ca3d60e4497da8d16525/chrome/browser/resources/md_downloads/manager.js
[modify] https://crrev.com/fd7dc9d92b1d29319061ca3d60e4497da8d16525/chrome/browser/resources/md_downloads/toolbar.html
[modify] https://crrev.com/fd7dc9d92b1d29319061ca3d60e4497da8d16525/chrome/browser/resources/md_downloads/toolbar.js
[modify] https://crrev.com/fd7dc9d92b1d29319061ca3d60e4497da8d16525/chrome/browser/ui/webui/md_downloads/md_downloads_ui.cc
[modify] https://crrev.com/fd7dc9d92b1d29319061ca3d60e4497da8d16525/chrome/test/data/webui/BUILD.gn

Project Member

Comment 6 by bugdroid1@chromium.org, Sep 12 2017

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

commit 41d310d791d8dcbf227a865b3d44d4edfee2aaf7
Author: dpapad <dpapad@chromium.org>
Date: Tue Sep 12 01:44:03 2017

Downloads WebUI: Rename ActionService to SearchService.

This is a follow up from r492869 which reduced the responsibilities of
ActionService to only contain search related logic.

Bug:  752319 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I81452d2ada95ed1ef4478f93961b8467b4724cb8
Reviewed-on: https://chromium-review.googlesource.com/610822
Reviewed-by: Scott Chen <scottchen@chromium.org>
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501144}
[modify] https://crrev.com/41d310d791d8dcbf227a865b3d44d4edfee2aaf7/chrome/browser/browser_resources.grd
[delete] https://crrev.com/5851f75e9af166477ceb75f3d51126c44dd77ead/chrome/browser/resources/md_downloads/action_service_unittest.gtestjs
[modify] https://crrev.com/41d310d791d8dcbf227a865b3d44d4edfee2aaf7/chrome/browser/resources/md_downloads/compiled_resources2.gyp
[modify] https://crrev.com/41d310d791d8dcbf227a865b3d44d4edfee2aaf7/chrome/browser/resources/md_downloads/manager.html
[modify] https://crrev.com/41d310d791d8dcbf227a865b3d44d4edfee2aaf7/chrome/browser/resources/md_downloads/manager.js
[rename] https://crrev.com/41d310d791d8dcbf227a865b3d44d4edfee2aaf7/chrome/browser/resources/md_downloads/search_service.html
[rename] https://crrev.com/41d310d791d8dcbf227a865b3d44d4edfee2aaf7/chrome/browser/resources/md_downloads/search_service.js
[add] https://crrev.com/41d310d791d8dcbf227a865b3d44d4edfee2aaf7/chrome/browser/resources/md_downloads/search_service_unittest.gtestjs
[modify] https://crrev.com/41d310d791d8dcbf227a865b3d44d4edfee2aaf7/chrome/browser/resources/md_downloads/toolbar.html
[modify] https://crrev.com/41d310d791d8dcbf227a865b3d44d4edfee2aaf7/chrome/browser/resources/md_downloads/toolbar.js
[modify] https://crrev.com/41d310d791d8dcbf227a865b3d44d4edfee2aaf7/chrome/browser/ui/webui/md_downloads/md_downloads_ui.cc
[modify] https://crrev.com/41d310d791d8dcbf227a865b3d44d4edfee2aaf7/chrome/test/data/webui/BUILD.gn
[modify] https://crrev.com/41d310d791d8dcbf227a865b3d44d4edfee2aaf7/chrome/test/data/webui/md_downloads/toolbar_tests.js

Status: Fixed (was: Started)
The browser proxy pattern is in place [1]. I don't have cycles to write new tests for Downloads. Declaring this as partially fixed.

[1] https://cs.chromium.org/chromium/src/chrome/browser/resources/md_downloads/browser_proxy.js. 

Sign in to add a comment