New issue
Advanced search Search tips

Issue 718472 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: May 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Change TestShellDelegate to make its test pref_service injectable

Project Member Reported by afakhry@chromium.org, May 4 2017

Issue description

While working on this CL: https://codereview.chromium.org/2857103007/
I failed a lot of ash tests because I was trying to read unregistered prefs from the test pref service.

Registering the various profile prefs happen in Chrome in browser_prefs.cc [https://cs.chromium.org/chromium/src/chrome/browser/prefs/browser_prefs.cc?q=browser_prefs.cc+package:%5Echromium$&l=467]. Of course this doesn't happen in ash.

It seems that the best approach is to let those tests interested in testing features related to the prefs_services inject their own into TestShellDelegate and register in it whatever prefs they want.
 
Also FYI: I added this TestPrefServiceSimple in TestShellDelegate as part of the CL: https://codereview.chromium.org/2827193004/ for  Issue 713934 . Now I think it wasn't a good idea.
Project Member

Comment 2 by bugdroid1@chromium.org, May 4 2017

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

commit 7093bce085613efd785b72f2a66ee8be27f7d6ed
Author: afakhry <afakhry@chromium.org>
Date: Thu May 04 18:22:24 2017

Make the pref_service in TestShellDelegate injectable.

This lets tests interested in testing features related to the
prefs_services inject their own into TestShellDelegate and
register in it whatever prefs they want.

BUG= 718472 

Review-Url: https://codereview.chromium.org/2861873004
Cr-Commit-Position: refs/heads/master@{#469387}

[modify] https://crrev.com/7093bce085613efd785b72f2a66ee8be27f7d6ed/ash/test/test_shell_delegate.cc
[modify] https://crrev.com/7093bce085613efd785b72f2a66ee8be27f7d6ed/ash/test/test_shell_delegate.h

Status: Fixed (was: Started)

Comment 4 by dchan@chromium.org, Aug 1 2017

Labels: VerifyIn-61

Comment 5 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment