New issue
Advanced search Search tips

Issue 742706 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug
Team-Security-UX


Show other hotlists

Hotlists containing this issue:
EnamelAndFriendsFixIt


Sign in to add a comment

Fix site settings tests

Project Member Reported by calamity@chromium.org, Jul 14 2017

Issue description

This bug will track work on cleaning up the site settings browser tests.

Current tasks:
test_site_settings_prefs_browser_proxy.js:
- Remove object definitions in prefsEmpty and populate a map with an iteration through settings.contentSettingsTypes
- Remove gigantic if else blocks in getDefaultValueForContentType, getExceptionList, and getOriginPermissions and replace with a map access with contentType as the key
site_details_test.js:
- Use a helper function to populate prefs, instead of defining a huge object.
- Access the test object in the test for the expected value rather than hardcoding expectations for certain values.
site_list_tests.js
- Replace exception constants with method-based, helper framework that was designed above.

As a guide, look for usages of embeddingOrigin inside chrome/test/data/webui/settings/ and eliminate where possible by using better abstractions.
 

Comment 1 by est...@chromium.org, Nov 10 2017

Labels: Hotlist-EnamelAndFriendsFixIt
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 8 2018

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

commit 1c66a54bf7fa36978422408cccdad7b037648ab5
Author: Patti <patricialor@chromium.org>
Date: Thu Feb 08 06:33:28 2018

Site Settings: Use settings.ContentSettingsTypes in site settings tests.

Refactor site settings tests to replace the custom content settings object used
with an object whose keys actually match up to the ContentSettingsTypes used in
non-test code. This removes the need for additional mappings in the test code,
which is a lot cleaner.

Bug:  742706 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I721cee963ad915bd8e179f44e5e39f7ea0b858bf
Reviewed-on: https://chromium-review.googlesource.com/861551
Commit-Queue: Patti <patricialor@chromium.org>
Reviewed-by: calamity <calamity@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535319}
[modify] https://crrev.com/1c66a54bf7fa36978422408cccdad7b037648ab5/chrome/browser/resources/settings/site_settings/constants.js
[modify] https://crrev.com/1c66a54bf7fa36978422408cccdad7b037648ab5/chrome/browser/ui/webui/site_settings_helper.cc
[modify] https://crrev.com/1c66a54bf7fa36978422408cccdad7b037648ab5/chrome/test/data/webui/settings/all_sites_tests.js
[modify] https://crrev.com/1c66a54bf7fa36978422408cccdad7b037648ab5/chrome/test/data/webui/settings/category_default_setting_tests.js
[modify] https://crrev.com/1c66a54bf7fa36978422408cccdad7b037648ab5/chrome/test/data/webui/settings/cr_settings_browsertest.js
[modify] https://crrev.com/1c66a54bf7fa36978422408cccdad7b037648ab5/chrome/test/data/webui/settings/site_details_permission_tests.js
[modify] https://crrev.com/1c66a54bf7fa36978422408cccdad7b037648ab5/chrome/test/data/webui/settings/site_details_tests.js
[modify] https://crrev.com/1c66a54bf7fa36978422408cccdad7b037648ab5/chrome/test/data/webui/settings/site_list_tests.js
[modify] https://crrev.com/1c66a54bf7fa36978422408cccdad7b037648ab5/chrome/test/data/webui/settings/test_site_settings_prefs_browser_proxy.js
[modify] https://crrev.com/1c66a54bf7fa36978422408cccdad7b037648ab5/chrome/test/data/webui/settings/test_util.js

Labels: OS-Chrome OS-Linux OS-Mac OS-Windows
Status: Fixed (was: Assigned)
:D

Sign in to add a comment