Skip to content

cherry-pick: 8120 to 2.11 - Migrate missing tests to new framework#8290

Merged
thalman merged 1 commit into
SSSD:sssd-2-11from
danlavu:cherrypick-8120-to-2-11
Dec 10, 2025
Merged

cherry-pick: 8120 to 2.11 - Migrate missing tests to new framework#8290
thalman merged 1 commit into
SSSD:sssd-2-11from
danlavu:cherrypick-8120-to-2-11

Conversation

@danlavu

@danlavu danlavu commented Dec 9, 2025

Copy link
Copy Markdown
Contributor

This PR implements tests from integ/test_ldap.py to new system tests

Reviewed-by: Dan Lavu [email protected]
Reviewed-by: Jakub Vávra [email protected]

@danlavu danlavu added Waiting for review Tests no-backport This should go to target branch only. labels Dec 9, 2025
@danlavu danlavu changed the title tests: Migrate missing tests to new framework cherry-pick: 8120 to 2.11 - Migrate missing tests to new framework Dec 9, 2025

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

Copy link
Copy Markdown

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 migrates several tests from the old integ/test_ldap.py framework to the new system test framework, creating a new test_nss.py file. The migration is a good step forward. I've found a couple of issues in the newly added tests: one is an incorrect type hint that restricts a test's applicability, and the other is an incomplete test that doesn't fully verify the caching behavior it's supposed to. I've provided specific suggestions to address these points. Additionally, a related test was refactored from test_authentication.py to test_nss.py, which is a logical improvement.

Comment thread src/tests/system/tests/test_nss.py
Comment thread src/tests/system/tests/test_nss.py

@jakub-vavra-cz jakub-vavra-cz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm

This PR implements tests from integ/test_ldap.py to new system tests

Reviewed-by: Dan Lavu <[email protected]>
Reviewed-by: Jakub Vávra <[email protected]>
Reviewed-by: Tomáš Halman <[email protected]>
@sssd-bot

Copy link
Copy Markdown
Contributor

The pull request was accepted by @thalman with the following PR CI status:


🟢 CodeQL (success)
🟢 rpm-build:centos-stream-10-x86_64:upstream (success)
🟢 rpm-build:fedora-41-x86_64:upstream (success)
🟢 rpm-build:fedora-42-x86_64:upstream (success)
🟢 rpm-build:fedora-43-x86_64:upstream (success)
🟢 rpm-build:fedora-rawhide-x86_64:upstream (success)
🔴 Build / make-distcheck (failure)
🔴 ci / intgcheck (centos-10) (failure)
🟢 ci / intgcheck (fedora-41) (success)
🟢 ci / intgcheck (fedora-42) (success)
🟢 ci / intgcheck (fedora-43) (success)
🟢 ci / intgcheck (fedora-44) (success)
🟢 ci / prepare (success)
🟢 ci / system (centos-10) (success)
🟢 ci / system (fedora-41) (success)
🟢 ci / system (fedora-42) (success)
🟢 ci / system (fedora-43) (success)
🟢 ci / system (fedora-44) (success)
🟢 Static code analysis / codeql (success)
🟢 Static code analysis / pre-commit (success)
🟢 Static code analysis / python-system-tests (success)


There are unsuccessful or unfinished checks. Make sure that the failures are not related to this pull request before merging.

@sssd-bot
sssd-bot force-pushed the cherrypick-8120-to-2-11 branch from d268969 to b9a248e Compare December 10, 2025 08:36
@thalman
thalman merged commit f4079f2 into SSSD:sssd-2-11 Dec 10, 2025
14 checks passed
@danlavu
danlavu deleted the cherrypick-8120-to-2-11 branch January 23, 2026 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Accepted no-backport This should go to target branch only. Tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants