Skip to content

adding subid test#8225

Merged
danlavu merged 1 commit into
SSSD:masterfrom
danlavu:tests-subid
Dec 10, 2025
Merged

adding subid test#8225
danlavu merged 1 commit into
SSSD:masterfrom
danlavu:tests-subid

Conversation

@danlavu

@danlavu danlavu commented Nov 22, 2025

Copy link
Copy Markdown
Contributor

No description provided.

gemini-code-assist[bot]
gemini-code-assist Bot reviewed Nov 22, 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 adds a new system test for subid configuration in IPA. My review focuses on the new test case, where I've found several copy-paste errors in the assertions that would cause the test to fail or report incorrect information. I've provided suggestions to fix the incorrect function calls and assertion messages.

Comment thread src/tests/system/tests/test_ipa.py Outdated
Comment thread src/tests/system/tests/test_ipa.py Outdated
Comment thread src/tests/system/tests/test_ipa.py Outdated
@danlavu
danlavu force-pushed the tests-subid branch 2 times, most recently from d731c99 to 64194c5 Compare November 22, 2025 06:54
@danlavu

danlavu commented Nov 22, 2025

Copy link
Copy Markdown
Contributor Author

This requires two other PRs.

jakub-vavra-cz
jakub-vavra-cz approved these changes Nov 24, 2025
View reviewed changes

@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

alexey-tikhonov
alexey-tikhonov reviewed Nov 24, 2025
View reviewed changes
Comment thread src/tests/system/tests/test_ipa.py Outdated
alexey-tikhonov
alexey-tikhonov reviewed Nov 24, 2025
View reviewed changes
Comment thread src/tests/system/tests/test_ipa.py Outdated
alexey-tikhonov
alexey-tikhonov reviewed Nov 24, 2025
View reviewed changes
Comment thread src/tests/system/tests/test_ipa.py Outdated
alexey-tikhonov
alexey-tikhonov reviewed Nov 24, 2025
View reviewed changes
Comment thread src/tests/system/tests/test_ipa.py Outdated
), f"Subordinate UID start range {uid.range_start} does not match: {subid.uid_start}!"
assert (
subid.uid_size == uid.range_size
), f"Subordinate UID start range {uid.range_size} does not match: {subid.uid_size}!"

@alexey-tikhonov alexey-tikhonov Nov 24, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy-paste error.

@danlavu danlavu Nov 24, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's correct, but I think it's from the confusing variable names and attributes.

subid from ipa has four attributes

  • subid.uid_size
  • subid.uid_range
  • subid.gid_size
  • subid.gid_range

tools.getsubids, returns two

  • range_size
  • range_start

subid.uid_size == object.range_size
subid.uid_start == object.range_start

subid.gid_size == group_object.range_size
subid.gid_start == group_object.range_start

The variables have been changed, let me know if it's still confusing.

@alexey-tikhonov alexey-tikhonov Nov 24, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The text itself - "Subordinate UID start range" - was a copy&paste error.

@alexey-tikhonov alexey-tikhonov Nov 24, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Text says "UID start range" but prints "{uid.range_size}".

@danlavu danlavu Dec 1, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

My mistake, fixed.

alexey-tikhonov
alexey-tikhonov reviewed Nov 24, 2025
View reviewed changes
Comment thread src/tests/system/tests/test_ipa.py Outdated
), f"Subordinate GID start range {gid.range_start} does not match: {subid.gid_start}!"
assert (
subid.gid_size == gid.range_size
), f"Subordinate GID start range {gid.range_size} does not match: {subid.gid_size}!"

@alexey-tikhonov alexey-tikhonov Nov 24, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy-paste error.

@danlavu danlavu Dec 1, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

alexey-tikhonov
alexey-tikhonov reviewed Nov 24, 2025
View reviewed changes
Comment thread src/tests/system/tests/test_ipa.py Outdated
@danlavu
danlavu force-pushed the tests-subid branch 2 times, most recently from d09a147 to 2724d2c Compare December 1, 2025 19:07
@alexey-tikhonov

alexey-tikhonov commented Dec 2, 2025

Copy link
Copy Markdown
Member

@danlavu, mypy fails:

tests/test_ipa.py:671: error: Item "None" of "SubIDEntry | None" has no attribute "range_start"  [union-attr]
tests/test_ipa.py:673: error: Item "None" of "SubIDEntry | None" has no attribute "range_size"  [union-attr]
tests/test_ipa.py:674: error: Item "None" of "SubIDEntry | None" has no attribute "range_size"  [union-attr]
tests/test_ipa.py:679: error: Item "None" of "SubIDEntry | None" has no attribute "range_start"  [union-attr]
tests/test_ipa.py:680: error: Item "None" of "SubIDEntry | None" has no attribute "range_start"  [union-attr]
tests/test_ipa.py:682: error: Item "None" of "SubIDEntry | None" has no attribute "range_size"  [union-attr]
tests/test_ipa.py:683: error: Item "None" of "SubIDEntry | None" has no attribute "range_size"  [union-attr]
Found 8 errors in 1 file (checked 33 source files)

@danlavu
danlavu force-pushed the tests-subid branch 2 times, most recently from 8b19216 to ce3167e Compare December 2, 2025 21:29
@alexey-tikhonov

alexey-tikhonov commented Dec 3, 2025

Copy link
Copy Markdown
Member

Note:

tests/test_ipa.py::test_ipa__subids_configured (ipa) PASSED              [ 22%]

@alexey-tikhonov

alexey-tikhonov commented Dec 3, 2025

Copy link
Copy Markdown
Member

@jakub-vavra-cz, are you fine with the latest version?

@alexey-tikhonov

alexey-tikhonov commented Dec 3, 2025

Copy link
Copy Markdown
Member

I set 'Blocked' until patch "do not add this commit" removed.

@alexey-tikhonov

alexey-tikhonov commented Dec 3, 2025

Copy link
Copy Markdown
Member

SSSD/sssd-test-framework#221 was merged.

@danlavu
danlavu force-pushed the tests-subid branch 3 times, most recently from 2fe4b25 to 42995ba Compare December 9, 2025 19:04
jakub-vavra-cz
jakub-vavra-cz approved these changes Dec 10, 2025
View reviewed changes

@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

@danlavu
danlavu merged commit 449913a into SSSD:master Dec 10, 2025
23 of 24 checks passed
This was referenced Dec 10, 2025
Merged
Closed
@danlavu
danlavu deleted the tests-subid branch January 23, 2026 14:43
@danlavu
danlavu restored the tests-subid branch January 27, 2026 03:24
@danlavu
danlavu deleted the tests-subid branch July 1, 2026 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alexey-tikhonov alexey-tikhonov alexey-tikhonov approved these changes
@jakub-vavra-cz jakub-vavra-cz jakub-vavra-cz 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

@alexey-tikhonov alexey-tikhonov

@jakub-vavra-cz jakub-vavra-cz

Labels

backport-to-sssd-2-9-4 Corresponds to C8S backport-to-sssd-2-9 Tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@danlavu @alexey-tikhonov @jakub-vavra-cz