-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
make: add capability to check config for test-with-config
#16795
make: add capability to check config for test-with-config
#16795
Conversation
Took your suggestions and added my own spin to it (and also fixed some of the wording ;-)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is also long and difficult to read -> comment to explain
I am ok with your spin on the comments mine where mostly for inspiration. I tested this with the check-config provided in PR #16797 failing and succeeding as expected ✔️ these PRs have a small problem of hen and egg this should be squashed and |
|
8e91cb5
to
0b23060
Compare
Not really: One introduces the feature and one uses it for one specific test. There is no cyclic dependency between them, so no hen-egg-problem :-). |
i got into the hen and egg while testing since i did not want to use another pr to test this of cause there is no circular dependancy graph |
test $$t = "empty" && break; \ | ||
$$t || \ | ||
(echo "System configuration for" \ | ||
"tests is not available, check $${t#$${RIOTBASE}/} failed." \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"tests is not available, check $${t#$${RIOTBASE}/} failed." \ | |
"tests is not available, check $${t#$${RIOTBASE}/}." \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
" check tests/gnrc_dhcpv6_client/tests-with-config/check-config.sh failed. "
i read the "check" a a verb and it sounded wrong to me, but if check and the filename are one noun it sounds correct
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That looks to me like a part of the sentence is missing...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i was just citing the part after the ,
but as said i was reading it kinda wrong
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is good to merge.
Contribution description
This adds another dependency for
test-with-config
that uses (optional)check-config
scripts within thetests-with-config
directory to check if the config required for the test is available. If the config is not available, themake test-with-config
target will fail with an appropriate error message.Testing procedure
I will provide a config checker for
tests/gnrc_dhcpv6_client
as a follow-up (#16797). With that, this PR will be testable.Issues/PRs references
None