Skip to content

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

Merged
thalman merged 1 commit intoSSSD: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 intoSSSD:sssd-2-11from
danlavu:cherrypick-8120-to-2-11

Conversation

@danlavu
Copy link

@danlavu danlavu commented Dec 9, 2025

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

Reviewed-by: Dan Lavu dlavu@redhat.com
Reviewed-by: Jakub Vávra jvavra@redhat.com

@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
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 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.

Copy link
Contributor

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

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 <dlavu@redhat.com>
Reviewed-by: Jakub Vávra <jvavra@redhat.com>
Reviewed-by: Tomáš Halman <thalman@redhat.com>
@sssd-bot
Copy link
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