Skip to content

intg: remove test_session_recording.py#8243

Merged
spoore1 merged 1 commit into
SSSD:masterfrom
spoore1:rm_intg_session_recording
Dec 4, 2025
Merged

intg: remove test_session_recording.py#8243
spoore1 merged 1 commit into
SSSD:masterfrom
spoore1:rm_intg_session_recording

Conversation

@spoore1

@spoore1 spoore1 commented Nov 26, 2025

Copy link
Copy Markdown
Contributor

No description provided.

gemini-code-assist[bot]
gemini-code-assist Bot reviewed Nov 26, 2025
View reviewed changes

@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 removes test_session_recording.py, which appears to be the complete integration test suite for the SSSD session recording feature. Based on other files in the repository such as src/man/sssd-session-recording.5.xml and src/util/session_recording.c, this feature is still active. Deleting its tests introduces a critical risk of regressions going undetected. It is essential to maintain test coverage for existing features. If these tests are being replaced, the new tests should ideally be part of the same pull request.

@spoore1
spoore1 force-pushed the rm_intg_session_recording branch from f3bbebb to b55b9d4 Compare November 26, 2025 14:19
@danlavu

danlavu commented Dec 3, 2025

Copy link
Copy Markdown
Contributor 
(.venv)  dlavu@rocket  ~/git/sssd   master  git_exists.sh src/tests/intg/test_session_recording.py
FILE FOUND →  master
FILE FOUND →  sssd-2-10
FILE FOUND →  sssd-2-11
FILE FOUND →  sssd-2-7
FILE FOUND →  sssd-2-8
FILE FOUND →  sssd-2-9
FILE FOUND →  sssd-2-9-4

Adding the necessary tag, but AFAIK, we are leaving 2-7 and 2-8 alone.

@danlavu danlavu self-assigned this Dec 3, 2025
@danlavu
danlavu self-requested a review December 3, 2025 17:08
@danlavu

danlavu commented Dec 3, 2025

Copy link
Copy Markdown
Contributor

This has been discussed, the priority to remove these tests are greater than having these intg tests. These tests will be written for the new test framework at a later time.

@spoore1 @sssd-bot
intg: remove test_session_recording.py
fbdd22a 
Removing intg session recording test file and removing from Makefile.

Reviewed-by: Dan Lavu <[email protected]>
@sssd-bot

sssd-bot commented Dec 3, 2025

Copy link
Copy Markdown
Contributor

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

🟢 CodeFactor (success)
🟢 CodeQL (success)
🟢 osh-diff-scan:fedora-rawhide-x86_64:upstream (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)
🟢 Analyze (target) / cppcheck (success)
🟢 Build / freebsd (success)
🟢 Build / make-distcheck (success)
🔴 ci / intgcheck (centos-10) (failure)
🟢 ci / intgcheck (fedora-41) (success)
🔴 ci / intgcheck (fedora-42) (failure)
🔴 ci / intgcheck (fedora-43) (failure)
🔴 ci / intgcheck (fedora-44) (failure)
🟢 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)
➖ Coverity scan / coverity (skipped)
🟢 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 rm_intg_session_recording branch from b55b9d4 to fbdd22a Compare December 3, 2025 22:40
@spoore1
spoore1 merged commit 5023916 into SSSD:master Dec 4, 2025
18 checks passed
This was referenced Dec 4, 2025
Merged
Merged
Merged
Merged
@spoore1
spoore1 deleted the rm_intg_session_recording branch January 27, 2026 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@danlavu danlavu danlavu approved these changes
@justin-stephenson justin-stephenson justin-stephenson approved these changes
+1 more reviewer
@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@danlavu danlavu

Labels

Accepted backport-to-sssd-2-9-4 Corresponds to C8S backport-to-sssd-2-9 backport-to-sssd-2-10 Tests Trivial A single reviewer is sufficient to review the Pull Request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@spoore1 @danlavu @sssd-bot @justin-stephenson @alexey-tikhonov