Change SettingsLanguagesPageBrowserTest to CrSettingsLanguagesPageTest

Change SettingsLanguagesPageBrowserTest to CrSettingsLanguagesPageTest

Transforms a browser test to a lighterweight browser test in an attempt
to avoid test flakes: don't load the settings page (in non-vulcanized
builds), don't make round trips to the real browser settings, and split
up the tests into four individual browser tests.

SettingsLanguagesPageBrowserTest is currently a SettingsPageBrowserTest
test fixture that loads chrome://md-settings with real browser settings
to test language settings.

This CL changes it into a CrSettingsBrowserTest, which differs by
loading only the <settings-languages-page> file and its dependencies,
starting with a blank page, mocking the various settings APIs, and
testing the element in isolation.

On a vulcanized build, we still load the entire chrome://md-settings
page. Once it loads, though, we clear that page, mock the various
settings APIs, and can still test a <settings-languages-page> in
relative isolation.

The mocha test suite is invoked in four parts, so each browser test has
to execute fewer mocha tests and should finish in less time.

R=dpapad@chromium.org
BUG=641400
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2837703002
Cr-Commit-Position: refs/heads/master@{#469789}
Committed: https://chromium.googlesource.com/chromium/src/+/dbea67d82a6fc4caa4558ae3066bfa76afdc18e1

[michaelpg, 2017-04-22 03:52:05]

Description was changed from

==========
Change SettingsLanguagesPageBrowserTest to CrSettingsLanguagesPageTest

Transforms a browser test to a lighterweight browser test in an attempt
to avoid test flakes: don't load the settings page (in non-vulcanized
builds), don't make round trips to the real browser settings, and split
up the tests into four individual browser tests.

SettingsLanguagesPageBrowserTest is currently a SettingsPageBrowserTest
test fixture that loads chrome://md-settings with real browser settings
to test language settings.

This CL changes it into a CrSettingsBrowserTest, which differs by
loading only the <settings-languages-page> file and its dependencies,
starting with a blank page, mocking the various settings APIs, and
testing the element in isolation.

On a vulcanized build, we still load the entire chrome://md-settings
page. Once it loads, though, we clear that page, mock the various
settings APIs, and can still test a <settings-languages-page> in
relative isolation.

The mocha test suite is invoked in four parts, so each browser test has
to execute fewer mocha tests and should finish in less time.

R=dpapad@chromium.org
BUG=641400
==========

to

==========
Change SettingsLanguagesPageBrowserTest to CrSettingsLanguagesPageTest

Transforms a browser test to a lighterweight browser test in an attempt
to avoid test flakes: don't load the settings page (in non-vulcanized
builds), don't make round trips to the real browser settings, and split
up the tests into four individual browser tests.

SettingsLanguagesPageBrowserTest is currently a SettingsPageBrowserTest
test fixture that loads chrome://md-settings with real browser settings
to test language settings.

This CL changes it into a CrSettingsBrowserTest, which differs by
loading only the <settings-languages-page> file and its dependencies,
starting with a blank page, mocking the various settings APIs, and
testing the element in isolation.

On a vulcanized build, we still load the entire chrome://md-settings
page. Once it loads, though, we clear that page, mock the various
settings APIs, and can still test a <settings-languages-page> in
relative isolation.

The mocha test suite is invoked in four parts, so each browser test has
to execute fewer mocha tests and should finish in less time.

R=dpapad@chromium.org
BUG=641400
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[michaelpg, 2017-04-22 04:04:25]

The CQ bit was checked by michaelpg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-22 04:04:36]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-22 05:14:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-22 05:14:12]

Dry run: This issue passed the CQ dry run.

[dpapad, 2017-04-25 20:41:17]

LGTM with nits. Thanks for doing this!

https://codereview.chromium.org/2837703002/diff/20001/chrome/test/data/webui/...
File chrome/test/data/webui/settings/languages_page_tests.js (right):

https://codereview.chromium.org/2837703002/diff/20001/chrome/test/data/webui/...
chrome/test/data/webui/settings/languages_page_tests.js:7: const TestNames = {
See other comment in this file about ES6 usage.

https://codereview.chromium.org/2837703002/diff/20001/chrome/test/data/webui/...
chrome/test/data/webui/settings/languages_page_tests.js:216:
MockInteractions.tap(paperToggle);
Is taping the internal paper-toggle necessary? Would it work if we just tap the
outer settings-toggle-button?

https://codereview.chromium.org/2837703002/diff/20001/chrome/test/data/webui/...
chrome/test/data/webui/settings/languages_page_tests.js:302: for (var language
of ['en-CA', 'en-US', 'tk', 'no'])
Technically the guide at
https://chromium.googlesource.com/chromium/src/+/master/docs/es6_chromium.md#...,
should be the authority on which ES6 features are OK to use in Chromium's WebUI
code and which not. Until recently me had paused that process (being busy with
MD Settings launch).

Can we hold off starting using ES6 features from test code, until our toolchain
is updated to support ES6? I am actively working on this
(https://codereview.chromium.org/2829933003), and hoping this will happen fairly
soon.

[michaelpg, 2017-05-04 22:54:20]

Waiting to see whether the PolymerTest.clearBody CL this depends on passes
tests.

https://codereview.chromium.org/2837703002/diff/20001/chrome/test/data/webui/...
File chrome/test/data/webui/settings/languages_page_tests.js (right):

https://codereview.chromium.org/2837703002/diff/20001/chrome/test/data/webui/...
chrome/test/data/webui/settings/languages_page_tests.js:7: const TestNames = {
On 2017/04/25 20:41:17, dpapad wrote:
> See other comment in this file about ES6 usage.

Acknowledged.

https://codereview.chromium.org/2837703002/diff/20001/chrome/test/data/webui/...
chrome/test/data/webui/settings/languages_page_tests.js:216:
MockInteractions.tap(paperToggle);
On 2017/04/25 20:41:17, dpapad wrote:
> Is taping the internal paper-toggle necessary? Would it work if we just tap
the
> outer settings-toggle-button?

Yes, it's necessary; nothing forwards the mock tap on the settings-toggle-button
to the paper-toggle-button.

https://codereview.chromium.org/2837703002/diff/20001/chrome/test/data/webui/...
chrome/test/data/webui/settings/languages_page_tests.js:302: for (var language
of ['en-CA', 'en-US', 'tk', 'no'])
On 2017/04/25 20:41:17, dpapad wrote:
> Technically the guide at
>
https://chromium.googlesource.com/chromium/src/+/master/docs/es6_chromium.md#...,
> should be the authority on which ES6 features are OK to use in Chromium's
WebUI
> code and which not. Until recently me had paused that process (being busy with
> MD Settings launch).
> 
> Can we hold off starting using ES6 features from test code, until our
toolchain
> is updated to support ES6? I am actively working on this
> (https://codereview.chromium.org/2829933003), and hoping this will happen
fairly
> soon.

As discussed in standup, since our tests already use these features (and I think
this test predates that guide) I will leave as-is.

[dpapad, 2017-05-05 17:12:09]

LGTM++

[michaelpg, 2017-05-05 18:51:54]

The CQ bit was checked by michaelpg@chromium.org

[commit-bot: I haz the power, 2017-05-05 18:52:40]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-05 18:57:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-05 18:57:29]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[michaelpg, 2017-05-05 21:03:47]

The CQ bit was checked by michaelpg@chromium.org

[michaelpg, 2017-05-05 21:03:47]

The patchset sent to the CQ was uploaded after l-g-t-m from dpapad@chromium.org
Link to the patchset: https://codereview.chromium.org/2837703002/#ps60001
(title: "rebase")

[commit-bot: I haz the power, 2017-05-05 21:05:17]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-05 22:55:03]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1494018227320030,
"parent_rev": "3caa6e14cb856d09bc62c2bbbcae32e2aeadf95b", "commit_rev":
"dbea67d82a6fc4caa4558ae3066bfa76afdc18e1"}

[commit-bot: I haz the power, 2017-05-05 22:55:33]

Description was changed from

==========
Change SettingsLanguagesPageBrowserTest to CrSettingsLanguagesPageTest

Transforms a browser test to a lighterweight browser test in an attempt
to avoid test flakes: don't load the settings page (in non-vulcanized
builds), don't make round trips to the real browser settings, and split
up the tests into four individual browser tests.

SettingsLanguagesPageBrowserTest is currently a SettingsPageBrowserTest
test fixture that loads chrome://md-settings with real browser settings
to test language settings.

This CL changes it into a CrSettingsBrowserTest, which differs by
loading only the <settings-languages-page> file and its dependencies,
starting with a blank page, mocking the various settings APIs, and
testing the element in isolation.

On a vulcanized build, we still load the entire chrome://md-settings
page. Once it loads, though, we clear that page, mock the various
settings APIs, and can still test a <settings-languages-page> in
relative isolation.

The mocha test suite is invoked in four parts, so each browser test has
to execute fewer mocha tests and should finish in less time.

R=dpapad@chromium.org
BUG=641400
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Change SettingsLanguagesPageBrowserTest to CrSettingsLanguagesPageTest

Transforms a browser test to a lighterweight browser test in an attempt
to avoid test flakes: don't load the settings page (in non-vulcanized
builds), don't make round trips to the real browser settings, and split
up the tests into four individual browser tests.

SettingsLanguagesPageBrowserTest is currently a SettingsPageBrowserTest
test fixture that loads chrome://md-settings with real browser settings
to test language settings.

This CL changes it into a CrSettingsBrowserTest, which differs by
loading only the <settings-languages-page> file and its dependencies,
starting with a blank page, mocking the various settings APIs, and
testing the element in isolation.

On a vulcanized build, we still load the entire chrome://md-settings
page. Once it loads, though, we clear that page, mock the various
settings APIs, and can still test a <settings-languages-page> in
relative isolation.

The mocha test suite is invoked in four parts, so each browser test has
to execute fewer mocha tests and should finish in less time.

R=dpapad@chromium.org
BUG=641400
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2837703002
Cr-Commit-Position: refs/heads/master@{#469789}
Committed:
https://chromium.googlesource.com/chromium/src/+/dbea67d82a6fc4caa4558ae3066b...
==========

[commit-bot: I haz the power, 2017-05-05 22:55:34]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/dbea67d82a6fc4caa4558ae3066b...