[e2e] Improve su related tests#1506
Conversation
Since those tests are quite simple, we can add them to another test and avoid having to go through the demanding device authentication process.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1506 +/- ##
==========================================
+ Coverage 81.54% 86.97% +5.42%
==========================================
Files 20 93 +73
Lines 1111 6443 +5332
Branches 0 111 +111
==========================================
+ Hits 906 5604 +4698
- Misses 205 783 +578
- Partials 0 56 +56 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
the test is failing in this job: https://authd-e2e-test-logs.adrian-dombeck.workers.dev/pr-1506/run-25435165169-1/resolute-authd-msentraid/log.html#s1-s10-t1-k12 |
denisonbarbosa
force-pushed
the
fix-e2e-su-tests
branch
from
2b9b3ac to
65b4d75
Compare
The previous "sudo su" was covering a different flow. The changes in this commit make sure we cover the cases where an empty "su" actually goes through the PAM stack, since it tries to authenticate the root user.
denisonbarbosa
force-pushed
the
fix-e2e-su-tests
branch
from
65b4d75 to
b292592
Compare
Yeah, I forgot we disabled the root password on the test VMs. It should be fixed with the recent push. |
adombeck
left a comment
adombeck
left a comment
There was a problem hiding this comment.
Ok, please re-request a review once you checked that the relevant tests pass. Thanks!
| Log In With Remote User Through CLI: Local Password ${username} ${local_password} | ||
|
|
||
| # Try to change username during su login, it should not be possible | ||
| Try Changing Username In su Log In ${username} |
There was a problem hiding this comment.
The keyword name Try xyz suggests to me that an expected outcome is that xyz works. I suggest changing it to:
| Try Changing Username In su Log In ${username} | |
| Check Username Cannot Be Changed When Using su ${username} |
There was a problem hiding this comment.
I don't necessarily agree with it, but I don't have a strong opinion on the name also. I'll update the keywords.
| Log In With Remote User Through CLI: Local Password ${username} ${local_password} | ||
|
|
||
| # Try to change username during su login, it should not be possible | ||
| Check That Username Cannot Be Changed When Using su ${username} |
There was a problem hiding this comment.
There was a problem hiding this comment.
Hm, this is weird. It's like the CTRL_L C from "Cancel Operation" never happened
By looking at the test logs, it seems like machinectl on Noble doesn't capture keyboard signals properly, so let's use a different approach instead.
denisonbarbosa
force-pushed
the
fix-e2e-su-tests
branch
from
9ec90fd to
cdc472e
Compare
2 participants
We used to have a separate .robot file for these tests, but going through the device authentication flow is too demanding for simple tests like these. This moves them to
login.robotand updates the "su without argument" check to ensure that authenticating therootuser is not an authd flow.