New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 679561 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

WebUI: cr_elements: Add missing tests

Project Member Reported by steve...@chromium.org, Jan 10 2017

Issue description

Some of the elements in ui/webui/resources/cr_elements are sparsely tested or untested. We should audit and implement missing tests.

In particular, the following have no tests at all:
cr_dialog
cr_expand_button
cr_search_field
cr_toolbar
cr_network_*
cr_policy_network_indicator
cr_policy_pref_indicator

 
I sent an email to the settings and extensions teams, but for anyone else paying attention to this issue (and for future reference), work on this for policy/* and network/* is currently blocked on the following problem:

Some of our elements have extension API dependencies, specifically ones in policy/ (chrome.settingsPrivate) and network/ (chrome.networkingPrivate).

In cr_elements_browsertest.js we set, e.g., 
browsePreload: 'chrome://resources/cr_elements/policy/cr_policy_pref_indicator.html'.

This does not (as near as I can tell) cause chrome to load any extension APIs.

We can kind of work around this by adding, e.g. settings_private.js as an extra library:

  extraLibraries: CrElementsBrowserTest.prototype.extraLibraries.concat([
    '../../../../../third_party/closure_compiler/externs/settings_private.js',
    'cr_policy_pref_indicator_tests.js',
  ]),

However this winds up executing externs, which we don't want to do.

Does anyone have any thoughts on how to load e.g. settingsPrivate and networkingPrivate for chrome://resources/cr_elements/* (or perhaps for specific subdirs or files)? Or even loading all extension apis (slower presumably, but it's already a browser test).

The only alternative I can think of would be to modify the externs generator to generate a fake implementation with types just for tests, which would be kind of a heavyweight solution (but might simplify some of our existing fakes).


Comment 2 by dpa...@chromium.org, Jan 19 2017

Would it be reasonable to always hide your usage of private extension APIs behind a "browser proxy"? Then you could create a TestBrowserProxy for your test and never have to load the real thing (there are already plenty examples of this, except that they wrap chrome.send calls, not extension API calls).

Comment 3 by dpa...@chromium.org, Jan 19 2017

BTW here is an example of a browser proxy wrapping calls to private extension APIs, https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/appearance_page/appearance_browser_proxy.js?l=45.

Comment 4 by dbeam@chromium.org, Jan 21 2017

conversely, we could probably make a specific C++ test fixture that enables the extension APIs for these tests.

Comment 5 by dbeam@chromium.org, Jan 21 2017

Cc: rdevlin....@chromium.org
Labels: -M-58 M-59
Project Member

Comment 7 by bugdroid1@chromium.org, Feb 17 2017

FYI, https://codereview.chromium.org/2700063002 added some tests for cr-dialog.
Status: Fixed (was: Started)
Summary: WebUI: cr_elements: Add missing tests (was: WebU: cr_elements: Add missing tests)
Added tests for:
cr_dialog
cr_policy_network_indicator
cr_policy_pref_indicator

Tests also do exist for cr_search_field_behavior / cr_toolbar in cr_toolbar_search_field_tests.js.

cr_expand_button is a fairly trivial wrapper around paper-icon-button and should probably be replaced with some CSS eventually.

Created  issue 700121  to track testing for internet_page and cr_network_* tests.

Status: Verified (was: Fixed)
closing it for now. please reopen if tests are not working as excepted.

Sign in to add a comment