Skip to content

[autobackport: sssd-2-12] adding sss_ssh_knownhosts test case#8493

Open
sssd-bot wants to merge 1 commit intoSSSD:sssd-2-12from
sssd-bot:SSSD-sssd-backport-pr8448-to-sssd-2-12
Open

[autobackport: sssd-2-12] adding sss_ssh_knownhosts test case#8493
sssd-bot wants to merge 1 commit intoSSSD:sssd-2-12from
sssd-bot:SSSD-sssd-backport-pr8448-to-sssd-2-12

Conversation

@sssd-bot
Copy link
Contributor

@sssd-bot sssd-bot commented Mar 4, 2026

This is an automatic backport of PR#8448 adding sss_ssh_knownhosts test case to branch sssd-2-12, created by @danlavu.

Please make sure this backport is correct.

Note

The commits were cherry-picked without conflicts.

You can push changes to this pull request

git remote add sssd-bot git@github.com:sssd-bot/sssd.git
git fetch sssd-bot refs/heads/SSSD-sssd-backport-pr8448-to-sssd-2-12
git checkout SSSD-sssd-backport-pr8448-to-sssd-2-12
git push sssd-bot SSSD-sssd-backport-pr8448-to-sssd-2-12 --force

Original commits
b4e88e8 - adding sss_ssh_knownhosts test case

Backported commits

  • 618ff6a - adding sss_ssh_knownhosts test case

Original Pull Request Body

Reviewed-by: Anuj Borah <aborah@redhat.com>
Reviewed-by: Alejandro López <allopez@redhat.com>
(cherry picked from commit b4e88e8)
Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds a system test for sss_ssh_knownhosts. The new test contains a syntax error that will prevent it from running. Additionally, it introduces dependencies on external network services, which can cause test flakiness. I've provided a suggestion to fix the syntax and remove the external dependencies to improve test reliability.

Comment on lines +69 to +70
[("sssd.io", True), ("1.1.1.1", True), ("client.test", True), ("asdf.test", False), ("super.bad.hostname", False)],
ids=["sssd.io", "1.1.1.1", "client.test", "asdf.test", "super.bad.hostname"],

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

There are two issues with the parametrization of this test:

  1. Syntax Error: There is a missing comma at the end of line 69, between the list of argvalues and the ids keyword argument. This will cause a SyntaxError.

  2. External Test Dependencies: The test cases for sssd.io and 1.1.1.1 introduce a dependency on external network services. This can lead to flaky tests if the test environment lacks internet access or if external DNS resolution fails. System tests should be self-contained to ensure they are reliable and reproducible.

I've provided a suggestion that fixes the syntax and removes the external dependencies. The client.test case is sufficient for testing a resolvable hostname within the test environment.

Suggested change
[("sssd.io", True), ("1.1.1.1", True), ("client.test", True), ("asdf.test", False), ("super.bad.hostname", False)],
ids=["sssd.io", "1.1.1.1", "client.test", "asdf.test", "super.bad.hostname"],
[("client.test", True), ("asdf.test", False), ("super.bad.hostname", False)],
ids=["client.test", "asdf.test", "super.bad.hostname"],

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants