MD Settings: fix improper use of prefs in tests

MD Settings: fix improper use of prefs in tests

Tests which load the complete chrome://md-settings page
already have <settings-prefs> elements initialized using
the browser pref store, and need not (and should not) reset
these elements.

This was hiding a bug in AppearanceSettingsBrowserTests.uiTests.

BUG=531750, 531786
Committed: https://crrev.com/b4f0ff0df44c8bc5eb25b1518cae1d1b24add741
Cr-Commit-Position: refs/heads/master@{#383197}

[michaelpg, 2016-03-24 03:18:38]

Description was changed from

==========
MD Settings: fix improper use of prefs in tests

BUG=
==========

to

==========
MD Settings: fix improper use of prefs in tests

Tests which load the complete chrome://md-settings page
already have <settings-prefs> elements initialized using
the browser pref store, and need not (and should not) reset
these elements.

This was hiding a bug in AppearanceSettingsBrowserTests.uiTests.

BUG=531750,531786
==========

[michaelpg, 2016-03-24 03:18:38]

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

[michaelpg, 2016-03-24 03:19:38]

Dave,

PTAL. I noticed these issues while I was trying to write my own
tests, and realized the "resetForTesting" function is confusing
and poorly named. A follow-up CL will address that.

[dschuyler, 2016-03-24 22:37:17]

On 2016/03/24 03:19:38, michaelpg wrote:
> Dave,
> 
> PTAL. I noticed these issues while I was trying to write my own
> tests, and realized the "resetForTesting" function is confusing
> and poorly named. A follow-up CL will address that.

This looks like it switches the test from using fakes to using the
browser prefs directly.  Can you tell me more about the follow-up
CL?  I'm wondering whether the follow-up will go back to prefs fake
and whether that CL would be put up right after this CL or whether it's
an 'in the future' thing.

[michaelpg, 2016-03-24 22:48:36]

On 2016/03/24 22:37:17, dschuyler wrote:
> On 2016/03/24 03:19:38, michaelpg wrote:
> > Dave,
> > 
> > PTAL. I noticed these issues while I was trying to write my own
> > tests, and realized the "resetForTesting" function is confusing
> > and poorly named. A follow-up CL will address that.
> 
> This looks like it switches the test from using fakes to using the
> browser prefs directly.  Can you tell me more about the follow-up
> CL?  I'm wondering whether the follow-up will go back to prefs fake
> and whether that CL would be put up right after this CL or whether it's
> an 'in the future' thing.

This browser test has always loaded Settings in a tab, which uses the real
(stub) browser profile. This is the correct behavior.

There's no need to fake the prefs here. We get a real set of prefs from
chrome.settingsPrivate (the promise returned in suiteSetup ensures we wait
until the C++ returns the prefs).

The follow-up CL is just a naming thing. And dbeam doesn't like it:
https://codereview.chromium.org/1827823003/

[michaelpg, 2016-03-24 22:50:30]

On 2016/03/24 22:48:36, michaelpg wrote:
> On 2016/03/24 22:37:17, dschuyler wrote:
> > On 2016/03/24 03:19:38, michaelpg wrote:
> > > Dave,
> > > 
> > > PTAL. I noticed these issues while I was trying to write my own
> > > tests, and realized the "resetForTesting" function is confusing
> > > and poorly named. A follow-up CL will address that.
> > 
> > This looks like it switches the test from using fakes to using the
> > browser prefs directly.  Can you tell me more about the follow-up
> > CL?  I'm wondering whether the follow-up will go back to prefs fake
> > and whether that CL would be put up right after this CL or whether it's
> > an 'in the future' thing.
> 
> This browser test has always loaded Settings in a tab, which uses the real
> (stub) browser profile. This is the correct behavior.
> 
> There's no need to fake the prefs here. We get a real set of prefs from
> chrome.settingsPrivate (the promise returned in suiteSetup ensures we wait
> until the C++ returns the prefs).
> 
> The follow-up CL is just a naming thing. And dbeam doesn't like it:
> https://codereview.chromium.org/1827823003/

To be clear, the old version LOOKS like it's using fake prefs, but because
settingsPrefs.resetForTesting() doesn't work right, the old version is
ACTUALLY using the real prefs -- "session.restore_on_startup" starts out
as 16, not 1234.

This is fine. So I'm removing the part that was trying to use fake prefs
(but wasn't even doing that).

[dschuyler, 2016-03-24 23:00:07]

Thanks for the background information.  Based on that explanation, LGTM.

[michaelpg, 2016-03-24 23:00:49]

The CQ bit was checked by michaelpg@chromium.org

[michaelpg, 2016-03-24 23:00:50]

The CQ bit was unchecked by michaelpg@chromium.org

[michaelpg, 2016-03-24 23:00:50]

The CQ bit was checked by michaelpg@chromium.org

[michaelpg, 2016-03-24 23:00:50]

The CQ bit was unchecked by michaelpg@chromium.org

[michaelpg, 2016-03-24 23:00:51]

The CQ bit was checked by michaelpg@chromium.org

[commit-bot: I haz the power, 2016-03-24 23:01:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1833613002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1833613002/1

[commit-bot: I haz the power, 2016-03-25 00:08:45]

Description was changed from

==========
MD Settings: fix improper use of prefs in tests

Tests which load the complete chrome://md-settings page
already have <settings-prefs> elements initialized using
the browser pref store, and need not (and should not) reset
these elements.

This was hiding a bug in AppearanceSettingsBrowserTests.uiTests.

BUG=531750,531786
==========

to

==========
MD Settings: fix improper use of prefs in tests

Tests which load the complete chrome://md-settings page
already have <settings-prefs> elements initialized using
the browser pref store, and need not (and should not) reset
these elements.

This was hiding a bug in AppearanceSettingsBrowserTests.uiTests.

BUG=531750,531786
==========

[commit-bot: I haz the power, 2016-03-25 00:08:45]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-03-25 00:10:39]

Description was changed from

==========
MD Settings: fix improper use of prefs in tests

Tests which load the complete chrome://md-settings page
already have <settings-prefs> elements initialized using
the browser pref store, and need not (and should not) reset
these elements.

This was hiding a bug in AppearanceSettingsBrowserTests.uiTests.

BUG=531750,531786
==========

to

==========
MD Settings: fix improper use of prefs in tests

Tests which load the complete chrome://md-settings page
already have <settings-prefs> elements initialized using
the browser pref store, and need not (and should not) reset
these elements.

This was hiding a bug in AppearanceSettingsBrowserTests.uiTests.

BUG=531750,531786

Committed: https://crrev.com/b4f0ff0df44c8bc5eb25b1518cae1d1b24add741
Cr-Commit-Position: refs/heads/master@{#383197}
==========

[commit-bot: I haz the power, 2016-03-25 00:10:40]

Patchset 1 (id:??) landed as
https://crrev.com/b4f0ff0df44c8bc5eb25b1518cae1d1b24add741
Cr-Commit-Position: refs/heads/master@{#383197}