Skip to content

Implement fundamental unit test coverage of the accountstate module.#580

Merged
jonasbardino merged 9 commits into
nextfrom
add/fundamental-unit-tests-for-accountstate
Jun 24, 2026
Merged

Implement fundamental unit test coverage of the accountstate module.#580
jonasbardino merged 9 commits into
nextfrom
add/fundamental-unit-tests-for-accountstate

Conversation

@jonasbardino

@jonasbardino jonasbardino commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Implement fundamental unit test coverage of the accountstate module.
Fixes a corner-case bug in reset_account_expire_cache when called with a specific client_id rather than the default (None) to reset all expire marks. The specific client_id version is not yet used in any actual code, so it should be safe.

Originally developed for the UUID user format support work in #561 but this part makes for a stand-alone PR to first increase overall unit test coverage.

Fixes a corner-case bug in `reset_account_expire_cache` when called with a
specific `client_id` rather than the default (None) to reset all expire marks.
@jonasbardino jonasbardino self-assigned this Jun 18, 2026
@jonasbardino jonasbardino added bug Something isn't working test-only Improvements or additions solely for better test coverage - without functionality changes labels Jun 18, 2026
@jonasbardino jonasbardino marked this pull request as ready for review June 19, 2026 08:34
@jonasbardino jonasbardino requested a review from a team June 19, 2026 08:34
Comment thread tests/test_mig_shared_accountstate.py Outdated

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

Approved when comment addressed

independently even if the assertRaises does not kick in. The tested code is a
bit clunky to quietly patch incorrect use but this version should make the test
more consistent. Fixed a couple of similar issues with othe tests using a
`with X` construct and not keeping variables aligned.
Simplified a few `self.configuration` uses where we already have aliased it as
`configuration`.
@ucphhpc ucphhpc deleted a comment from Martin-Rehr Jun 19, 2026
@jonasbardino

Copy link
Copy Markdown
Contributor Author

Thanks for the review and feedback. I've tried to address it, and deleted the extra comment you had marked as obsolete but noted you couldn't delete.

…alid

values to avoid inconsistent handling.
The underlying function could really use more robust handling of such invalid
values but for now we just want to make sure that the tests cover the current
behaviour in order to let the UUID version aim for bug-for-bug compatibility
as a start.
@Martin-Rehr

Copy link
Copy Markdown
Contributor

Thanks for the review and feedback. I've tried to address it, and deleted the extra comment you had marked as obsolete but noted you couldn't delete.

Thanks, looks good

jonasbardino added a commit that referenced this pull request Jun 23, 2026
…D user ID

format cases. Also tests the recently adjusted check_account_accessible helper
thoroughly.
jonasbardino added a commit that referenced this pull request Jun 23, 2026
…D user ID

format cases. Also tests the recently adjusted check_account_accessible helper
thoroughly.
@jonasbardino jonasbardino merged commit 19670da into next Jun 24, 2026
11 checks passed
@jonasbardino jonasbardino deleted the add/fundamental-unit-tests-for-accountstate branch June 24, 2026 06:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working test-only Improvements or additions solely for better test coverage - without functionality changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants