Test: housekeeping: test_sss_ssh_knownhosts.py => test_ipa.py #7478
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@danlavu what could be the |
madhuriupadhye
force-pushed
the
ssh_knowhost_hosekeeping
branch
from
03a1871 to
7aa9b3d
Compare
danlavu
requested changes
Jul 8, 2024
There was a problem hiding this comment.
As a part of housekeeping, we're also clarifying the docs, so it's clear what the test is doing and trimming unnecessary verbiage.
:title: sss_ssh_knownhosts returns public keys by name
:setup:
1. Create IPA host "ssh.ipa.test", public keys and IP resolvable via DNS
2. Enable ssh responder
3. Start SSSD
:steps:
1. Run "sss_ssh_knownhosts ssh.ipa.test"
:expectedresults:
1. All public keys were printed
:customerscenario: False
"""
can be revised to
:title: sss_ssh_knownhosts returns public keys by name
:setup:
1. Create host with SSH key
2. Configure SSSD with SSH responder
3. Start SSSD
:steps:
1. Lookup SSH key
:expectedresults:
1. All public keys were printed
:customerscenario: False
"""
- The topology is IPA, so saying it's IPA is redundant.
- Resolvable by DNS, that can be assumed.
- Actual hostname, it doesn't really add anything to describing what the test does, in actually I think it overly complicates it.
I think one or two sentences at the top describing what the feature does. Question, should this be an IPA test or is it testing the command sss_ssh_knownhostsproxy? From the name I think tools, so that should be the requirement, if you think this more about testing the retrieval of the public host key from IPA, we should change this to test_ipa__hostkeys or something.
madhuriupadhye
force-pushed
the
ssh_knowhost_hosekeeping
branch
3 times, most recently
from
e464771 to
89dad7e
Compare
madhuriupadhye
changed the title
danlavu
requested changes
Jul 10, 2024
madhuriupadhye
force-pushed
the
ssh_knowhost_hosekeeping
branch
from
89dad7e to
6d2f112
Compare
danlavu
approved these changes
Jul 11, 2024
|
All 'system' tests were failing. |
alexey-tikhonov
added
Changes requested
and removed
Accepted
Ready to push
Update the docstring of each test. Update the test case method name. Added assertion error messages on failures. Signed-off-by: Madhuri Upadhye <[email protected]>
madhuriupadhye
force-pushed
the
ssh_knowhost_hosekeeping
branch
from
6d2f112 to
a6b7a92
Compare
|
@alexey-tikhonov these have been fixed |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added assertion error messages on failures.