Commit ad77b789 authored by Paul Miller's avatar Paul Miller Committed by Commit Bot

Variations: Rename 'win' to 'windows' in testing config

This is to match server-side tools. Since GN uses "win", there must
still be some boundary where "win" is converted to "windows"; move the
conversion from fieldtrial_to_struct.py to field_trial_config/BUILD.gn.

Also add "android_webview" to the field trial config presubmit.

BUG=707911

Cq-Include-Trybots: master.tryserver.chromium.perf:obbs_fyi
Change-Id: I73c2d2fa98410e32ae77d053098d704ee40378ac
Reviewed-on: https://chromium-review.googlesource.com/1130348
Commit-Queue: Paul Miller <paulmiller@chromium.org>
Reviewed-by: default avatarAlexei Svitkine <asvitkine@chromium.org>
Reviewed-by: default avatarNed Nguyen <nednguyen@google.com>
Reviewed-by: default avatarTarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574297}
parent e226149a
...@@ -19,20 +19,26 @@ action("field_trial_testing_config_action") { ...@@ -19,20 +19,26 @@ action("field_trial_testing_config_action") {
"$target_gen_dir/$out_name.h", "$target_gen_dir/$out_name.h",
] ]
if (current_os == "win") {
platform = "windows"
} else {
platform = current_os
}
args = [ args = [
rebase_path(source, root_build_dir), rebase_path(source, root_build_dir),
"--destbase=" + rebase_path(target_gen_dir, root_build_dir), "--destbase=" + rebase_path(target_gen_dir, root_build_dir),
"--namespace=variations", "--namespace=variations",
"--schema=" + "--schema=" +
rebase_path("field_trial_testing_config_schema.json", root_build_dir), rebase_path("field_trial_testing_config_schema.json", root_build_dir),
"--platform=" + current_os, "--platform=" + platform,
"--output=$out_name", "--output=$out_name",
] ]
# At build-time, Chrome and WebView both use platform "android", but at # At build-time, Android Chrome and WebView both use platform "android", but
# run-time, variations hase separate platforms "android" and "android_webview". # at run-time, variations has separate platforms "android" and
# So if building "android", also include WebView. # "android_webview". So if building "android", also include WebView.
if (current_os == "android") { if (platform == "android") {
args += [ "--platform=android_webview" ] args += [ "--platform=android_webview" ]
} }
} }
......
...@@ -159,8 +159,8 @@ def ValidateData(json_data, file_path, message_type): ...@@ -159,8 +159,8 @@ def ValidateData(json_data, file_path, message_type):
if not isinstance(experiment_config['platforms'], list): if not isinstance(experiment_config['platforms'], list):
return _CreateMalformedConfigMessage(message_type, file_path, return _CreateMalformedConfigMessage(message_type, file_path,
'Expecting list for platforms in Study[%s]', study) 'Expecting list for platforms in Study[%s]', study)
supported_platforms = ['android', 'chromeos', 'ios', 'linux', 'mac', supported_platforms = ['android', 'android_webview', 'chromeos', 'ios',
'win'] 'linux', 'mac', 'windows']
experiment_platforms = experiment_config['platforms'] experiment_platforms = experiment_config['platforms']
unsupported_platforms = list(set(experiment_platforms).difference( unsupported_platforms = list(set(experiment_platforms).difference(
supported_platforms)) supported_platforms))
......
...@@ -50,7 +50,7 @@ Each *study configuration* is a dictionary containing `platforms` and ...@@ -50,7 +50,7 @@ Each *study configuration* is a dictionary containing `platforms` and
`experiments`. `experiments`.
`platforms` is an array of strings, indicating the targetted platforms. The `platforms` is an array of strings, indicating the targetted platforms. The
strings may be `android`, `chromeos`, `ios`, `linux`, `mac`, or `win`. strings may be `android`, `chromeos`, `ios`, `linux`, `mac`, or `windows`.
`experiments` is an array containing the *experiments*. `experiments` is an array containing the *experiments*.
...@@ -109,7 +109,7 @@ the form `//N` where `N` is between 0 and 9. ...@@ -109,7 +109,7 @@ the form `//N` where `N` is between 0 and 9.
{ {
"AStudyWithExperimentComment": [ "AStudyWithExperimentComment": [
{ {
"platforms": ["chromeos", "linux", "mac", "win"], "platforms": ["chromeos", "linux", "mac", "windows"],
"experiments": [ "experiments": [
{ {
"//0": "This is the first comment line.", "//0": "This is the first comment line.",
...@@ -129,7 +129,7 @@ Simply specify two different study configurations in the study: ...@@ -129,7 +129,7 @@ Simply specify two different study configurations in the study:
{ {
"DifferentExperimentsPerPlatform": [ "DifferentExperimentsPerPlatform": [
{ {
"platforms": ["chromeos", "linux", "mac", "win"], "platforms": ["chromeos", "linux", "mac", "windows"],
"experiments": [{ "name": "DesktopExperiment" }] "experiments": [{ "name": "DesktopExperiment" }]
}, },
{ {
......
...@@ -42,7 +42,7 @@ def GetExperimentArgs(): ...@@ -42,7 +42,7 @@ def GetExperimentArgs():
elif platform.system().lower() == 'linux': elif platform.system().lower() == 'linux':
my_platform = 'linux' my_platform = 'linux'
elif platform.system().lower() == 'windows': elif platform.system().lower() == 'windows':
my_platform = 'win' my_platform = 'windows'
elif platform.system().lower() == 'darwin': elif platform.system().lower() == 'darwin':
my_platform = 'mac' my_platform = 'mac'
else: else:
......
...@@ -104,7 +104,7 @@ class PerfBenchmark(benchmark.Benchmark): ...@@ -104,7 +104,7 @@ class PerfBenchmark(benchmark.Benchmark):
if target_os == 'darwin': if target_os == 'darwin':
return 'mac' return 'mac'
if target_os.startswith('win'): if target_os.startswith('win'):
return 'win' return 'windows'
if target_os.startswith('linux'): if target_os.startswith('linux'):
return 'linux' return 'linux'
return target_os return target_os
......
...@@ -29,16 +29,13 @@ _platforms = [ ...@@ -29,16 +29,13 @@ _platforms = [
'ios', 'ios',
'linux', 'linux',
'mac', 'mac',
'win', 'windows',
] ]
# Convert a platform argument to the matching Platform enum value in # Convert a platform argument to the matching Platform enum value in
# components/variations/proto/study.proto. # components/variations/proto/study.proto.
def _PlatformEnumValue(platform): def _PlatformEnumValue(platform):
assert platform in _platforms assert platform in _platforms
# TODO(crbug/707911): Remove the 'win' special case.
if platform == 'win':
return 'Study::PLATFORM_WINDOWS'
return 'Study::PLATFORM_' + platform.upper() return 'Study::PLATFORM_' + platform.upper()
def _Load(filename): def _Load(filename):
......
...@@ -14,7 +14,7 @@ class FieldTrialToStruct(unittest.TestCase): ...@@ -14,7 +14,7 @@ class FieldTrialToStruct(unittest.TestCase):
config = { config = {
'Trial1': [ 'Trial1': [
{ {
'platforms': ['win'], 'platforms': ['windows'],
'experiments': [ 'experiments': [
{ {
'name': 'Group1', 'name': 'Group1',
...@@ -39,13 +39,13 @@ class FieldTrialToStruct(unittest.TestCase): ...@@ -39,13 +39,13 @@ class FieldTrialToStruct(unittest.TestCase):
], ],
'Trial2': [ 'Trial2': [
{ {
'platforms': ['win'], 'platforms': ['windows'],
'experiments': [{'name': 'OtherGroup'}] 'experiments': [{'name': 'OtherGroup'}]
} }
], ],
'TrialWithForcingFlag': [ 'TrialWithForcingFlag': [
{ {
'platforms': ['win'], 'platforms': ['windows'],
'experiments': [ 'experiments': [
{ {
'name': 'ForcedGroup', 'name': 'ForcedGroup',
...@@ -56,7 +56,7 @@ class FieldTrialToStruct(unittest.TestCase): ...@@ -56,7 +56,7 @@ class FieldTrialToStruct(unittest.TestCase):
] ]
} }
result = fieldtrial_to_struct._FieldTrialConfigToDescription(config, result = fieldtrial_to_struct._FieldTrialConfigToDescription(config,
['win']) ['windows'])
expected = { expected = {
'elements': { 'elements': {
'kFieldTrialConfig': { 'kFieldTrialConfig': {
...@@ -115,7 +115,7 @@ class FieldTrialToStruct(unittest.TestCase): ...@@ -115,7 +115,7 @@ class FieldTrialToStruct(unittest.TestCase):
_MULTIPLE_PLATFORM_CONFIG = { _MULTIPLE_PLATFORM_CONFIG = {
'Trial1': [ 'Trial1': [
{ {
'platforms': ['win', 'ios'], 'platforms': ['windows', 'ios'],
'experiments': [ 'experiments': [
{ {
'name': 'Group1', 'name': 'Group1',
...@@ -148,7 +148,7 @@ class FieldTrialToStruct(unittest.TestCase): ...@@ -148,7 +148,7 @@ class FieldTrialToStruct(unittest.TestCase):
], ],
'Trial2': [ 'Trial2': [
{ {
'platforms': ['win', 'mac'], 'platforms': ['windows', 'mac'],
'experiments': [{'name': 'OtherGroup'}] 'experiments': [{'name': 'OtherGroup'}]
} }
] ]
...@@ -229,7 +229,7 @@ class FieldTrialToStruct(unittest.TestCase): ...@@ -229,7 +229,7 @@ class FieldTrialToStruct(unittest.TestCase):
fieldtrial_to_struct.main([ fieldtrial_to_struct.main([
'--schema=' + schema, '--schema=' + schema,
'--output=' + test_output_filename, '--output=' + test_output_filename,
'--platform=win', '--platform=windows',
'--year=2015', '--year=2015',
unittest_data_dir + 'test_config.json' unittest_data_dir + 'test_config.json'
]) ])
......
...@@ -109,7 +109,7 @@ def main(): ...@@ -109,7 +109,7 @@ def main():
print_shell_cmd = len(sys.argv) >= 4 and sys.argv[3] == 'shell_cmd' print_shell_cmd = len(sys.argv) >= 4 and sys.argv[3] == 'shell_cmd'
supported_platforms = ['android', 'android_webview', 'chromeos', 'ios', supported_platforms = ['android', 'android_webview', 'chromeos', 'ios',
'linux', 'mac', 'win'] 'linux', 'mac', 'windows']
if sys.argv[2] not in supported_platforms: if sys.argv[2] not in supported_platforms:
print ('\'%s\' is an unknown platform. Supported platforms: %s' % print ('\'%s\' is an unknown platform. Supported platforms: %s' %
(sys.argv[2], supported_platforms)) (sys.argv[2], supported_platforms))
......
...@@ -30,13 +30,13 @@ class FieldTrialUtilUnittest(unittest.TestCase): ...@@ -30,13 +30,13 @@ class FieldTrialUtilUnittest(unittest.TestCase):
config = '''{ config = '''{
"BrowserBlackList": [ "BrowserBlackList": [
{ {
"platforms": ["win"], "platforms": ["windows"],
"experiments": [{"name": "Enabled"}] "experiments": [{"name": "Enabled"}]
} }
], ],
"SimpleParams": [ "SimpleParams": [
{ {
"platforms": ["win"], "platforms": ["windows"],
"experiments": [ "experiments": [
{ {
"name": "Default", "name": "Default",
...@@ -48,7 +48,7 @@ class FieldTrialUtilUnittest(unittest.TestCase): ...@@ -48,7 +48,7 @@ class FieldTrialUtilUnittest(unittest.TestCase):
], ],
"c": [ "c": [
{ {
"platforms": ["win"], "platforms": ["windows"],
"experiments": [ "experiments": [
{ {
"name": "d.", "name": "d.",
...@@ -60,7 +60,7 @@ class FieldTrialUtilUnittest(unittest.TestCase): ...@@ -60,7 +60,7 @@ class FieldTrialUtilUnittest(unittest.TestCase):
} }
] ]
}''' }'''
result = self.runGenerateArgs(config, 'win') result = self.runGenerateArgs(config, 'windows')
self.assertEqual(['--force-fieldtrials=' self.assertEqual(['--force-fieldtrials='
'BrowserBlackList/Enabled/SimpleParams/Default/c/d.', 'BrowserBlackList/Enabled/SimpleParams/Default/c/d.',
'--force-fieldtrial-params=' '--force-fieldtrial-params='
...@@ -73,7 +73,7 @@ class FieldTrialUtilUnittest(unittest.TestCase): ...@@ -73,7 +73,7 @@ class FieldTrialUtilUnittest(unittest.TestCase):
config = '''{ config = '''{
"X": [ "X": [
{ {
"platforms": ["win"], "platforms": ["windows"],
"experiments": [ "experiments": [
{ {
"name": "x", "name": "x",
...@@ -84,7 +84,7 @@ class FieldTrialUtilUnittest(unittest.TestCase): ...@@ -84,7 +84,7 @@ class FieldTrialUtilUnittest(unittest.TestCase):
], ],
"Y": [ "Y": [
{ {
"platforms": ["win"], "platforms": ["windows"],
"experiments": [ "experiments": [
{ {
"name": "Default", "name": "Default",
...@@ -95,7 +95,7 @@ class FieldTrialUtilUnittest(unittest.TestCase): ...@@ -95,7 +95,7 @@ class FieldTrialUtilUnittest(unittest.TestCase):
] ]
}''' }'''
with self.assertRaises(Exception) as raised: with self.assertRaises(Exception) as raised:
self.runGenerateArgs(config, 'win') self.runGenerateArgs(config, 'windows')
self.assertEqual('Duplicate feature(s) in enable_features: x', self.assertEqual('Duplicate feature(s) in enable_features: x',
str(raised.exception)) str(raised.exception))
...@@ -103,7 +103,7 @@ class FieldTrialUtilUnittest(unittest.TestCase): ...@@ -103,7 +103,7 @@ class FieldTrialUtilUnittest(unittest.TestCase):
config = '''{ config = '''{
"X": [ "X": [
{ {
"platforms": ["win"], "platforms": ["windows"],
"experiments": [ "experiments": [
{ {
"name": "x", "name": "x",
...@@ -114,7 +114,7 @@ class FieldTrialUtilUnittest(unittest.TestCase): ...@@ -114,7 +114,7 @@ class FieldTrialUtilUnittest(unittest.TestCase):
], ],
"Y": [ "Y": [
{ {
"platforms": ["win"], "platforms": ["windows"],
"experiments": [ "experiments": [
{ {
"name": "Default", "name": "Default",
...@@ -125,7 +125,7 @@ class FieldTrialUtilUnittest(unittest.TestCase): ...@@ -125,7 +125,7 @@ class FieldTrialUtilUnittest(unittest.TestCase):
] ]
}''' }'''
with self.assertRaises(Exception) as raised: with self.assertRaises(Exception) as raised:
self.runGenerateArgs(config, 'win') self.runGenerateArgs(config, 'windows')
self.assertEqual('Duplicate feature(s) in enable_features: y, z', self.assertEqual('Duplicate feature(s) in enable_features: y, z',
str(raised.exception)) str(raised.exception))
...@@ -134,7 +134,7 @@ class FieldTrialUtilUnittest(unittest.TestCase): ...@@ -134,7 +134,7 @@ class FieldTrialUtilUnittest(unittest.TestCase):
config = '''{ config = '''{
"X": [ "X": [
{ {
"platforms": ["win"], "platforms": ["windows"],
"experiments": [ "experiments": [
{ {
"name": "x", "name": "x",
...@@ -145,7 +145,7 @@ class FieldTrialUtilUnittest(unittest.TestCase): ...@@ -145,7 +145,7 @@ class FieldTrialUtilUnittest(unittest.TestCase):
], ],
"Y": [ "Y": [
{ {
"platforms": ["win"], "platforms": ["windows"],
"experiments": [ "experiments": [
{ {
"name": "Default", "name": "Default",
...@@ -156,7 +156,7 @@ class FieldTrialUtilUnittest(unittest.TestCase): ...@@ -156,7 +156,7 @@ class FieldTrialUtilUnittest(unittest.TestCase):
] ]
}''' }'''
with self.assertRaises(Exception) as raised: with self.assertRaises(Exception) as raised:
self.runGenerateArgs(config, 'win') self.runGenerateArgs(config, 'windows')
self.assertEqual('Conflicting features set as both enabled and disabled: x', self.assertEqual('Conflicting features set as both enabled and disabled: x',
str(raised.exception)) str(raised.exception))
......
{ {
"TestTrial1": [ "TestTrial1": [
{ {
"platforms": ["win"], "platforms": ["windows"],
"experiments": [{"name": "TestGroup1"}] "experiments": [{"name": "TestGroup1"}]
} }
], ],
"TestTrial2": [ "TestTrial2": [
{ {
"platforms": ["win"], "platforms": ["windows"],
"experiments": [ "experiments": [
{ {
"name": "TestGroup2", "name": "TestGroup2",
...@@ -32,13 +32,13 @@ ...@@ -32,13 +32,13 @@
], ],
"TestTrial3": [ "TestTrial3": [
{ {
"platforms": ["win"], "platforms": ["windows"],
"experiments": [{"name": "TestGroup3", "enable_features": ["X"]}] "experiments": [{"name": "TestGroup3", "enable_features": ["X"]}]
} }
], ],
"TrialWithForcingFlag": [ "TrialWithForcingFlag": [
{ {
"platforms": ["win"], "platforms": ["windows"],
"experiments": [ "experiments": [
{ {
"name": "ForcedGroup", "name": "ForcedGroup",
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment