Skip to content

Conversation

@adombeck
Copy link
Contributor

As discussed in #1082 (comment), we now only convert user and group names to lowercase in public API methods. In all other functions, we assume that the names are already in lowercase, to avoid repeatedly converting the same name to lowercase in each layer of our code.

Copy link
Contributor

@3v1n0 3v1n0 left a comment

Choose a reason for hiding this comment

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

Thanks for the cleanup!

// authd uses lowercase user and group names
uInfo.Name = strings.ToLower(uInfo.Name)
for i, g := range uInfo.Groups {
uInfo.Groups[i].Name = strings.ToLower(g.Name)
Copy link
Contributor

Choose a reason for hiding this comment

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

This part is already tested?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adombeck adombeck marked this pull request as ready for review November 17, 2025 22:44
@adombeck adombeck requested a review from a team as a code owner November 17, 2025 22:44
Remove leftover golden files from tests that don't exist anymore by
running

    git rm -r internal/**/testdata/golden
    TESTS_UPDATE_GOLDEN=1 go test ./internal/...
We use underscores instead of spaces in test names.
Immediately convert username to lowercase, for consistency with how it's
done in the other methods.
We use underscores instead of spaces in test names.
For consistency with the other *_with_uppercase test cases.
The tests change DB content, so we should also check the result.
We now only convert user and group names to lowercase in public API
methods. In all other functions, we assume that the names are already in
lowercase, to avoid repeatedly converting the same name to lowercase in
each layer of our code.
@adombeck adombeck force-pushed the convert-names-to-lowercase-in-apis branch from 6de67e4 to 8066a69 Compare November 17, 2025 22:45
@adombeck adombeck merged commit 73a529b into main Nov 17, 2025
12 of 14 checks passed
@adombeck adombeck deleted the convert-names-to-lowercase-in-apis branch November 17, 2025 22:45
@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 93.54839% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.78%. Comparing base (35545e4) to head (8066a69).
⚠️ Report is 17 commits behind head on main.

Files with missing lines Patch % Lines
internal/services/pam/pam.go 90.47% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1125      +/-   ##
==========================================
- Coverage   87.79%   87.78%   -0.01%     
==========================================
  Files          89       89              
  Lines        6151     6150       -1     
  Branches      111      111              
==========================================
- Hits         5400     5399       -1     
  Misses        695      695              
  Partials       56       56              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants