From e1b8d78aaa096c136d85f4a14acdde1e5cae23ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20Prchl=C3=ADk?= Date: Wed, 22 Feb 2023 17:21:20 +0100 Subject: [PATCH 1/5] Expose "key address" to normalization callbacks Keys belong to various objects like step phases or fmf ids, and tmt can do better job when reporting unexpected input values. Another step is to expose the location of the key to normalization callbacks and a dedicated exception for reporting the violations in a unified manner. The key address is already used by schema validation when reporting validation errors, and while it's often subpar, it enhances error messages a bit. In the follow-up patches, improving the addresses will happen - for example, sometimes a class name is printed instead of a fmf node name, usually because the node name is not available. --- tests/core/fmf-id/test.sh | 5 +-- tests/unit/test_base.py | 5 ++- tests/unit/test_dataclasses.py | 38 ++++++++++---------- tmt/base.py | 41 +++++++++++---------- tmt/libraries/beakerlib.py | 4 +-- tmt/steps/__init__.py | 3 +- tmt/steps/discover/fmf.py | 4 +-- tmt/steps/discover/shell.py | 14 ++++---- tmt/steps/prepare/install.py | 9 +++-- tmt/steps/provision/testcloud.py | 6 ++-- tmt/utils.py | 62 +++++++++++++++++++++++++------- 11 files changed, 117 insertions(+), 74 deletions(-) diff --git a/tests/core/fmf-id/test.sh b/tests/core/fmf-id/test.sh index 78cee8df36..a70d60cdb8 100755 --- a/tests/core/fmf-id/test.sh +++ b/tests/core/fmf-id/test.sh @@ -14,7 +14,7 @@ rlJournalStart rlRun -s "tmt plan -vvvv show /plan-with-invalid-ref" 2 rlAssertGrep "warn: /plan-with-invalid-ref:discover .* is not valid under any of the given schemas" $rlRun_LOG - rlAssertGrep "Failed to load step data for DiscoverFmfStepData: The 'ref' field must be a string, got 'int'." $rlRun_LOG + rlAssertGrep "Failed to load step data for DiscoverFmfStepData: Field 'DiscoverFmfStepData:ref' can be string, 'int' found." $rlRun_LOG rlRun -s "tmt plan -vvvv show /remote-plan-with-valid-ref" @@ -28,7 +28,8 @@ rlJournalStart rlAssertGrep "some-package" $rlRun_LOG rlRun -s "tmt test -vvvv show /test-with-invalid-ref" 2 - rlAssertGrep "The 'ref' field must be a string, got 'int'." $rlRun_LOG + rlAssertGrep "warn: /test-with-invalid-ref:require .* is not valid under any of the given schemas" $rlRun_LOG + rlAssertGrep "Field 'ref' can be not set or string, 'int' found." $rlRun_LOG rlPhaseEnd rlPhaseStartCleanup diff --git a/tests/unit/test_base.py b/tests/unit/test_base.py index e164b9f9e5..e27f91ccfd 100644 --- a/tests/unit/test_base.py +++ b/tests/unit/test_base.py @@ -139,7 +139,10 @@ def test_link(): note=fmf_id['note'])] # Invalid links and relations - with pytest.raises(SpecificationError, match='Invalid link'): + with pytest.raises( + SpecificationError, + match="Field 'link' can be string, fmf id or list of their commbinations," + " 'int' found."): Links(data=123) with pytest.raises(SpecificationError, match='Multiple relations'): Links(data=dict(verifies='one', blocks='another')) diff --git a/tests/unit/test_dataclasses.py b/tests/unit/test_dataclasses.py index 8725bc5b50..4fd33685c9 100644 --- a/tests/unit/test_dataclasses.py +++ b/tests/unit/test_dataclasses.py @@ -16,7 +16,7 @@ def test_sanity(): def test_field_normalize_callback(root_logger: tmt.log.Logger) -> None: - def _normalize_foo(raw_value: Any, logger: tmt.log.Logger) -> int: + def _normalize_foo(key_address: str, raw_value: Any, logger: tmt.log.Logger) -> int: if raw_value is None: return None @@ -24,9 +24,8 @@ def _normalize_foo(raw_value: Any, logger: tmt.log.Logger) -> int: return int(raw_value) except ValueError as exc: - raise tmt.utils.SpecificationError( - "Field 'foo' can be either unset or integer," - f" '{type(raw_value).__name__}' found.") from exc + raise tmt.utils.NormalizationError(key_address, raw_value, 'not set or integer') \ + from exc @dataclasses.dataclass class DummyContainer(SerializableContainer): @@ -39,25 +38,25 @@ class DummyContainer(SerializableContainer): data = DummyContainer() assert data.foo == 1 - dataclass_normalize_field(data, 'foo', None, root_logger) + dataclass_normalize_field(data, ':foo', 'foo', None, root_logger) assert data.foo is None - dataclass_normalize_field(data, 'foo', 2, root_logger) + dataclass_normalize_field(data, ':foo', 'foo', 2, root_logger) assert data.foo == 2 - dataclass_normalize_field(data, 'foo', '3', root_logger) + dataclass_normalize_field(data, ':foo', 'foo', '3', root_logger) assert data.foo == 3 with pytest.raises( tmt.utils.SpecificationError, - match=r"Field 'foo' can be either unset or integer, 'str' found."): - dataclass_normalize_field(data, 'foo', 'will crash', root_logger) + match=r"Field ':foo' can be not set or integer, 'str' found."): + dataclass_normalize_field(data, ':foo', 'foo', 'will crash', root_logger) assert data.foo == 3 def test_field_normalize_special_method(root_logger: tmt.log.Logger) -> None: - def normalize_foo(cls, raw_value: Any, logger: tmt.log.Logger) -> int: + def normalize_foo(cls, key_address: str, raw_value: Any, logger: tmt.log.Logger) -> int: if raw_value is None: return None @@ -65,9 +64,8 @@ def normalize_foo(cls, raw_value: Any, logger: tmt.log.Logger) -> int: return int(raw_value) except ValueError as exc: - raise tmt.utils.SpecificationError( - "Field 'foo' can be either unset or integer," - f" '{type(raw_value).__name__}' found.") from exc + raise tmt.utils.NormalizationError(key_address, raw_value, 'not set or integer') \ + from exc @dataclasses.dataclass class DummyContainer(SerializableContainer): @@ -81,19 +79,19 @@ class DummyContainer(SerializableContainer): data = DummyContainer() assert data.foo == 1 - dataclass_normalize_field(data, 'foo', None, root_logger) + dataclass_normalize_field(data, ':foo', 'foo', None, root_logger) assert data.foo is None - dataclass_normalize_field(data, 'foo', 2, root_logger) + dataclass_normalize_field(data, ':foo', 'foo', 2, root_logger) assert data.foo == 2 - dataclass_normalize_field(data, 'foo', '3', root_logger) + dataclass_normalize_field(data, ':foo', 'foo', '3', root_logger) assert data.foo == 3 with pytest.raises( tmt.utils.SpecificationError, - match=r"Field 'foo' can be either unset or integer, 'str' found."): - dataclass_normalize_field(data, 'foo', 'will crash', root_logger) + match=r"Field ':foo' can be not set or integer, 'str' found."): + dataclass_normalize_field(data, ':foo', 'foo', 'will crash', root_logger) assert data.foo == 3 @@ -110,9 +108,9 @@ class DummyContainer(SerializableContainer): foo_metadata.normalize_callback = MagicMock() - dataclass_normalize_field(data, 'foo', 'will crash', root_logger) + dataclass_normalize_field(data, ':foo', 'foo', 'will crash', root_logger) - foo_metadata.normalize_callback.assert_called_once_with('will crash', root_logger) + foo_metadata.normalize_callback.assert_called_once_with(':foo', 'will crash', root_logger) data._normalize_foo.assert_not_called() diff --git a/tmt/base.py b/tmt/base.py index 7fa98baf81..4552f66bb0 100644 --- a/tmt/base.py +++ b/tmt/base.py @@ -161,8 +161,8 @@ def from_spec(cls, raw: _RawFmfId) -> 'FmfId': # TODO: with mandatory validation, this can go away. ref = raw.get('ref', None) if not isinstance(ref, (type(None), str)): - raise tmt.utils.SpecificationError( - f"The 'ref' field must be a string, got '{type(ref).__name__}'.") + # TODO: deliver better key address + raise tmt.utils.NormalizationError('ref', ref, 'not set or string') fmf_id = FmfId() @@ -374,8 +374,8 @@ def from_spec(cls, raw: _RawDependencyFmfId) -> 'DependencyFmfId': # type: igno # TODO: with mandatory validation, this can go away. ref = raw.get('ref', None) if not isinstance(ref, (type(None), str)): - raise tmt.utils.SpecificationError( - f"The 'ref' field must be a string, got '{type(ref).__name__}'.") + # TODO: deliver better key address + raise tmt.utils.NormalizationError('ref', ref, 'not set or string') fmf_id = DependencyFmfId() @@ -486,6 +486,7 @@ def dependency_factory(raw_dependency: Optional[_RawDependencyItem]) -> Dependen def normalize_require( + key_address: str, raw_require: Optional[_RawDependency], logger: tmt.log.Logger) -> List[Dependency]: """ @@ -537,7 +538,7 @@ def assert_simple_dependencies( CoreT = TypeVar('CoreT', bound='Core') -def _normalize_link(value: _RawLinks, logger: tmt.log.Logger) -> 'Links': +def _normalize_link(key_address: str, value: _RawLinks, logger: tmt.log.Logger) -> 'Links': return Links(data=value) @@ -560,7 +561,8 @@ class Core( enabled: bool = True order: int = field( default=DEFAULT_ORDER, - normalize=lambda raw_value, logger: DEFAULT_ORDER if raw_value is None else int(raw_value)) + normalize=lambda key_address, raw_value, logger: + DEFAULT_ORDER if raw_value is None else int(raw_value)) link: Optional['Links'] = field( default=None, normalize=_normalize_link) @@ -570,10 +572,11 @@ class Core( normalize=tmt.utils.normalize_string_list) tier: Optional[str] = field( default=None, - normalize=lambda raw_value, logger: None if raw_value is None else str(raw_value)) + normalize=lambda key_address, raw_value, logger: + None if raw_value is None else str(raw_value)) adjust: Optional[List[_RawAdjustRule]] = field( default_factory=list, - normalize=lambda raw_value, logger: [] if raw_value is None + normalize=lambda key_address, raw_value, logger: [] if raw_value is None else ([raw_value] if not isinstance(raw_value, list) else raw_value)) _KEYS_SHOW_ORDER = [ @@ -902,10 +905,12 @@ class Test( # `test` is mandatory, must exist, so how to initialize if it's missing :( test: Optional[ShellScript] = field( default=None, - normalize=lambda raw_value, logger: None if raw_value is None else ShellScript(raw_value)) + normalize=lambda key_address, raw_value, logger: + None if raw_value is None else ShellScript(raw_value)) path: Optional[Path] = field( default=None, - normalize=lambda raw_value, logger: None if raw_value is None else Path(raw_value)) + normalize=lambda key_address, raw_value, logger: + None if raw_value is None else Path(raw_value)) framework: str = "shell" manual: bool = False require: List[Dependency] = field( @@ -1496,9 +1501,8 @@ def _get_environment_vars(self, node: fmf.Tree) -> EnvironmentType: # Environment variables from files environment_files = node.get("environment-file") or [] if not isinstance(environment_files, list): - raise tmt.utils.SpecificationError( - f"The 'environment-file' should be a list. " - f"Received '{type(environment_files).__name__}'.") + raise tmt.utils.NormalizationError( + f'{self.name}:environment-file', environment_files, 'list of paths') combined = tmt.utils.environment_files_to_dict( filenames=environment_files, root=Path(node.root) if node.root else None, @@ -2045,8 +2049,8 @@ class Story( title: Optional[str] = None priority: Optional[StoryPriority] = field( default=None, - normalize=lambda raw_value, - logger: None if raw_value is None else StoryPriority(raw_value)) + normalize=lambda key_address, raw_value, logger: + None if raw_value is None else StoryPriority(raw_value)) _KEYS_SHOW_ORDER = [ 'summary', @@ -3483,10 +3487,9 @@ def __init__(self, *, data: Optional[_RawLinks] = None): # TODO: this should not happen with mandatory validation if data is not None and not isinstance(data, (str, dict, list)): - raise tmt.utils.SpecificationError( - "Invalid link specification " - "(should be a string, fmf id or list of their combinations), " - f"got '{type(data).__name__}'.") + # TODO: deliver better key address, needs to know the parent + raise tmt.utils.NormalizationError( + 'link', data, 'string, fmf id or list of their commbinations') # Nothing to do if no data provided if data is None: diff --git a/tmt/libraries/beakerlib.py b/tmt/libraries/beakerlib.py index f49cafa53f..c7c27c0647 100644 --- a/tmt/libraries/beakerlib.py +++ b/tmt/libraries/beakerlib.py @@ -265,9 +265,9 @@ def fetch(self) -> None: raise tmt.utils.GeneralError( f"Library '{self.name}' not found in '{self.repo}'.") self.require = tmt.base.normalize_require( - library_node.get('require', []), self.parent._logger) + f'{self.name}:require', library_node.get('require', []), self.parent._logger) self.recommend = tmt.base.normalize_require( - library_node.get('recommend', []), self.parent._logger) + f'{self.name}:recommend', library_node.get('recommend', []), self.parent._logger) # Create a symlink if the library is deep in the structure # FIXME: hot fix for https://github.com/beakerlib/beakerlib/pull/72 diff --git a/tmt/steps/__init__.py b/tmt/steps/__init__.py index df38ffae31..ffa05a2161 100644 --- a/tmt/steps/__init__.py +++ b/tmt/steps/__init__.py @@ -999,7 +999,8 @@ def _update_data_from_options(self, keys: Optional[List[str]] = None) -> None: if value is None or value == [] or value == () or value is False: continue - tmt.utils.dataclass_normalize_field(self.data, keyname, value, self._logger) + tmt.utils.dataclass_normalize_field( + self.data, f'{self.name}:{keyname}', keyname, value, self._logger) def wake(self) -> None: """ diff --git a/tmt/steps/discover/fmf.py b/tmt/steps/discover/fmf.py index b49366ce93..6a8fe3e330 100644 --- a/tmt/steps/discover/fmf.py +++ b/tmt/steps/discover/fmf.py @@ -50,14 +50,14 @@ class DiscoverFmfStepData(tmt.steps.discover.DiscoverStepData): # TODO: with mandatory validation, this can go away. def _normalize_ref( self, + key_address: str, value: Optional[Any], logger: tmt.log.Logger) -> Optional[str]: if value is None: return None if not isinstance(value, str): - raise tmt.utils.SpecificationError( - f"The 'ref' field must be a string, got '{type(value).__name__}'.") + raise tmt.utils.NormalizationError(key_address, value, 'string') return value diff --git a/tmt/steps/discover/shell.py b/tmt/steps/discover/shell.py index 6f5ec2531b..35d1468570 100644 --- a/tmt/steps/discover/shell.py +++ b/tmt/steps/discover/shell.py @@ -39,7 +39,7 @@ class TestDescription( # soon as possible - nobody want's to keep two very same lists of attributes. test: ShellScript = field( default=ShellScript(''), - normalize=lambda raw_value, logger: ShellScript(raw_value), + normalize=lambda key_address, raw_value, logger: ShellScript(raw_value), serialize=lambda test: str(test), unserialize=lambda serialized_test: ShellScript(serialized_test) ) @@ -51,11 +51,12 @@ class TestDescription( order: int = field( # TODO: ugly circular dependency (see tmt.base.DEFAULT_ORDER) default=50, - normalize=lambda raw_value, logger: 50 if raw_value is None else int(raw_value) + normalize=lambda key_address, raw_value, logger: + 50 if raw_value is None else int(raw_value) ) link: Optional[tmt.base.Links] = field( default=None, - normalize=lambda raw_value, logger: tmt.base.Links(data=raw_value), + normalize=lambda key_address, raw_value, logger: tmt.base.Links(data=raw_value), # Using `to_spec()` on purpose: `Links` does not provide serialization # methods, because specification of links is already good enough. We # can use existing `to_spec()` method, and undo it with a simple @@ -74,11 +75,12 @@ class TestDescription( ) tier: Optional[str] = field( default=None, - normalize=lambda raw_value, logger: None if raw_value is None else str(raw_value) + normalize=lambda key_address, raw_value, logger: + None if raw_value is None else str(raw_value) ) adjust: Optional[List[tmt.base._RawAdjustRule]] = field( default=None, - normalize=lambda raw_value, logger: [] if raw_value is None else ( + normalize=lambda key_address, raw_value, logger: [] if raw_value is None else ( [raw_value] if not isinstance(raw_value, list) else raw_value ) ) @@ -149,7 +151,7 @@ def to_spec(self) -> Dict[str, Any]: class DiscoverShellData(tmt.steps.discover.DiscoverStepData): tests: List[TestDescription] = field( default_factory=list, - normalize=lambda raw_value, logger: [ + normalize=lambda key_address, raw_value, logger: [ TestDescription.from_spec(raw_datum, logger) for raw_datum in cast(List[Dict[str, Any]], raw_value) ], diff --git a/tmt/steps/prepare/install.py b/tmt/steps/prepare/install.py index 33657c9c3e..7911d61297 100644 --- a/tmt/steps/prepare/install.py +++ b/tmt/steps/prepare/install.py @@ -473,11 +473,10 @@ class PrepareInstallData(tmt.steps.prepare.PrepareStepData): multiple=True, help='Package name or path to rpm to be installed.', # PrepareInstall supports *simple* requirements only - normalize=lambda value, logger: tmt.base.assert_simple_dependencies( - tmt.base.normalize_require(value, logger), + normalize=lambda key_address, value, logger: tmt.base.assert_simple_dependencies( + tmt.base.normalize_require(key_address, value, logger), "'install' plugin support simple packages only, no fmf links are allowed", - logger - ), + logger), serialize=lambda packages: [package.to_spec() for package in packages], unserialize=lambda serialized: [ tmt.base.DependencySimple.from_spec(package) @@ -522,7 +521,7 @@ class PrepareInstallData(tmt.steps.prepare.PrepareStepData): ) -@tmt.steps.provides_method('install') +@ tmt.steps.provides_method('install') class PrepareInstall(tmt.steps.prepare.PreparePlugin): """ Install packages on the guest diff --git a/tmt/steps/provision/testcloud.py b/tmt/steps/provision/testcloud.py index 27ae1999ef..1b9cc50feb 100644 --- a/tmt/steps/provision/testcloud.py +++ b/tmt/steps/provision/testcloud.py @@ -635,9 +635,9 @@ def go(self) -> None: if value is not None: try: setattr(data, int_key, int(value)) - except ValueError: - raise tmt.utils.SpecificationError( - f"Value '{value}' cannot be converted to int for '{int_key}' attribute.") + except ValueError as exc: + raise tmt.utils.NormalizationError( + f'{self.name}:{int_key}', value, 'integer') from exc for key, value in data.items(): if key == 'memory': diff --git a/tmt/utils.py b/tmt/utils.py index 70f0bfb292..2ffe5a7c00 100644 --- a/tmt/utils.py +++ b/tmt/utils.py @@ -1326,6 +1326,39 @@ def __init__( self.validation_errors = validation_errors +class NormalizationError(SpecificationError): + """ Raised when a key normalization fails """ + + def __init__( + self, + key_address: str, + raw_value: Any, + expected_type: str, + *args: Any, + **kwargs: Any) -> None: + """ + Raised when a key normalization fails. + + A subclass of :py:class:`SpecificationError`, but describing errors + that appear in a very specific point of key loading in a unified manner. + + :param key_address: the key in question, preferrably with detailed location, + e.g. ``/plans/foo:discover[0].tests``. + :param raw_value: input value, the one that failed the normalization. + :param expected_type: string description of expected, allowed types, as + a hint in the error message. + """ + + super().__init__( + f"Field '{key_address}' can be {expected_type}, '{type(raw_value).__name__}' found.", + *args, + **kwargs) + + self.key_address = key_address + self.raw_value = raw_value + self.expected_type = expected_type + + class ConvertError(MetadataError): """ Metadata conversion error """ @@ -4203,11 +4236,12 @@ def __init__( # A type representing compatible sources of keys and values. KeySource = Union[Dict[str, Any], fmf.Tree] -NormalizeCallback = Callable[[Any, tmt.log.Logger], T] +NormalizeCallback = Callable[[str, Any, tmt.log.Logger], T] def dataclass_normalize_field( container: Any, + key_address: str, keyname: str, raw_value: Any, logger: tmt.log.Logger) -> Any: @@ -4231,7 +4265,7 @@ def dataclass_normalize_field( normalize_callback = getattr(container, f'_normalize_{keyname}', None) if normalize_callback: - value = normalize_callback(raw_value, logger) + value = normalize_callback(key_address, raw_value, logger) else: value = raw_value @@ -4244,7 +4278,7 @@ def dataclass_normalize_field( # Keep for debugging purposes, as long as normalization settles down. if value is None or value == [] or value == (): logger.debug( - 'field normalized to false-ish value', + f'field "{key_address}" normalized to false-ish value', f'{container.__class__.__name__}.{keyname}', level=4, topic=tmt.log.Topic.KEY_NORMALIZATION) @@ -4286,6 +4320,7 @@ def dataclass_normalize_field( def normalize_string_list( + key_address: str, value: Union[None, str, List[str]], logger: tmt.log.Logger) -> List[str]: """ @@ -4315,6 +4350,7 @@ def normalize_string_list( def normalize_path_list( + key_address: str, value: Union[None, str, List[str]], logger: tmt.log.Logger) -> List[Path]: """ @@ -4346,12 +4382,11 @@ def normalize_path_list( if isinstance(value, (list, tuple)): return [Path(path) for path in value] - # TODO: propagate field name down to normalization callbacks for better exceptions - raise SpecificationError( - f"Field can be either path or list of paths, '{type(value).__name__}' found.") + raise NormalizationError(key_address, value, 'path or list of paths') def normalize_shell_script_list( + key_address: str, value: Union[None, str, List[str]], logger: tmt.log.Logger) -> List[ShellScript]: """ @@ -4383,12 +4418,11 @@ def normalize_shell_script_list( if isinstance(value, (list, tuple)): return [ShellScript(str(item)) for item in value] - # TODO: propagate field name down to normalization callbacks for better exceptions - raise SpecificationError( - f"Field can be either string or list of strings, '{type(value).__name__}' found.") + raise NormalizationError(key_address, value, 'string or list of strings') def normalize_fmf_context( + key_address: str, value: Optional[Dict[Any, Any]], logger: tmt.log.Logger) -> FmfContextType: if value is None: @@ -4433,6 +4467,7 @@ class NormalizeKeysMixin(_CommonBase): # It would require a clone of dataclass.field() though. def _normalize_string_list( self, + key_address: str, value: Union[None, str, List[str]], logger: tmt.log.Logger) -> List[str]: if value is None: @@ -4444,11 +4479,11 @@ def _normalize_string_list( if isinstance(value, (list, tuple)): return [item for item in value] - raise SpecificationError( - f"Field can be either string or list of strings, '{type(value).__name__}' found.") + raise NormalizationError(key_address, value, 'string or list of strings') def _normalize_environment( self, + key_address: str, value: Optional[Dict[str, Any]], logger: tmt.log.Logger) -> EnvironmentType: if value is None: @@ -4460,11 +4495,12 @@ def _normalize_environment( def _normalize_script( self, + key_address: str, value: Union[None, str, List[str]], logger: tmt.log.Logger) -> List[ShellScript]: """ Normalize inputs to a list of shell scripts """ - return normalize_shell_script_list(value, logger) + return normalize_shell_script_list(key_address, value, logger) @classmethod def _iter_key_annotations(cls) -> Generator[Tuple[str, Any], None, None]: @@ -4610,7 +4646,7 @@ def _load_keys( debug('raw value', str(value)) debug('raw value type', str(type(value))) - value = dataclass_normalize_field(self, keyname, value, logger) + value = dataclass_normalize_field(self, key_address, keyname, value, logger) debug('final value', str(value)) debug('final value type', str(type(value))) From 618dd6b616d9c491a3d80e5a7a076ecf074a2606 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20Prchl=C3=ADk?= Date: Tue, 16 May 2023 23:43:29 +0200 Subject: [PATCH 2/5] squash: fixed typo --- tests/unit/test_base.py | 2 +- tmt/base.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_base.py b/tests/unit/test_base.py index e27f91ccfd..9937c9872a 100644 --- a/tests/unit/test_base.py +++ b/tests/unit/test_base.py @@ -141,7 +141,7 @@ def test_link(): # Invalid links and relations with pytest.raises( SpecificationError, - match="Field 'link' can be string, fmf id or list of their commbinations," + match="Field 'link' can be string, fmf id or list of their combinations," " 'int' found."): Links(data=123) with pytest.raises(SpecificationError, match='Multiple relations'): diff --git a/tmt/base.py b/tmt/base.py index 4552f66bb0..2cbb9b3009 100644 --- a/tmt/base.py +++ b/tmt/base.py @@ -3489,7 +3489,7 @@ def __init__(self, *, data: Optional[_RawLinks] = None): if data is not None and not isinstance(data, (str, dict, list)): # TODO: deliver better key address, needs to know the parent raise tmt.utils.NormalizationError( - 'link', data, 'string, fmf id or list of their commbinations') + 'link', data, 'string, fmf id or list of their combinations') # Nothing to do if no data provided if data is None: From 5aaa916ee5c995c435ccc3bd3ff38f8091fb5519 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20Prchl=C3=ADk?= Date: Mon, 22 May 2023 20:50:07 +0200 Subject: [PATCH 3/5] squash: change wording --- tests/core/fmf-id/test.sh | 4 ++-- tests/unit/test_base.py | 2 +- tests/unit/test_dataclasses.py | 8 ++++---- tmt/base.py | 16 +++++++++++----- tmt/steps/discover/fmf.py | 2 +- tmt/steps/provision/testcloud.py | 2 +- tmt/utils.py | 8 ++++---- 7 files changed, 24 insertions(+), 18 deletions(-) diff --git a/tests/core/fmf-id/test.sh b/tests/core/fmf-id/test.sh index a70d60cdb8..4558f0e0c9 100755 --- a/tests/core/fmf-id/test.sh +++ b/tests/core/fmf-id/test.sh @@ -14,7 +14,7 @@ rlJournalStart rlRun -s "tmt plan -vvvv show /plan-with-invalid-ref" 2 rlAssertGrep "warn: /plan-with-invalid-ref:discover .* is not valid under any of the given schemas" $rlRun_LOG - rlAssertGrep "Failed to load step data for DiscoverFmfStepData: Field 'DiscoverFmfStepData:ref' can be string, 'int' found." $rlRun_LOG + rlAssertGrep "Failed to load step data for DiscoverFmfStepData: Field 'DiscoverFmfStepData:ref' must be string, 'int' found." $rlRun_LOG rlRun -s "tmt plan -vvvv show /remote-plan-with-valid-ref" @@ -29,7 +29,7 @@ rlJournalStart rlRun -s "tmt test -vvvv show /test-with-invalid-ref" 2 rlAssertGrep "warn: /test-with-invalid-ref:require .* is not valid under any of the given schemas" $rlRun_LOG - rlAssertGrep "Field 'ref' can be not set or string, 'int' found." $rlRun_LOG + rlAssertGrep "Field 'ref' must be unset or a string, 'int' found." $rlRun_LOG rlPhaseEnd rlPhaseStartCleanup diff --git a/tests/unit/test_base.py b/tests/unit/test_base.py index 9937c9872a..d3393ff772 100644 --- a/tests/unit/test_base.py +++ b/tests/unit/test_base.py @@ -141,7 +141,7 @@ def test_link(): # Invalid links and relations with pytest.raises( SpecificationError, - match="Field 'link' can be string, fmf id or list of their combinations," + match="Field 'link' must be a string, a fmf id or a list of their combinations," " 'int' found."): Links(data=123) with pytest.raises(SpecificationError, match='Multiple relations'): diff --git a/tests/unit/test_dataclasses.py b/tests/unit/test_dataclasses.py index 4fd33685c9..ee0b6ff792 100644 --- a/tests/unit/test_dataclasses.py +++ b/tests/unit/test_dataclasses.py @@ -24,7 +24,7 @@ def _normalize_foo(key_address: str, raw_value: Any, logger: tmt.log.Logger) -> return int(raw_value) except ValueError as exc: - raise tmt.utils.NormalizationError(key_address, raw_value, 'not set or integer') \ + raise tmt.utils.NormalizationError(key_address, raw_value, 'unset or an integer') \ from exc @dataclasses.dataclass @@ -49,7 +49,7 @@ class DummyContainer(SerializableContainer): with pytest.raises( tmt.utils.SpecificationError, - match=r"Field ':foo' can be not set or integer, 'str' found."): + match=r"Field ':foo' must be unset or an integer, 'str' found."): dataclass_normalize_field(data, ':foo', 'foo', 'will crash', root_logger) assert data.foo == 3 @@ -64,7 +64,7 @@ def normalize_foo(cls, key_address: str, raw_value: Any, logger: tmt.log.Logger) return int(raw_value) except ValueError as exc: - raise tmt.utils.NormalizationError(key_address, raw_value, 'not set or integer') \ + raise tmt.utils.NormalizationError(key_address, raw_value, 'unset or an integer') \ from exc @dataclasses.dataclass @@ -90,7 +90,7 @@ class DummyContainer(SerializableContainer): with pytest.raises( tmt.utils.SpecificationError, - match=r"Field ':foo' can be not set or integer, 'str' found."): + match=r"Field ':foo' must be unset or an integer, 'str' found."): dataclass_normalize_field(data, ':foo', 'foo', 'will crash', root_logger) assert data.foo == 3 diff --git a/tmt/base.py b/tmt/base.py index 2cbb9b3009..c839253e22 100644 --- a/tmt/base.py +++ b/tmt/base.py @@ -162,7 +162,7 @@ def from_spec(cls, raw: _RawFmfId) -> 'FmfId': ref = raw.get('ref', None) if not isinstance(ref, (type(None), str)): # TODO: deliver better key address - raise tmt.utils.NormalizationError('ref', ref, 'not set or string') + raise tmt.utils.NormalizationError('ref', ref, 'unset or a string') fmf_id = FmfId() @@ -375,7 +375,7 @@ def from_spec(cls, raw: _RawDependencyFmfId) -> 'DependencyFmfId': # type: igno ref = raw.get('ref', None) if not isinstance(ref, (type(None), str)): # TODO: deliver better key address - raise tmt.utils.NormalizationError('ref', ref, 'not set or string') + raise tmt.utils.NormalizationError('ref', ref, 'unset or a string') fmf_id = DependencyFmfId() @@ -504,7 +504,13 @@ def normalize_require( if isinstance(raw_require, str) or isinstance(raw_require, dict): return [dependency_factory(raw_require)] - return [dependency_factory(require) for require in raw_require] + if isinstance(raw_require, list): + return [dependency_factory(require) for require in raw_require] + + raise tmt.utils.NormalizationError( + key_address, + raw_require, + 'a string, a library, a file or a list of their combinations') def assert_simple_dependencies( @@ -1502,7 +1508,7 @@ def _get_environment_vars(self, node: fmf.Tree) -> EnvironmentType: environment_files = node.get("environment-file") or [] if not isinstance(environment_files, list): raise tmt.utils.NormalizationError( - f'{self.name}:environment-file', environment_files, 'list of paths') + f'{self.name}:environment-file', environment_files, 'unset or a list of paths') combined = tmt.utils.environment_files_to_dict( filenames=environment_files, root=Path(node.root) if node.root else None, @@ -3489,7 +3495,7 @@ def __init__(self, *, data: Optional[_RawLinks] = None): if data is not None and not isinstance(data, (str, dict, list)): # TODO: deliver better key address, needs to know the parent raise tmt.utils.NormalizationError( - 'link', data, 'string, fmf id or list of their combinations') + 'link', data, 'a string, a fmf id or a list of their combinations') # Nothing to do if no data provided if data is None: diff --git a/tmt/steps/discover/fmf.py b/tmt/steps/discover/fmf.py index 6a8fe3e330..294ecce57b 100644 --- a/tmt/steps/discover/fmf.py +++ b/tmt/steps/discover/fmf.py @@ -57,7 +57,7 @@ def _normalize_ref( return None if not isinstance(value, str): - raise tmt.utils.NormalizationError(key_address, value, 'string') + raise tmt.utils.NormalizationError(key_address, value, 'a string') return value diff --git a/tmt/steps/provision/testcloud.py b/tmt/steps/provision/testcloud.py index 1b9cc50feb..84469d4f14 100644 --- a/tmt/steps/provision/testcloud.py +++ b/tmt/steps/provision/testcloud.py @@ -637,7 +637,7 @@ def go(self) -> None: setattr(data, int_key, int(value)) except ValueError as exc: raise tmt.utils.NormalizationError( - f'{self.name}:{int_key}', value, 'integer') from exc + f'{self.name}:{int_key}', value, 'an integer') from exc for key, value in data.items(): if key == 'memory': diff --git a/tmt/utils.py b/tmt/utils.py index 2ffe5a7c00..0061687b8a 100644 --- a/tmt/utils.py +++ b/tmt/utils.py @@ -1350,7 +1350,7 @@ def __init__( """ super().__init__( - f"Field '{key_address}' can be {expected_type}, '{type(raw_value).__name__}' found.", + f"Field '{key_address}' must be {expected_type}, '{type(raw_value).__name__}' found.", *args, **kwargs) @@ -4382,7 +4382,7 @@ def normalize_path_list( if isinstance(value, (list, tuple)): return [Path(path) for path in value] - raise NormalizationError(key_address, value, 'path or list of paths') + raise NormalizationError(key_address, value, 'a path or a list of paths') def normalize_shell_script_list( @@ -4418,7 +4418,7 @@ def normalize_shell_script_list( if isinstance(value, (list, tuple)): return [ShellScript(str(item)) for item in value] - raise NormalizationError(key_address, value, 'string or list of strings') + raise NormalizationError(key_address, value, 'a string or a list of strings') def normalize_fmf_context( @@ -4479,7 +4479,7 @@ def _normalize_string_list( if isinstance(value, (list, tuple)): return [item for item in value] - raise NormalizationError(key_address, value, 'string or list of strings') + raise NormalizationError(key_address, value, 'a string or a list of strings') def _normalize_environment( self, From 0159cdc374461984a7ca8af23d586588dfab4b4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20Prchl=C3=ADk?= Date: Thu, 1 Jun 2023 11:21:28 +0200 Subject: [PATCH 4/5] squash: fix a typo --- tests/core/fmf-id/test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/core/fmf-id/test.sh b/tests/core/fmf-id/test.sh index 4558f0e0c9..a8592b7ed6 100755 --- a/tests/core/fmf-id/test.sh +++ b/tests/core/fmf-id/test.sh @@ -14,7 +14,7 @@ rlJournalStart rlRun -s "tmt plan -vvvv show /plan-with-invalid-ref" 2 rlAssertGrep "warn: /plan-with-invalid-ref:discover .* is not valid under any of the given schemas" $rlRun_LOG - rlAssertGrep "Failed to load step data for DiscoverFmfStepData: Field 'DiscoverFmfStepData:ref' must be string, 'int' found." $rlRun_LOG + rlAssertGrep "Failed to load step data for DiscoverFmfStepData: Field 'DiscoverFmfStepData:ref' must be a string, 'int' found." $rlRun_LOG rlRun -s "tmt plan -vvvv show /remote-plan-with-valid-ref" From 75bfbbdb1d7866a21be7a5e6d0c36d594470fc1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20Prchl=C3=ADk?= Date: Thu, 1 Jun 2023 13:02:49 +0200 Subject: [PATCH 5/5] squash: fix a typo --- tmt/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tmt/base.py b/tmt/base.py index c839253e22..e8cac5ad1a 100644 --- a/tmt/base.py +++ b/tmt/base.py @@ -567,7 +567,7 @@ class Core( enabled: bool = True order: int = field( default=DEFAULT_ORDER, - normalize=lambda key_address, raw_value, logger: + normalize=lambda key_address, raw_value, logger: DEFAULT_ORDER if raw_value is None else int(raw_value)) link: Optional['Links'] = field( default=None,