-
Notifications
You must be signed in to change notification settings - Fork 36
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
Add test for EFI log processing #1324
base: master
Are you sure you want to change the base?
Conversation
f20e1c3
to
864c08c
Compare
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.
Looks good to me - thanks! :)
Just added some suggestions for using fragments to provide generic/boilerplate bits in the kickstart via kickstart fragments. :)
efi-log.ks.in
Outdated
%ksappend repos/default.ks | ||
network --bootproto=dhcp | ||
|
||
bootloader --timeout=1 |
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 other than the %post script this kickstart tests does not really need anything special, right ? In that case I would suggest using of the "common boilerplate" kickstart fragments instead to inject the common things. For example this one:
A similar (mainly needs to do stuff in %post) kickstart tests that uses this fragment:
kickstart-tests/ui_cmdline.ks.in
Line 6 in 2ca0796
%ksappend common/common_no_payload.ks |
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.
Updated according to suggestion.
|
||
%end | ||
|
||
%post |
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.
The validation boilerplate could be replaced by this fragment: https://github.com/rhinstaller/kickstart-tests/blob/master/fragments/shared/validation/success_if_result_empty_standalone.ks
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.
Updated according to suggestion
'/usr/libexec/anaconda/log-capture' is an util to generate runtime logs. This commit adds test validating output of this util.
864c08c
to
66e8289
Compare
@M4rtinK thanks for the review. I have updated the code following your guidelines. Also I have split test cases in half: check if the util exists, call the util and expect |
/test-tmt |
/test-os-variants |
Agreed with the split - like this we can easily see from the logs what exactly went wrong, nice! :) As for validation - it creates a tarball, right ? I guess we could check the file exists and is of non-zero size ? But I leave it up to you, given we already check the return code. :) |
'/usr/libexec/anaconda/log-capture' is an util to generate runtime logs. This commit adds test validating output of this util.