[MD settings] split up site settings tests

[MD settings] split up site settings tests

This CL separates the site settings browser tests.

(Code health)
BUG=None

Review-Url: https://codereview.chromium.org/2862463002
Cr-Commit-Position: refs/heads/master@{#469527}
Committed: https://chromium.googlesource.com/chromium/src/+/3884ab631ec16724abd70b40a57cf01b953eeaab

[dschuyler, 2017-05-03 01:34:00]

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

[commit-bot: I haz the power, 2017-05-03 01:34:38]

Dry run: CQ is trying da patch.

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

[dschuyler, 2017-05-03 01:37:19]

dschuyler@chromium.org changed reviewers:
+ dbeam@chromium.org

[dschuyler, 2017-05-03 01:37:20]

Thanks for reviewing https://codereview.chromium.org/2855943004/

This CL separates the rest of the site settings browser tests. In a similar
fashion to the above CL, there is just the one file with the main changes, the
other files all just have the cr.define removed and have been reformatted.

[dschuyler, 2017-05-03 01:38:19]

On 2017/05/03 01:37:20, dschuyler wrote:
> Thanks for reviewing https://codereview.chromium.org/2855943004/
> 
> This CL separates the rest of the site settings browser tests. In a similar
> fashion to the above CL, there is just the one file with the main changes, the
> other files all just have the cr.define removed and have been reformatted.

That 'main' file being cr_settings_browsertest.js.

[commit-bot: I haz the power, 2017-05-03 01:38:30]

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

[commit-bot: I haz the power, 2017-05-03 01:38:31]

Dry run: Try jobs failed on following builders:
  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_...)

[dschuyler, 2017-05-03 19:35:37]

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

[commit-bot: I haz the power, 2017-05-03 19:36:17]

Dry run: CQ is trying da patch.

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

[dschuyler, 2017-05-03 19:38:57]

Merge with master and comment change.

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

https://codereview.chromium.org/2862463002/diff/20001/chrome/test/data/webui/...
chrome/test/data/webui/settings/site_list_tests.js:59: * An example of prefs
controlledBy policy.
Aside from the formatting change, this comment was spelling/typo corrected from
saying, "controlleBy".

[commit-bot: I haz the power, 2017-05-03 20:01:12]

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

[commit-bot: I haz the power, 2017-05-03 20:01:12]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[dschuyler, 2017-05-03 20:50:18]

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

[commit-bot: I haz the power, 2017-05-03 20:50:43]

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-05-03 21:44:53]

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

[commit-bot: I haz the power, 2017-05-03 21:44:54]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dschuyler, 2017-05-03 22:54:02]

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

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

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-05-04 00:08:43]

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

[commit-bot: I haz the power, 2017-05-04 00:08:44]

Dry run: This issue passed the CQ dry run.

[Dan Beam, 2017-05-04 19:27:17]

https://codereview.chromium.org/2862463002/diff/60001/chrome/test/data/webui/...
File chrome/test/data/webui/settings/cr_settings_browsertest.js (right):

https://codereview.chromium.org/2862463002/diff/60001/chrome/test/data/webui/...
chrome/test/data/webui/settings/cr_settings_browsertest.js:803:
TEST_F('CrSettingsSiteDataDetailsTest', 'SiteSettings', function() {
is there a purpose to any of the second names?  they just seem to be duplicative
of the former names

https://codereview.chromium.org/2862463002/diff/60001/chrome/test/data/webui/...
chrome/test/data/webui/settings/cr_settings_browsertest.js:943: });
uhhhhh, wait, what?  i think this just running the same tests 3 times...

[dschuyler, 2017-05-04 22:23:58]

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

[commit-bot: I haz the power, 2017-05-04 22:24:25]

Dry run: CQ is trying da patch.

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

[dschuyler, 2017-05-04 22:31:47]

https://codereview.chromium.org/2862463002/diff/60001/chrome/test/data/webui/...
File chrome/test/data/webui/settings/cr_settings_browsertest.js (right):

https://codereview.chromium.org/2862463002/diff/60001/chrome/test/data/webui/...
chrome/test/data/webui/settings/cr_settings_browsertest.js:803:
TEST_F('CrSettingsSiteDataDetailsTest', 'SiteSettings', function() {
On 2017/05/04 19:27:17, Dan Beam wrote:
> is there a purpose to any of the second names?  they just seem to be
duplicative
> of the former names

Afaik they are a sub-label for the test: they can be any string but cannot be
duplicated. I could change them to something like
TEST_F('CrSettingsSiteDataDetailsTest', 'apple',
TEST_F('CrSettingsSiteDataDetailsTest', 'banana',
TEST_F('CrSettingsSiteDataDetailsTest', 'carrot',

But this won't work
TEST_F('CrSettingsSiteDataDetailsTest', 'same',
TEST_F('CrSettingsSiteDataDetailsTest', 'same',
TEST_F('CrSettingsSiteDataDetailsTest', 'same',

Using a partial version of the first label seemed common, so I was trying to
keep with that. Sometimes 'All' is used. I don't have a strong opinion about
what second label is used.

https://codereview.chromium.org/2862463002/diff/60001/chrome/test/data/webui/...
chrome/test/data/webui/settings/cr_settings_browsertest.js:943: });
On 2017/05/04 19:27:17, Dan Beam wrote:
> uhhhhh, wait, what?  i think this just running the same tests 3 times...

Yep thanks! forgot to change that.

Done.

[Dan Beam, 2017-05-04 22:57:42]

congrats!  the blame will now say you're the owner of ALL these tests!

lgtm

https://codereview.chromium.org/2862463002/diff/80001/chrome/test/data/webui/...
File chrome/test/data/webui/settings/cr_settings_browsertest.js (right):

https://codereview.chromium.org/2862463002/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/settings/cr_settings_browsertest.js:829:
'CrSettingsCategoryDefaultSettingTest', 'CategoryDefaultSetting',
what's the point of the second name here?  it just seems to make things ugly

https://codereview.chromium.org/2862463002/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/settings/cr_settings_browsertest.js:855:
'CrSettingsCategorySettingExceptionsTest', 'CategorySettingExceptions',
same

https://codereview.chromium.org/2862463002/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/settings/cr_settings_browsertest.js:943:
mocha.grep('AddExceptionDialog').run();
sweet

[commit-bot: I haz the power, 2017-05-04 23:29:59]

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

[commit-bot: I haz the power, 2017-05-04 23:29:59]

Dry run: This issue passed the CQ dry run.

[dschuyler, 2017-05-04 23:45:41]

https://codereview.chromium.org/2862463002/diff/80001/chrome/test/data/webui/...
File chrome/test/data/webui/settings/cr_settings_browsertest.js (right):

https://codereview.chromium.org/2862463002/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/settings/cr_settings_browsertest.js:829:
'CrSettingsCategoryDefaultSettingTest', 'CategoryDefaultSetting',
On 2017/05/04 22:57:42, Dan Beam wrote:
> what's the point of the second name here?  it just seems to make things ugly

Moved to https://codereview.chromium.org/2862023004/

https://codereview.chromium.org/2862463002/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/settings/cr_settings_browsertest.js:855:
'CrSettingsCategorySettingExceptionsTest', 'CategorySettingExceptions',
On 2017/05/04 22:57:42, Dan Beam wrote:
> same

Acknowledged.

https://codereview.chromium.org/2862463002/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/settings/cr_settings_browsertest.js:943:
mocha.grep('AddExceptionDialog').run();
On 2017/05/04 22:57:42, Dan Beam wrote:
> sweet

Acknowledged.

[dschuyler, 2017-05-04 23:45:47]

The CQ bit was checked by dschuyler@chromium.org

[commit-bot: I haz the power, 2017-05-04 23:46:30]

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-04 23:52:25]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1493941547458700,
"parent_rev": "8d28e669ef279c46b174cc78a5a24c729fb720a5", "commit_rev":
"3884ab631ec16724abd70b40a57cf01b953eeaab"}

[commit-bot: I haz the power, 2017-05-04 23:52:42]

Description was changed from

==========
[MD settings] split up site settings tests

This CL separates the site settings browser tests.

(Code health)
BUG=None
==========

to

==========
[MD settings] split up site settings tests

This CL separates the site settings browser tests.

(Code health)
BUG=None

Review-Url: https://codereview.chromium.org/2862463002
Cr-Commit-Position: refs/heads/master@{#469527}
Committed:
https://chromium.googlesource.com/chromium/src/+/3884ab631ec16724abd70b40a57c...
==========

[commit-bot: I haz the power, 2017-05-04 23:52:43]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/3884ab631ec16724abd70b40a57c...