[MD settings] unit test for incognito activation

chrome/test/data/webui/settings/site_list_tests.js

Index: chrome/test/data/webui/settings/site_list_tests.js
diff --git a/chrome/test/data/webui/settings/site_list_tests.js b/chrome/test/data/webui/settings/site_list_tests.js
index 1cf86362890a2406b1f47cd4565e5d0099f97ea5..c2387110eb885924fc7e3f0b1c257ac785072f6d 100644
--- a/chrome/test/data/webui/settings/site_list_tests.js
+++ b/chrome/test/data/webui/settings/site_list_tests.js
@@ -59,6 +59,24 @@ cr.define('site_list', function() {
   };
 
   /**
+   * An example of prefs controlleBy policy.
+   * @type {SiteSettingsPref}
+   */
+  var prefsControlled = {
+    exceptions: {
+      plugins: [
+        {
+          category: 'plugins',
+          embeddingOrigin: 'http://foo-block.com',
+          origin: 'http://foo-block.com',
+          setting: 'block',
+          source: 'policy',
+        },
+      ]
+    }
+  };
+
+  /**
    * An example pref with mixed schemes (present and absent).
    * @type {SiteSettingsPref}
    */
@@ -594,6 +612,28 @@ cr.define('site_list', function() {
             });
       });
 
+      test('update lists for incognito', function() {
+        var contentType = settings.ContentSettingsTypes.PLUGINS;
+        var categorySubtype = settings.PermissionValues.BLOCK;
+        setUpCategory(contentType, categorySubtype, prefsControlled);
+        return browserProxy.whenCalled('getExceptionList').then(
+            function(actualContentType) {
+              assertEquals(1, testElement.sites.length);
+              browserProxy.setIncognito(true);
+              return browserProxy.whenCalled('getExceptionList');

[dpapad, 2017/03/23 21:31:45]

Are you waiting on a new call to getExceptionList() here? If so, there should be
a browserProxy.resetResolver('getExceptionList') call first, otherwise
whenCalled() simply resolves because of the previous call to getExceptionList(),
not because of the new one.

Same below.

[dschuyler, 2017/03/23 22:13:50]

Done.

+            }).then(function() {
+              assertEquals(2, testElement.sites.length);

[dpapad, 2017/03/23 21:31:45]

Asserting that the |sites| Polymer property has a certain number of entries does
not provide as much coverage as we could. For example someone can erroneously
remove "items=[[sites]]" from
https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/site_s...
and this test would not break.

Can we assert on the expected DOM structure instead, as follows?
assertEquals(2, testElement.$$('.list-item').length)
This would increase the test coverage of <site-list>.

Also it is not clear to me why |sites| is a public property. From a quick search
it seems it is only used within site_list.js and therefore could be made
private, which would be one more reason not to access it directly from the test.

[dschuyler, 2017/03/23 22:13:51]

We can; I've switched it. I'm concerned that this makes the test a bit more
compound which can make it harder to see where a failure is occurring (but not
enough to resist the suggestion). 
That's reasonable and sounds like a good change for a separate CL.

+              browserProxy.setIncognito(false);
+              return browserProxy.whenCalled('getExceptionList');
+            }).then(function() {
+              assertEquals(1, testElement.sites.length);
+              browserProxy.setIncognito(true);
+              return browserProxy.whenCalled('getExceptionList');
+            }).then(function() {
+              assertEquals(2, testElement.sites.length);
+            });
+      });
+
       test('initial INCOGNITO BLOCK state is correct', function() {
         var contentType = settings.ContentSettingsTypes.COOKIES;
         var categorySubtype = settings.PermissionValues.BLOCK;