Commit 81163fe3 authored by Xianzhu Wang's avatar Xianzhu Wang Committed by Commit Bot

Simplification and improvement to flag-specific config

- Tighten config matching: the specified flags must start with all
  config flags to match. This avoids ambiguity when matching
  specified flags ["--aa", "--bb"] to 2 configs
    {
      "name": "a",
      "args": ["--aa"]
    },
    {
      "name": "b",
      "args": ["--bb"]
    },
  Now the first config will be matched, instead of exception raised.

- Allow multiple flags in additional-driver-flag.setting.

Bug: 1019501
Change-Id: I8018a14ac9562e7482e3a776730a2d6195f4355c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1897341
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: default avatarSteve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712169}
parent d54b5c6f
...@@ -266,17 +266,13 @@ class Port(object): ...@@ -266,17 +266,13 @@ class Port(object):
best_match = None best_match = None
configs = self._flag_specific_configs() configs = self._flag_specific_configs()
for name in configs: for name in configs:
# To match, the specified flags must contain all config args. # To match the specified flags must start with all config args.
args = configs[name] args = configs[name]
if any(arg not in specified_flags for arg in args): if specified_flags[:len(args)] != args:
continue continue
# The first config matching the highest number of specified flags wins. # The first config matching the highest number of specified flags wins.
if not best_match or len(configs[best_match]) < len(args): if not best_match or len(configs[best_match]) < len(args):
best_match = name best_match = name
else:
assert len(configs[best_match]) > len(args), (
'Ambiguous flag-specific configs {} and {}: they match the same number of additional driver args.'
.format(best_match, name))
if best_match: if best_match:
return best_match return best_match
...@@ -311,17 +307,14 @@ class Port(object): ...@@ -311,17 +307,14 @@ class Port(object):
def _specified_additional_driver_flags(self): def _specified_additional_driver_flags(self):
"""Returns the list of additional driver flags specified by the user in """Returns the list of additional driver flags specified by the user in
the following ways, concatenated: the following ways, concatenated:
1. A single flag in web_tests/additional-driver-flag.setting. If present, 1. Flags in web_tests/additional-driver-flag.setting.
this flag will be the first flag.
2. flags expanded from --flag-specific=<name> based on flag-specific config. 2. flags expanded from --flag-specific=<name> based on flag-specific config.
3. Zero or more flags passed by --additional-driver-flag. 3. Zero or more flags passed by --additional-driver-flag.
""" """
flags = [] flags = []
flag_file = self._filesystem.join(self.web_tests_dir(), 'additional-driver-flag.setting') flag_file = self._filesystem.join(self.web_tests_dir(), 'additional-driver-flag.setting')
if self._filesystem.exists(flag_file): if self._filesystem.exists(flag_file):
flag = self._filesystem.read_text_file(flag_file).strip() flags = self._filesystem.read_text_file(flag_file).split()
if flag:
flags = [flag]
flag_specific_option = self.get_option('flag_specific') flag_specific_option = self.get_option('flag_specific')
if flag_specific_option: if flag_specific_option:
......
...@@ -452,9 +452,9 @@ class PortTest(LoggingTestCase): ...@@ -452,9 +452,9 @@ class PortTest(LoggingTestCase):
port_c = self.make_port(options=optparse.Values( port_c = self.make_port(options=optparse.Values(
{'additional_driver_flag': ['--bb', '--cc']})) {'additional_driver_flag': ['--bb', '--cc']}))
port_c.host.filesystem.write_text_file(flag_file, '--bb') port_c.host.filesystem.write_text_file(flag_file, '--bb --dd')
# We don't remove duplicated flags at this time. # We don't remove duplicated flags at this time.
self.assertEqual(port_c._specified_additional_driver_flags(), ['--bb', '--bb', '--cc']); self.assertEqual(port_c._specified_additional_driver_flags(), ['--bb', '--dd', '--bb', '--cc']);
self.assertEqual(port_c._flag_specific_config_name(), 'bb') self.assertEqual(port_c._flag_specific_config_name(), 'bb')
def _write_flag_specific_config(self, port): def _write_flag_specific_config(self, port):
...@@ -462,8 +462,8 @@ class PortTest(LoggingTestCase): ...@@ -462,8 +462,8 @@ class PortTest(LoggingTestCase):
port.host.filesystem.join(port.web_tests_dir(), 'FlagSpecificConfig'), port.host.filesystem.join(port.web_tests_dir(), 'FlagSpecificConfig'),
'[' '['
' {"name": "a", "args": ["--aa"]},' ' {"name": "a", "args": ["--aa"]},'
' {"name": "b", "args": ["--aa", "--bb"]},' ' {"name": "b", "args": ["--bb", "--aa"]},'
' {"name": "c", "args": ["--aa", "--cc"]}' ' {"name": "c", "args": ["--bb", "--cc"]}'
']') ']')
def test_flag_specific_config_name_from_options_and_config(self): def test_flag_specific_config_name_from_options_and_config(self):
...@@ -478,40 +478,48 @@ class PortTest(LoggingTestCase): ...@@ -478,40 +478,48 @@ class PortTest(LoggingTestCase):
self._write_flag_specific_config(port_a2) self._write_flag_specific_config(port_a2)
self.assertEqual(port_a2._flag_specific_config_name(), 'a') self.assertEqual(port_a2._flag_specific_config_name(), 'a')
port_a3 = self.make_port(options=optparse.Values(
{'additional_driver_flag': ['--aa', '--bb']}))
self._write_flag_specific_config(port_a3)
self.assertEqual(port_a3._flag_specific_config_name(), 'a')
port_b1 = self.make_port(options=optparse.Values( port_b1 = self.make_port(options=optparse.Values(
{'additional_driver_flag': ['--bb']})) {'additional_driver_flag': ['--bb', '--aa']}))
self._write_flag_specific_config(port_b1) self._write_flag_specific_config(port_b1)
# No match. Fallback to first specified flag. self.assertEqual(port_b1._flag_specific_config_name(), 'b')
self.assertEqual(port_b1._flag_specific_config_name(), 'bb')
port_b2 = self.make_port(options=optparse.Values( port_b2 = self.make_port(options=optparse.Values(
{'additional_driver_flag': ['--aa', '--bb']})) {'additional_driver_flag': ['--bb', '--aa', '--cc']}))
self._write_flag_specific_config(port_b2) self._write_flag_specific_config(port_b2)
self.assertEqual(port_b2._flag_specific_config_name(), 'b') self.assertEqual(port_b2._flag_specific_config_name(), 'b')
port_b3 = self.make_port(options=optparse.Values( port_b3 = self.make_port(options=optparse.Values(
{'additional_driver_flag': ['--bb', '--aa']})) {'additional_driver_flag': ['--bb', '--aa', '--dd']}))
self._write_flag_specific_config(port_b3) self._write_flag_specific_config(port_b3)
self.assertEqual(port_b3._flag_specific_config_name(), 'b') self.assertEqual(port_b3._flag_specific_config_name(), 'b')
port_b4 = self.make_port(options=optparse.Values( port_c1 = self.make_port(options=optparse.Values(
{'additional_driver_flag': ['--bb', '--aa', '--dd']})) {'additional_driver_flag': ['--bb', '--cc']}))
self._write_flag_specific_config(port_b4) self._write_flag_specific_config(port_c1)
self.assertEqual(port_b4._flag_specific_config_name(), 'b') self.assertEqual(port_c1._flag_specific_config_name(), 'c')
def test_ambigous_flag_specific_match(self): port_c2 = self.make_port(options=optparse.Values(
port = self.make_port(options=optparse.Values( {'additional_driver_flag': ['--bb', '--cc', '--aa']}))
{'additional_driver_flag': ['--aa', '--bb', '--cc']})) self._write_flag_specific_config(port_c2)
self._write_flag_specific_config(port) self.assertEqual(port_c2._flag_specific_config_name(), 'c')
# pylint: disable=protected-access
self.assertRaises(AssertionError, port._flag_specific_config_name)
def test_flag_specific_fallback(self): def test_flag_specific_fallback(self):
port = self.make_port(options=optparse.Values( port_b = self.make_port(options=optparse.Values(
{'additional_driver_flag': ['--bb']}))
self._write_flag_specific_config(port_b)
# No match. Fallback to first specified flag.
self.assertEqual(port_b._flag_specific_config_name(), 'bb')
port_d = self.make_port(options=optparse.Values(
{'additional_driver_flag': ['--dd', '--ee']})) {'additional_driver_flag': ['--dd', '--ee']}))
self._write_flag_specific_config(port) self._write_flag_specific_config(port_d)
# pylint: disable=protected-access # pylint: disable=protected-access
self.assertEqual(port._flag_specific_config_name(), 'dd') self.assertEqual(port_d._flag_specific_config_name(), 'dd')
def test_flag_specific_option(self): def test_flag_specific_option(self):
port_a = self.make_port(options=optparse.Values({'flag_specific': 'a'})) port_a = self.make_port(options=optparse.Values({'flag_specific': 'a'}))
...@@ -522,7 +530,7 @@ class PortTest(LoggingTestCase): ...@@ -522,7 +530,7 @@ class PortTest(LoggingTestCase):
port_b = self.make_port(options=optparse.Values( port_b = self.make_port(options=optparse.Values(
{'flag_specific': 'a', 'additional_driver_flag': ['--bb']})) {'flag_specific': 'a', 'additional_driver_flag': ['--bb']}))
self._write_flag_specific_config(port_b) self._write_flag_specific_config(port_b)
self.assertEqual(port_b._flag_specific_config_name(), 'b') self.assertEqual(port_b._flag_specific_config_name(), 'a')
port_d = self.make_port(options=optparse.Values({'flag_specific': 'd'})) port_d = self.make_port(options=optparse.Values({'flag_specific': 'd'}))
self._write_flag_specific_config(port_d) self._write_flag_specific_config(port_d)
......
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