Support proper merging of --enable-features args.

It's convenient to be able to specify this for both bots and in
individual test suite exceptions, where it's desired to turn on a
particular feature for all tests that are run on a particular
bot. Detect this situation and merge them into proper comma-sorted
lists.

Bug: 826341
Tbr: jbudorick@chromium.org
Change-Id: I11ff1c498182625dae3cfc5c2f0e5e0b4cc4bc5c
Reviewed-on: https://chromium-review.googlesource.com/1042960Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: Kenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555871}

testing/buildbot/generate_buildbot_json.py

@@ -282,6 +282,36 @@ class BBJSONGenerator(object):
       return []
     return exception.get('key_removals', {}).get(tester_name, [])
 
+  def maybe_fixup_args_array(self, arr):
+    # The incoming array of strings may be an array of command line
+    # arguments. To make it easier to turn on certain features per-bot
+    # or per-test-suite, look specifically for any --enable-features
+    # flags, and merge them into comma-separated lists. (This might
+    # need to be extended to handle other arguments in the future,
+    # too.)
+    enable_str = '--enable-features='
+    enable_str_len = len(enable_str)
+    enable_features_args = []
+    idx = 0
+    first_idx = -1
+    while idx < len(arr):
+      flag = arr[idx]
+      delete_current_entry = False
+      if flag.startswith(enable_str):
+        arg = flag[enable_str_len:]
+        enable_features_args.extend(arg.split(','))
+        if first_idx < 0:
+          first_idx = idx
+        else:
+          delete_current_entry = True
+      if delete_current_entry:
+        del arr[idx]
+      else:
+        idx += 1
+    if first_idx >= 0:
+      arr[first_idx] = enable_str + ','.join(enable_features_args)
+    return arr
+
   def dictionary_merge(self, a, b, path=None, update=True):
     """http://stackoverflow.com/questions/7204805/
         python-dictionaries-of-dictionaries-merge
@@ -301,7 +331,7 @@ class BBJSONGenerator(object):
           # arguments adjacent (like --time-out-ms [arg], etc.)
           if all(isinstance(x, str)
                  for x in itertools.chain(a[key], b[key])):
-            a[key] = a[key] + b[key]
+            a[key] = self.maybe_fixup_args_array(a[key] + b[key])
           else:
             # TODO(kbr): this only works properly if the two arrays are
             # the same length, which is currently always the case in the
@@ -327,10 +357,9 @@ class BBJSONGenerator(object):
     return a
 
   def initialize_args_for_test(self, generated_test, tester_config):
-    if 'args' in tester_config:
-      if 'args' not in generated_test:
-        generated_test['args'] = []
-      generated_test['args'].extend(tester_config['args'])
+    if 'args' in tester_config or 'args' in generated_test:
+      generated_test['args'] = self.maybe_fixup_args_array(
+        generated_test.get('args', []) + tester_config.get('args', []))
 
   def initialize_swarming_dictionary_for_test(self, generated_test,
                                               tester_config):

testing/buildbot/generate_buildbot_json_unittest.py

@@ -50,6 +50,24 @@ FOO_GTESTS_WATERFALL = """\
 ]
 """
 
+FOO_GTESTS_WITH_ENABLE_FEATURES_WATERFALL = """\
+[
+  {
+    'name': 'chromium.test',
+    'machines': {
+      'Fake Tester': {
+        'test_suites': {
+          'gtest_tests': 'foo_tests',
+        },
+        'args': [
+          '--enable-features=Baz',
+        ],
+      },
+    },
+  },
+]
+"""
+
 FOO_GTESTS_MULTI_DIMENSION_WATERFALL = """\
 [
   {
@@ -312,6 +330,18 @@ FOO_TEST_SUITE_WITH_ARGS = """\
 }
 """
 
+FOO_TEST_SUITE_WITH_ENABLE_FEATURES = """\
+{
+  'foo_tests': {
+    'foo_test': {
+      'args': [
+        '--enable-features=Foo,Bar',
+      ],
+    },
+  },
+}
+"""
+
 FOO_SCRIPT_SUITE = """\
 {
   'foo_scripts': {
@@ -637,6 +667,26 @@ MERGED_ARGS_OUTPUT = """\
 }
 """
 
+MERGED_ENABLE_FEATURES_OUTPUT = """\
+{
+  "AAAAA1 AUTOGENERATED FILE DO NOT EDIT": {},
+  "AAAAA2 See generate_buildbot_json.py to make changes": {},
+  "Fake Tester": {
+    "gtest_tests": [
+      {
+        "args": [
+          "--enable-features=Foo,Bar,Baz"
+        ],
+        "swarming": {
+          "can_use_on_swarming_builders": true
+        },
+        "test": "foo_test"
+      }
+    ]
+  }
+}
+"""
+
 MODIFIED_OUTPUT = """\
 {
   "AAAAA1 AUTOGENERATED FILE DO NOT EDIT": {},
@@ -995,6 +1045,13 @@ class UnitTest(unittest.TestCase):
     fbb.files['chromium.test.json'] = MERGED_ARGS_OUTPUT
     fbb.check_output_file_consistency(verbose=True)
 
+  def test_enable_features_arg_merges(self):
+    fbb = FakeBBGen(FOO_GTESTS_WITH_ENABLE_FEATURES_WATERFALL,
+                    FOO_TEST_SUITE_WITH_ENABLE_FEATURES,
+                    EMPTY_EXCEPTIONS)
+    fbb.files['chromium.test.json'] = MERGED_ENABLE_FEATURES_OUTPUT
+    fbb.check_output_file_consistency(verbose=True)
+
   def test_test_filtering(self):
     fbb = FakeBBGen(COMPOSITION_GTEST_SUITE_WATERFALL,
                     GOOD_COMPOSITION_TEST_SUITES,