Update buildbot_steps lint to handle env variables

https://github.com/servo/saltfs/pull/687 added support
for specifying environment variables in `buildbot_steps.yml`.
Update the servo-tidy buildbot_steps.yml linter to reflect this.

Use the voluptuous Python library (BSD 3-clause license) for validation
in lieu of a much larger hand-written implementation.
Update the tidy self tests to take into account the new error messages.

2 files changed, 24 insertions, 13 deletions

diff --git a/python/tidy/servo_tidy/tidy.py b/python/tidy/servo_tidy/tidy.py
index 2f638abe099..e12ae4cdeb3 100644
--- a/python/tidy/servo_tidy/tidy.py
+++ b/python/tidy/servo_tidy/tidy.py
@@ -17,8 +17,10 @@ import re
 import StringIO
 import subprocess
 import sys
+
 import colorama
 import toml
+import voluptuous
 import yaml
 
 from licenseck import MPL, APACHE, COPYRIGHT, licenses_toml, licenses_dep_toml
@@ -775,15 +777,24 @@ def duplicate_key_yaml_constructor(loader, node, deep=False):
 
 
 def lint_buildbot_steps_yaml(mapping):
-    # Check for well-formedness of contents
-    # A well-formed buildbot_steps.yml should be a map to list of strings
-    for k in mapping.keys():
-        if not isinstance(mapping[k], list):
-            raise ValueError("Key '{}' maps to type '{}', but list expected".format(k, type(mapping[k]).__name__))
-
-        # check if value is a list of strings
-        for item in itertools.ifilter(lambda i: not isinstance(i, str), mapping[k]):
-            raise ValueError("List mapped to '{}' contains non-string element".format(k))
+    from voluptuous import Any, Extra, Required, Schema
+
+    # Note: dictionary keys are optional by default in voluptuous
+    env = Schema({Extra: str})
+    commands = Schema([str])
+    schema = Schema({
+        'env': env,
+        Extra: Any(
+            commands,
+            {
+                'env': env,
+                Required('commands'): commands,
+            },
+        ),
+    })
+
+    # Signals errors via exception throwing
+    schema(mapping)
 
 
 class SafeYamlLoader(yaml.SafeLoader):
@@ -811,8 +822,8 @@ def check_yaml(file_name, contents):
         yield (line, e)
     except KeyError as e:
         yield (None, "Duplicated Key ({})".format(e.message))
-    except ValueError as e:
-        yield (None, e.message)
+    except voluptuous.MultipleInvalid as e:
+        yield (None, str(e))
 
 
 def check_for_possible_duplicate_json_keys(key_value_pairs):
diff --git a/python/tidy/servo_tidy_tests/test_tidy.py b/python/tidy/servo_tidy_tests/test_tidy.py
index dd4c06db4c0..432d5c1117e 100644
--- a/python/tidy/servo_tidy_tests/test_tidy.py
+++ b/python/tidy/servo_tidy_tests/test_tidy.py
@@ -210,12 +210,12 @@ class CheckTidiness(unittest.TestCase):
 
     def test_non_list_mapped_buildbot_steps(self):
         errors = tidy.collect_errors_for_files(iterFile('non_list_mapping_buildbot_steps.yml'), [tidy.check_yaml], [], print_text=False)
-        self.assertEqual("Key 'non-list-key' maps to type 'str', but list expected", errors.next()[2])
+        self.assertEqual("expected a list for dictionary value @ data['non-list-key']", errors.next()[2])
         self.assertNoMoreErrors(errors)
 
     def test_non_string_list_mapping_buildbot_steps(self):
         errors = tidy.collect_errors_for_files(iterFile('non_string_list_buildbot_steps.yml'), [tidy.check_yaml], [], print_text=False)
-        self.assertEqual("List mapped to 'mapping_key' contains non-string element", errors.next()[2])
+        self.assertEqual("expected str @ data['mapping_key'][0]", errors.next()[2])
         self.assertNoMoreErrors(errors)
 
     def test_lock(self):