Skip to content

Commit

Permalink
Update buildbot_steps lint to handle env variables
Browse files Browse the repository at this point in the history
servo/saltfs#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.
  • Loading branch information
aneeshusa committed Jan 8, 2018
1 parent 753e2bc commit d0abd1c
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 13 deletions.
1 change: 1 addition & 0 deletions python/requirements.txt
Expand Up @@ -15,6 +15,7 @@ pep8 == 1.5.7
pyflakes == 0.8.1

# For buildbot checking
voluptuous == 0.10.5
PyYAML == 3.12

# For test-webidl
Expand Down
33 changes: 22 additions & 11 deletions python/tidy/servo_tidy/tidy.py
Expand Up @@ -17,8 +17,10 @@
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
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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):
Expand Down
4 changes: 2 additions & 2 deletions python/tidy/servo_tidy_tests/test_tidy.py
Expand Up @@ -210,12 +210,12 @@ def test_yaml_with_duplicate_key(self):

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):
Expand Down

0 comments on commit d0abd1c

Please sign in to comment.