-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
systemd-cryptenroll: adding integration test cases #27020
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
Conversation
|
Most of the part missing coverage on the cryptenroll.c module are in the parse_argv() function. There are about 300 lines currently not covered. Thank you sirs. |
|
For a test like "systemd-cryptenroll --password", As it is the script probably doesn't move beyond that test. I'm currently working on how I could pass in those 3 required inputs in the test script, so that the interpreter takes them automatically, executes the test and moves to the next test. Any insight will be greatly appreciated. Thank you. |
|
please rebase on latest main, the test should pass then |
Exactly sir. I'll make a push now. I'll make more push today. I hope to cover all by tomorrow. For test cases where a user is supposed to input their credentials sir. What would you recommend I do. Like "systemd-cryptenroll --password" All I saw while searching was to use expect scripts, which isn't at all used in the codebase. Thank you sir. |
|
Set the |
Thank you so much sir. |
86d438e to
d3ac85e
Compare
|
I did 22 tests total. For my long task "fix an issue with the label RFE", I will try to work on the cryptenroll tool still. |
|
@bluca, @keszybz @mrc0mmand @DaanDeMeyer @yuwata could you please help confirm if the failure on "mkosi / ci (arch, rolling) (pull request)" is due to my code? I had "mkos/ ci (centos, 9) (pull request)" as the one failing initially, I wanted to confirm it was due to any issues with my tests, Thank you. |
|
All the usual checks now pass. Thank you sirs. |
|
Awesome. tpm2 pcr RFE next. Thank you. |
| ret="$(! systemd-cryptenroll --fido2-with-client-pin=false 2> >(grep "No block device node specified"))" | ||
| test -n "${ret}" |
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.
Hmm, what exactly is this test testing? Why not just use a pipe?
(IIUC, the goal is to assert something about stderr, but since stdout and stderr are mixed, it's not really testing much.)
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 saw the line for that isn't covered on Coveralls..
So I decided to test such error situation.
Could you clarify further about the pipe sir?
Thank you
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.
What he means is that checking the output like that is not very useful, as we change debug log quite often. Instead, simply check that i's failing as expected, eg:
systemd-cryptenroll --fido2-with-client-pin=false && { echo 'unexpected success'; exit 1; }
please send a follow-up to apply thorough
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.
Ohh wow. Thank you so much sir.
I'm really grateful.
I had this concern of the set -ex flag.
I didn't want the test to exit after just one failed test, as that would make the negative test cases sort of hard to do.
Can I open a new PR on this right now?
Or is there a way to rework this very one?
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.
open a new pr
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.
Awesome.
Thank you sir
|
new PR opened sir: |
…ng rather than redirections. Follow up on systemd#27020
Use in-line error handling rather than redirections. Follow up on systemd#27020
Use in-line error handling rather than redirections. Follow up on #27020
Use in-line error handling rather than redirections. Follow up on systemd#27020
Use in-line error handling rather than redirections. Follow up on systemd#27020 (cherry picked from commit 27d45db)
Use in-line error handling rather than redirections. Follow up on systemd#27020 (cherry picked from commit 27d45db) (cherry picked from commit 2950b4e)
Use in-line error handling rather than redirections. Follow up on systemd#27020 (cherry picked from commit 27d45db)
Use in-line error handling rather than redirections. Follow up on systemd#27020 (cherry picked from commit 27d45db) Related: RHEL-16182
No description provided.