-
Notifications
You must be signed in to change notification settings - Fork 151
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
Second round of output cleaning #440
Conversation
373b6a1
to
d103953
Compare
8dbaa75
to
4c0919d
Compare
b225503
to
f1b9d01
Compare
Thanks for the review! |
e326f92
to
15e598d
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.
Greetings, I have emerged from the woodwork - sorry for dropping off the radar.
Can we split off /etc/sysconfig
in separate commit please - I could swear I've saw some distro use it.
Sure, ok. |
|
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.
Note: removing sysconfig thing is a good idea IMHO. We just need to a) ensure we have dkms.conf equivalent + is parsed and b) add a bit scary note in the release notes.
But that's for another day. Thanks, updated PR good.
Second out of output refining. Example with autoinstall and multiple modules, before:
Most of the
autoinstall
output is useless and not well placed. Also the depmod line should be attached to the "Installng [..]" output and separated from the "Sign [..]" output.After changing the output, it's structured like this, with each block separated by context:
After spending lot of time trying to figure out why the output trapped by the tests is slightly different on old Alpine containers and the old Ubuntu VM that was used for the test, I narrowed it down to the
invoke_command
combined withstdbuf
. Funny thing is that if you run the commands directly on the containers the output is consistent, and does not exhibit the extra blank line before "Cleaning build area".To avoid this issue entirely, in
generalize_expected_output
remove all blank lines, so the output is trapped 100% the same on all containers.