Expose "key address" to normalization callbacks #1869
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
happz
force-pushed
the
normalize-include-key-address
branch
from
bec564b to
2912b56
Compare
happz
force-pushed
the
normalize-include-key-address
branch
from
2912b56 to
2b3de71
Compare
|
Hi @happz, your PR looks good to me. And I also tested your patch by updating
(tmt) huanli@kvm-01-guest01:dev$ git branch
* dev.huanli.1881.20230417
main
normalize-include-key-address
(tmt) huanli@kvm-01-guest01:dev$ git diff
diff --git a/tmt/base.py b/tmt/base.py
index 7347dc4..7f316a3 100644
--- a/tmt/base.py
+++ b/tmt/base.py
@@ -766,7 +766,11 @@ class Test(Core, tmt.export.Exportable['Test']):
# TODO: mandatory schema validation would remove the need for Optional...
# `test` is mandatory, must exist, so how to initialize if it's missing :(
if value is None:
- return ShellScript('')
+ raise tmt.utils.NormalizationError(key_address, value, 'string')
+
+ # Raise an error if value is not a string, e.g. False or True
+ if not isinstance(value, str):
+ raise tmt.utils.NormalizationError(key_address, value, 'string')
return ShellScript(value)
(tmt) huanli@kvm-01-guest01:foo$ pwd
/home/huanli/dev/1001869/foo/tests/foo
(tmt) huanli@kvm-01-guest01:foo$ cat main.fmf
summary: Concise summary describing what the test does
test: false
framework: beakerlib
(tmt) huanli@kvm-01-guest01:foo$ tmt run -avvv provision -h local
/var/tmp/tmt/run-001
Found 1 plan.
/plans
summary: Basic smoke test
discover
how: fmf
order: 50
directory: /home/huanli/dev/1001869/foo
warn: /tests/foo:test - False is not of type 'string'
finish
prune: Prune plan workdir '/var/tmp/tmt/run-001/plans'.
summary: 0 tasks completed
Field "/tests/foo:test" can be string, "bool" found.
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Looks good !!! |
happz
force-pushed
the
normalize-include-key-address
branch
from
2b3de71 to
46e7b0c
Compare
happz
force-pushed
the
normalize-include-key-address
branch
3 times, most recently
from
eecdddc to
85b182a
Compare
happz
force-pushed
the
normalize-include-key-address
branch
from
e9bfa79 to
9529a77
Compare
|
Fix for testing-farm:fedora-38-x86_64:internal-plugins in progress |
lukaszachy
approved these changes
May 17, 2023
happz
force-pushed
the
normalize-include-key-address
branch
from
9529a77 to
55c5982
Compare
thrix
requested changes
May 22, 2023
janhavlin
reviewed
May 22, 2023
happz
force-pushed
the
normalize-include-key-address
branch
2 times, most recently
from
ede7100 to
8b0c81c
Compare
happz
force-pushed
the
normalize-include-key-address
branch
from
8b0c81c to
9295641
Compare
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.
happz
force-pushed
the
normalize-include-key-address
branch
from
2b951f1 to
75bfbbd
Compare
psss
reviewed
Jun 6, 2023
| @@ -522,7 +521,7 @@ class PrepareInstallData(tmt.steps.prepare.PrepareStepData): | |||
| ) | |||
|
|
|||
|
|
|||
| @tmt.steps.provides_method('install') | |||
| @ tmt.steps.provides_method('install') | |||
There was a problem hiding this comment.
Oops, nope, that does not look like an intention. It shouldn't have any effect, but it's not supposed to happen.
There was a problem hiding this comment.
Ok, we can fix with some other patch.
There was a problem hiding this comment.
Here's the fix: #2128
4N0body5
pushed a commit
that referenced
this pull request
Jun 7, 2023
Keys belong to various objects like step phases or fmf ids, and tmt can do a 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 an fmf node name, usually, because the node name is not available.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.