diff options
Diffstat (limited to 'lib')
| -rw-r--r-- | lib/spack/spack/config.py | 77 | ||||
| -rw-r--r-- | lib/spack/spack/test/config.py | 73 | ||||
| -rw-r--r-- | lib/spack/spack/test/conftest.py | 20 |
3 files changed, 140 insertions, 30 deletions
diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py index d54dd74d5a..ec37c0a972 100644 --- a/lib/spack/spack/config.py +++ b/lib/spack/spack/config.py @@ -187,7 +187,6 @@ class ConfigScope(object): return self.sections[section] def write_section(self, section): - import jsonschema filename = self.get_section_filename(section) data = self.get_section(section) try: @@ -195,8 +194,6 @@ class ConfigScope(object): with open(filename, 'w') as f: _validate_section(data, section_schemas[section]) syaml.dump(data, stream=f, default_flow_style=False) - except jsonschema.ValidationError as e: - raise ConfigSanityError(e, data) except (yaml.YAMLError, IOError) as e: raise ConfigFileError( "Error writing to config file: '%s'" % str(e)) @@ -531,8 +528,9 @@ def scopes(): def _validate_section_name(section): """Exit if the section is not a valid section.""" if section not in section_schemas: - tty.die("Invalid config section: '%s'. Options are: %s" - % (section, " ".join(section_schemas.keys()))) + raise ConfigSectionError( + "Invalid config section: '%s'. Options are: %s" + % (section, " ".join(section_schemas.keys()))) def _validate_section(data, schema): @@ -697,6 +695,10 @@ class ConfigError(SpackError): """Superclass for all Spack config related errors.""" +class ConfigSectionError(ConfigError): + """Error for referring to a bad config section name in a configuration.""" + + class ConfigFileError(ConfigError): """Issue reading or accessing a configuration file.""" @@ -705,10 +707,38 @@ class ConfigFormatError(ConfigError): """Raised when a configuration format does not match its schema.""" def __init__(self, validation_error, data): - # Try to get line number from erroneous instance and its parent - instance_mark = getattr(validation_error.instance, '_start_mark', None) - parent_mark = getattr(validation_error.parent, '_start_mark', None) - path = [str(s) for s in getattr(validation_error, 'path', None)] + location = '<unknown file>' + mark = self._get_mark(validation_error, data) + if mark: + location = '%s' % mark.name + if mark.line is not None: + location += ':%d' % (mark.line + 1) + + message = '%s: %s' % (location, validation_error.message) + super(ConfigError, self).__init__(message) + + def _get_mark(self, validation_error, data): + """Get the file/line mark fo a validation error from a Spack YAML file. + """ + def _get_mark_or_first_member_mark(obj): + # mark of object itelf + mark = getattr(obj, '_start_mark', None) + if mark: + return mark + + # mark of first member if it is a container + if isinstance(obj, (list, dict)): + first_member = next(iter(obj), None) + if first_member: + mark = getattr(first_member, '_start_mark', None) + if mark: + return mark + + # Try various places, starting with instance and parent + for obj in (validation_error.instance, validation_error.parent): + mark = _get_mark_or_first_member_mark(obj) + if mark: + return mark def get_path(path, data): if path: @@ -719,29 +749,18 @@ class ConfigFormatError(ConfigError): # Try really hard to get the parent (which sometimes is not # set) This digs it out of the validated structure if it's not # on the validation_error. - if path and not parent_mark: - parent_path = list(path)[:-1] - parent = get_path(parent_path, data) + path = validation_error.path + if path: + parent = get_path(list(path)[:-1], data) if path[-1] in parent: if isinstance(parent, dict): - keylist = parent.keys() + keylist = list(parent.keys()) elif isinstance(parent, list): keylist = parent idx = keylist.index(path[-1]) - parent_mark = getattr(keylist[idx], '_start_mark', None) - - if instance_mark: - location = '%s:%d' % (instance_mark.name, instance_mark.line + 1) - elif parent_mark: - location = '%s:%d' % (parent_mark.name, parent_mark.line + 1) - elif path: - location = 'At ' + ':'.join(path) - else: - location = '<unknown line>' - - message = '%s: %s' % (location, validation_error.message) - super(ConfigError, self).__init__(message) - + mark = getattr(keylist[idx], '_start_mark', None) + if mark: + return mark -class ConfigSanityError(ConfigFormatError): - """Same as ConfigFormatError, raised when config is written by Spack.""" + # give up and return None if nothing worked + return None diff --git a/lib/spack/spack/test/config.py b/lib/spack/spack/test/config.py index 7b1e7d0ee1..95cd941547 100644 --- a/lib/spack/spack/test/config.py +++ b/lib/spack/spack/test/config.py @@ -518,7 +518,7 @@ def test_internal_config_from_data(): def test_keys_are_ordered(): - + """Test that keys in Spack YAML files retain their order from the file.""" expected_order = ( 'bin', 'man', @@ -543,3 +543,74 @@ def test_keys_are_ordered(): for actual, expected in zip(prefix_inspections, expected_order): assert actual == expected + + +def test_config_format_error(mutable_config): + """This is raised when we try to write a bad configuration.""" + with pytest.raises(spack.config.ConfigFormatError): + spack.config.set('compilers', {'bad': 'data'}, scope='site') + + +def get_config_error(filename, schema, yaml_string): + """Parse a YAML string and return the resulting ConfigFormatError. + + Fail if there is no ConfigFormatError + """ + with open(filename, 'w') as f: + f.write(yaml_string) + + # parse and return error, or fail. + try: + spack.config._read_config_file(filename, schema) + except spack.config.ConfigFormatError as e: + return e + else: + pytest.fail('ConfigFormatError was not raised!') + + +def test_config_parse_dict_in_list(tmpdir): + with tmpdir.as_cwd(): + e = get_config_error( + 'repos.yaml', spack.schema.repos.schema, """\ +repos: +- https://foobar.com/foo +- https://foobar.com/bar +- error: + - abcdef +- https://foobar.com/baz +""") + assert "repos.yaml:4" in str(e) + + +def test_config_parse_str_not_bool(tmpdir): + with tmpdir.as_cwd(): + e = get_config_error( + 'config.yaml', spack.schema.config.schema, """\ +config: + verify_ssl: False + checksum: foobar + dirty: True +""") + assert "config.yaml:3" in str(e) + + +def test_config_parse_list_in_dict(tmpdir): + with tmpdir.as_cwd(): + e = get_config_error( + 'mirrors.yaml', spack.schema.mirrors.schema, """\ +mirrors: + foo: http://foobar.com/baz + bar: http://barbaz.com/foo + baz: http://bazfoo.com/bar + travis: [1, 2, 3] +""") + assert "mirrors.yaml:5" in str(e) + + +def test_bad_config_section(config): + """Test that getting or setting a bad section gives an error.""" + with pytest.raises(spack.config.ConfigSectionError): + spack.config.set('foobar', 'foobar') + + with pytest.raises(spack.config.ConfigSectionError): + spack.config.get('foobar') diff --git a/lib/spack/spack/test/conftest.py b/lib/spack/spack/test/conftest.py index d1f96451d4..71c4a95446 100644 --- a/lib/spack/spack/test/conftest.py +++ b/lib/spack/spack/test/conftest.py @@ -254,6 +254,26 @@ def config(configuration_dir): spack.package_prefs.PackagePrefs.clear_caches() +@pytest.fixture(scope='function') +def mutable_config(tmpdir_factory, configuration_dir, config): + """Like config, but tests can modify the configuration.""" + spack.package_prefs.PackagePrefs.clear_caches() + + mutable_dir = tmpdir_factory.mktemp('mutable_config').join('tmp') + configuration_dir.copy(mutable_dir) + + real_configuration = spack.config.config + + spack.config.config = spack.config.Configuration( + *[spack.config.ConfigScope(name, str(mutable_dir)) + for name in ['site', 'system', 'user']]) + + yield spack.config.config + + spack.config.config = real_configuration + spack.package_prefs.PackagePrefs.clear_caches() + + def _populate(mock_db): """Populate a mock database with packages. |