Skip to content

Add a few more user state dirs made by actual user creation in provision user helper#563

Merged
jonasbardino merged 2 commits into
nextfrom
adjust/unit-test-usersupp-to-include-more-actual-user-dirs-and-polish
Jun 9, 2026
Merged

Add a few more user state dirs made by actual user creation in provision user helper#563
jonasbardino merged 2 commits into
nextfrom
adjust/unit-test-usersupp-to-include-more-actual-user-dirs-and-polish

Conversation

@jonasbardino

Copy link
Copy Markdown
Contributor

Add a few more user state dirs made by actual user creation and importantly expected e.g. during useradm operations like edit_user, delete_user, etc. and therefore difficult to do without when adding test coverage a.o. in #561. Introduce two more common DNs used across tests: Other User and No Such User.
Polish a long comment with some typos and incomplete ending.

…tantly

expected e.g. during useradm operations like edit_user, delete_user, etc. and
therefore difficult to do without when adding test coverage a.o. in #561.
Introduce two more common DNs used across tests: Other User and No Such User.
Polish a long comment with some typos and incomplete ending.
@jonasbardino jonasbardino added the test-only Improvements or additions solely for better test coverage - without functionality changes label May 29, 2026
@jonasbardino jonasbardino marked this pull request as ready for review May 29, 2026 21:04
@albu-diku albu-diku self-requested a review June 4, 2026 08:52

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

That's a good place to do it I think - just double checked and the last set of changes in the area did unify on the ..._dirs function as I'd remembered.

Two ideas - it might be worth having a constant that lists which keys of users to loop over calling mkdir, the idea would be keep similarity with _USERADM_PATH_KEYS. The second is creating there directories unconditionally means a bunch of ensure_dirs_exist calls in the tests for the corresponding dirs are no longer needed and I wonder if it is worth cleaning those up here as well (mostly to save any confusion in the future).

comments to get the PR wrapped up. Minor format-only changes from automatic
long-line wrap in editor.
@jonasbardino

Copy link
Copy Markdown
Contributor Author

That's a good place to do it I think - just double checked and the last set of changes in the area did unify on the ..._dirs function as I'd remembered.

Two ideas - it might be worth having a constant that lists which keys of users to loop over calling mkdir, the idea would be keep similarity with _USERADM_PATH_KEYS. The second is creating there directories unconditionally means a bunch of ensure_dirs_exist calls in the tests for the corresponding dirs are no longer needed and I wonder if it is worth cleaning those up here as well (mostly to save any confusion in the future).

Thanks for the review and insights @albu-diku . I've added comments to link to the _USERADM_PATH_KEYS variable and a TODO about cleaning up old related _ensure_dirs_exist for a later occasion, as I've already taken too many detours from the main PR :-s

@jonasbardino jonasbardino merged commit bc931aa into next Jun 9, 2026
11 checks passed
@jonasbardino jonasbardino deleted the adjust/unit-test-usersupp-to-include-more-actual-user-dirs-and-polish branch June 9, 2026 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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