New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 637432 link

Starred by 2 users

Issue metadata

Status: Verified
Owner: ----
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Feature

Blocked on:
issue 629357



Sign in to add a comment

Remove support for run-by-value and the value argument.

Project Member Reported by scunning...@chromium.org, Aug 12 2016

Issue description

The EP Test Framework allows you to run a policy_* test case by specifying the expected policy value. But if that value is a list of strings, such as --args= 'value=chrome://settings,https://google.com'), it will fail to match the corresponding value as stored in the TEST_CASE dictionary. Thus, the test exits with error, e.g., "ERROR: No test case for value: 'chrome://settings'".

The values stored in the TEST_CASE dictionary are precisely formatted to meet the requirements of the DM Server. So, we can't change them. Instead, we need a better way of converting those values into a format that can be reliably compared to policy value given by the user in the command line args.

The value comparison is done in the class method _get_case_by_value(policy_value). It tries to compare a white-space stripped version of the user-given policy_value with a white-space stripped json-formatted version of the test case value.

The equivalence comparison in this method needs to be replaced with a better, more robust version that can handle list-of-values, as well continue to handle all of the following types of user-entered data:
1) simple string (e.g., for EnterpriseWebStoreURL, ChromeOsReleaseChannel)
2) integer value
3) boolean value
4) CSV string (e.g., for AllowedDomainsForApps)
5) python list (e.g., for CookiesAllowedForUrls, URLBlacklist),
6) python dictionary (e.g., for ProxySettings, UserAvatarImage, WallpaperImage, PowerManagementIdleSettings)
7) list of python dictionaries (e.g., for DeviceAppPack, ManagedBookmarks, UsbDetachableWhitelist)
8) json formatted string (e.g., for OpenNetworkConfiguration, DefaultPrinterSelection).

Final Word: Some have suggested removing support for run-by-value. If this suggested in adopted, then this problem goes away.
 
Owner: scunning...@chromium.org
Status: Assigned (was: Available)
Solution: Remove support for run-by-value.
Labels: -Type-Bug Type-Feature
Also, complete the TODO(scunningham) to remove support for case+value=None.
Blockedon: 629357
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 19 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/dae7d8a91a244fd75f79a93b8913c0f69d290546

commit dae7d8a91a244fd75f79a93b8913c0f69d290546
Author: Scott Cunningham <scunningham@chromium.org>
Date: Tue Aug 16 20:40:07 2016

Remove support for run-by-value and the value argument.

Version 1 of the Enterprise Base Class allowed a caller to run a
test case based on the expected policy value. This feature added
unwanted complexity to the framework, and did not work reliably
(the string value had to match exactly what was shown on the
chrome://policy page, but was not the same as what was sent to the
fake DM Server.

Support for run-by-value, and entire value argument, is removed by
this CL in version 2. Note that there are three tests in the EE2E
test framework that still run tests BY_VALUE. These should updated
to instead be run by test case name.

BUG= chromium:637432 
TEST=None

Change-Id: I8bcc6e7ea94e1f69da6af5affb7ff52844fd3ecc
Reviewed-on: https://chromium-review.googlesource.com/371563
Tested-by: Scott Cunningham <scunningham@chromium.org>
Reviewed-by: Katherine Threlkeld <kathrelkeld@chromium.org>
Reviewed-by: Scott Cunningham <scunningham@chromium.org>

[modify] https://crrev.com/dae7d8a91a244fd75f79a93b8913c0f69d290546/client/cros/enterprise_policy_base.py

Project Member

Comment 6 by bugdroid1@chromium.org, Aug 19 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/dae7d8a91a244fd75f79a93b8913c0f69d290546

commit dae7d8a91a244fd75f79a93b8913c0f69d290546
Author: Scott Cunningham <scunningham@chromium.org>
Date: Tue Aug 16 20:40:07 2016

Remove support for run-by-value and the value argument.

Version 1 of the Enterprise Base Class allowed a caller to run a
test case based on the expected policy value. This feature added
unwanted complexity to the framework, and did not work reliably
(the string value had to match exactly what was shown on the
chrome://policy page, but was not the same as what was sent to the
fake DM Server.

Support for run-by-value, and entire value argument, is removed by
this CL in version 2. Note that there are three tests in the EE2E
test framework that still run tests BY_VALUE. These should updated
to instead be run by test case name.

BUG= chromium:637432 
TEST=None

Change-Id: I8bcc6e7ea94e1f69da6af5affb7ff52844fd3ecc
Reviewed-on: https://chromium-review.googlesource.com/371563
Tested-by: Scott Cunningham <scunningham@chromium.org>
Reviewed-by: Katherine Threlkeld <kathrelkeld@chromium.org>
Reviewed-by: Scott Cunningham <scunningham@chromium.org>

[modify] https://crrev.com/dae7d8a91a244fd75f79a93b8913c0f69d290546/client/cros/enterprise_policy_base.py

Status: Fixed (was: Assigned)
Summary: Remove support for run-by-value and the value argument. (was: enterprise_policy_base.py self._get_case_by_value() does not work with some value types, such as list of strings.)
Labels: VerifyIn-54

Comment 9 by dchan@chromium.org, Oct 7 2016

Labels: VerifyIn-55
Cc: krishna...@chromium.org
Status: Verified (was: Fixed)
Owner: ----

Sign in to add a comment