generate_buildbot_json.py: Mixins must be sorted

Uses python AST parsing to ensure that the keys for swarming mixins are
sorted.

Change-Id: I450a683454903ad8a5a730a3abc3c9ef2c482f29
Reviewed-on: https://chromium-review.googlesource.com/1231832
Commit-Queue: Stephen Martinis <martiniss@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592271}

testing/buildbot/generate_buildbot_json.py

@@ -841,7 +841,7 @@ class BBJSONGenerator(object):
       'win32-dbg',
     ]
 
-  def check_input_file_consistency(self):
+  def check_input_file_consistency(self, verbose=False):
     self.load_configuration_files()
     self.check_composition_test_suites()
 
@@ -940,6 +940,58 @@ class BBJSONGenerator(object):
                      ' referenced in a waterfall, machine, or test suite.' % (
                          str(missing_mixins)))
 
+    self.check_input_files_sorting(verbose)
+
+  def check_input_files_sorting(self, verbose=False):
+    bad_files = []
+    # FIXME: Expand to other files. It's unclear if every other file should be
+    # similarly sorted.
+    for filename in ('swarming_mixins.pyl',):
+      parsed = ast.parse(self.read_file(self.pyl_file_path(filename)))
+
+      def type_assert(itm, typ): # pragma: no cover
+        if not isinstance(itm, typ):
+          raise BBGenErr(
+              'Invalid .pyl file %s. %s expected to be %s, is %s' % (
+                  filename, itm, typ, type(itm)))
+
+      # Must be a module.
+      type_assert(parsed, ast.Module)
+      module = parsed.body
+
+      # Only one expression in the module.
+      type_assert(module, list)
+      if len(module) != 1: # pragma: no cover
+        raise BBGenErr('Invalid .pyl file %s' % filename)
+      expr = module[0]
+      type_assert(expr, ast.Expr)
+
+      # Value should be a dictionary.
+      value = expr.value
+      type_assert(value, ast.Dict)
+
+      keys = []
+      # The keys of this dict are ordered as ordered in the file; normal python
+      # dictionary keys are given an arbitrary order, but since we parsed the
+      # file itself, the order as given in the file is preserved.
+      for key in value.keys:
+        type_assert(key, ast.Str)
+        keys.append(key.s)
+
+      if sorted(keys) != keys:
+        bad_files.append(filename)
+        if verbose: # pragma: no cover
+          for line in difflib.unified_diff(
+              sorted(keys),
+              keys):
+            print line
+
+    if bad_files:
+      raise BBGenErr(
+          'The following files have unsorted top level keys: %s' % (
+              ', '.join(bad_files)))
+
+
   def check_output_file_consistency(self, verbose=False):
     self.load_configuration_files()
     # All waterfalls must have been written by this script already.

testing/buildbot/generate_buildbot_json_unittest.py

@@ -209,6 +209,29 @@ FOO_GTESTS_TEST_MIXIN_WATERFALL = """\
 ]
 """
 
+FOO_GTESTS_SORTING_MIXINS_WATERFALL = """\
+[
+  {
+    'swarming_mixins': ['a_mixin', 'b_mixin', 'c_mixin'],
+    'name': 'chromium.test',
+    'machines': {
+      'Fake Tester': {
+        'swarming': {
+          'dimension_sets': [
+            {
+              'kvm': '1',
+            },
+          ],
+        },
+        'test_suites': {
+          'gtest_tests': 'foo_tests',
+        },
+      },
+    },
+  },
+]
+"""
+
 FOO_LINUX_GTESTS_WATERFALL = """\
 [
   {
@@ -1350,6 +1373,8 @@ consoles {
 }
 """
 
+# These mixins are invalid; if passed to check_input_file_consistency, they will
+# fail. These are used for output file consistency checks.
 SWARMING_MIXINS = """\
 {
   'dimension_mixin': {
@@ -1369,6 +1394,34 @@ SWARMING_MIXINS = """\
 }
 """
 
+SWARMING_MIXINS_UNSORTED = """\
+{
+  'b_mixin': {
+    'b': 'b',
+  },
+  'a_mixin': {
+    'a': 'a',
+  },
+  'c_mixin': {
+    'c': 'c',
+  },
+}
+"""
+
+SWARMING_MIXINS_SORTED = """\
+{
+  'a_mixin': {
+    'a': 'a',
+  },
+  'b_mixin': {
+    'b': 'b',
+  },
+  'c_mixin': {
+    'c': 'c',
+  },
+}
+"""
+
 class UnitTest(unittest.TestCase):
   def test_base_generator(self):
     # Only needed for complete code coverage.
@@ -1755,6 +1808,22 @@ class UnitTest(unittest.TestCase):
     with self.assertRaises(generate_buildbot_json.BBGenErr):
       fbb.check_output_file_consistency(verbose=True)
 
+  def test_swarming_mixins_must_be_sorted(self):
+    fbb = FakeBBGen(FOO_GTESTS_SORTING_MIXINS_WATERFALL,
+                    FOO_TEST_SUITE,
+                    EMPTY_PYL_FILE,
+                    SWARMING_MIXINS_UNSORTED,
+                    LUCI_MILO_CFG)
+    with self.assertRaises(generate_buildbot_json.BBGenErr):
+      fbb.check_input_file_consistency()
+
+    fbb = FakeBBGen(FOO_GTESTS_SORTING_MIXINS_WATERFALL,
+                    FOO_TEST_SUITE,
+                    EMPTY_PYL_FILE,
+                    SWARMING_MIXINS_SORTED,
+                    LUCI_MILO_CFG)
+    fbb.check_input_file_consistency()
+
 
 if __name__ == '__main__':
   unittest.main()

testing/buildbot/swarming_mixins.pyl

@@ -15,14 +15,14 @@
 # already exists.
 
 {
-  'linux-trusty': {
+  'arm_tester': {
     'dimensions': {
-      'os': 'Ubuntu-14.04',
+      'cpu': 'armv7l-32',
     }
   },
-  'arm_tester': {
+  'linux-trusty': {
     'dimensions': {
-      'cpu': 'armv7l-32',
+      'os': 'Ubuntu-14.04',
     }
-  }
+  },
 }