config add: infer type based on JSON schema validation errors (#27035)
- [x] Allow dding enumerated types and types whose default value is forbidden by the schema
- [x] Add a test for using enumerated types in the tests for `spack config add`
- [x] Make `config add` tests use the `mutable_config` fixture so they do not
      affect other tests
Co-authored-by: Todd Gamblin <tgamblin@llnl.gov>
			
			
This commit is contained in:
		 Massimiliano Culpo
					Massimiliano Culpo
				
			
				
					committed by
					
						 GitHub
						GitHub
					
				
			
			
				
	
			
			
			 GitHub
						GitHub
					
				
			
						parent
						
							20b7485cd4
						
					
				
				
					commit
					3eb52b48b8
				
			| @@ -1067,22 +1067,36 @@ def get_valid_type(path): | |||||||
|     path given, the priority order is ``list``, ``dict``, ``str``, ``bool``, |     path given, the priority order is ``list``, ``dict``, ``str``, ``bool``, | ||||||
|     ``int``, ``float``. |     ``int``, ``float``. | ||||||
|     """ |     """ | ||||||
|  |     types = { | ||||||
|  |         'array': list, | ||||||
|  |         'object': syaml.syaml_dict, | ||||||
|  |         'string': str, | ||||||
|  |         'boolean': bool, | ||||||
|  |         'integer': int, | ||||||
|  |         'number': float | ||||||
|  |     } | ||||||
|  | 
 | ||||||
|     components = process_config_path(path) |     components = process_config_path(path) | ||||||
|     section = components[0] |     section = components[0] | ||||||
|     for type in (list, syaml.syaml_dict, str, bool, int, float): | 
 | ||||||
|         try: |     # Use None to construct the test data | ||||||
|             ret = type() |     test_data = None | ||||||
|             test_data = ret |     for component in reversed(components): | ||||||
|             for component in reversed(components): |         test_data = {component: test_data} | ||||||
|                 test_data = {component: test_data} | 
 | ||||||
|             validate(test_data, section_schemas[section]) |     try: | ||||||
|             return ret |         validate(test_data, section_schemas[section]) | ||||||
|         except (ConfigFormatError, AttributeError): |     except (ConfigFormatError, AttributeError) as e: | ||||||
|             # This type won't validate, try the next one |         jsonschema_error = e.validation_error | ||||||
|             # Except AttributeError because undefined behavior of dict ordering |         if jsonschema_error.validator == 'type': | ||||||
|             # in python 3.5 can cause the validator to raise an AttributeError |             return types[jsonschema_error.validator_value]() | ||||||
|             # instead of a ConfigFormatError. |         elif jsonschema_error.validator == 'anyOf': | ||||||
|             pass |             for subschema in jsonschema_error.validator_value: | ||||||
|  |                 anyof_type = subschema.get('type') | ||||||
|  |                 if anyof_type is not None: | ||||||
|  |                     return types[anyof_type]() | ||||||
|  |     else: | ||||||
|  |         return type(None) | ||||||
|     raise ConfigError("Cannot determine valid type for path '%s'." % path) |     raise ConfigError("Cannot determine valid type for path '%s'." % path) | ||||||
| 
 | 
 | ||||||
| 
 | 
 | ||||||
| @@ -1301,6 +1315,7 @@ class ConfigFormatError(ConfigError): | |||||||
|     def __init__(self, validation_error, data, filename=None, line=None): |     def __init__(self, validation_error, data, filename=None, line=None): | ||||||
|         # spack yaml has its own file/line marks -- try to find them |         # spack yaml has its own file/line marks -- try to find them | ||||||
|         # we prioritize these over the inputs |         # we prioritize these over the inputs | ||||||
|  |         self.validation_error = validation_error | ||||||
|         mark = self._get_mark(validation_error, data) |         mark = self._get_mark(validation_error, data) | ||||||
|         if mark: |         if mark: | ||||||
|             filename = mark.name |             filename = mark.name | ||||||
|   | |||||||
| @@ -225,7 +225,7 @@ def test_config_with_c_argument(mutable_empty_config): | |||||||
|     # Add the path to the config |     # Add the path to the config | ||||||
|     config("add", args.config_vars[0], scope='command_line') |     config("add", args.config_vars[0], scope='command_line') | ||||||
|     output = config("get", 'config') |     output = config("get", 'config') | ||||||
|     assert "config:\n  install_root:\n  - root: /path/to/config.yaml" in output |     assert "config:\n  install_root:\n    root: /path/to/config.yaml" in output | ||||||
| 
 | 
 | ||||||
| 
 | 
 | ||||||
| def test_config_add_ordered_dict(mutable_empty_config): | def test_config_add_ordered_dict(mutable_empty_config): | ||||||
|   | |||||||
| @@ -279,21 +279,32 @@ def test_write_to_same_priority_file(mock_low_high_config, compiler_specs): | |||||||
| # Test setting config values via path in filename | # Test setting config values via path in filename | ||||||
| 
 | 
 | ||||||
| 
 | 
 | ||||||
| def test_add_config_path(): | def test_add_config_path(mutable_config): | ||||||
| 
 |  | ||||||
|     # Try setting a new install tree root |     # Try setting a new install tree root | ||||||
|     path = "config:install_tree:root:/path/to/config.yaml" |     path = "config:install_tree:root:/path/to/config.yaml" | ||||||
|     spack.config.add(path, scope="command_line") |     spack.config.add(path) | ||||||
|     set_value = spack.config.get('config')['install_tree']['root'] |     set_value = spack.config.get('config')['install_tree']['root'] | ||||||
|     assert set_value == '/path/to/config.yaml' |     assert set_value == '/path/to/config.yaml' | ||||||
| 
 | 
 | ||||||
|     # Now a package:all setting |     # Now a package:all setting | ||||||
|     path = "packages:all:compiler:[gcc]" |     path = "packages:all:compiler:[gcc]" | ||||||
|     spack.config.add(path, scope="command_line") |     spack.config.add(path) | ||||||
|     compilers = spack.config.get('packages')['all']['compiler'] |     compilers = spack.config.get('packages')['all']['compiler'] | ||||||
|     assert "gcc" in compilers |     assert "gcc" in compilers | ||||||
| 
 | 
 | ||||||
| 
 | 
 | ||||||
|  | @pytest.mark.regression('17543,23259') | ||||||
|  | def test_add_config_path_with_enumerated_type(mutable_config): | ||||||
|  |     spack.config.add("config:concretizer:clingo") | ||||||
|  |     assert spack.config.get('config')['concretizer'] == "clingo" | ||||||
|  | 
 | ||||||
|  |     spack.config.add("config:concretizer:original") | ||||||
|  |     assert spack.config.get('config')['concretizer'] == "original" | ||||||
|  | 
 | ||||||
|  |     with pytest.raises(spack.config.ConfigError): | ||||||
|  |         spack.config.add("config:concretizer:foo") | ||||||
|  | 
 | ||||||
|  | 
 | ||||||
| def test_add_config_filename(mock_low_high_config, tmpdir): | def test_add_config_filename(mock_low_high_config, tmpdir): | ||||||
| 
 | 
 | ||||||
|     config_yaml = tmpdir.join('config-filename.yaml') |     config_yaml = tmpdir.join('config-filename.yaml') | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user