Skip to content

test: comprehensive coverage for accented character names#123

Merged
TytaniumDev merged 2 commits into
mainfrom
test/accented-name-coverage
Mar 31, 2026
Merged

test: comprehensive coverage for accented character names#123
TytaniumDev merged 2 commits into
mainfrom
test/accented-name-coverage

Conversation

@TytaniumDev

Copy link
Copy Markdown
Owner

Summary

  • Expands test coverage for accented WoW character names added in fix: accept accented characters in community roster names #120
  • Tests umlauts (ü, ä, ö), acute/grave accents (é, à, í, ó, ú), circumflex (ô, û), cedilla (ç), tilde (ñ), ring (å), and ligatures (æ, œ)
  • Covers bare names, names starting with accented characters, and realm-qualified accented names
  • Adds end-to-end AddCommunityPlayer tests verifying accented names are normalized correctly

Test plan

  • 43 community service tests pass (up from 36)
  • Full suite: 323/323 pass
  • Luacheck clean

🤖 Generated with Claude Code

Add tests for umlauts, acute/grave accents, circumflex, cedilla, tilde,
ring, and ligatures — both bare names and realm-qualified. Also add
end-to-end AddCommunityPlayer tests with accented names.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

@github-actions github-actions 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.

All CI checks passed.

@claude claude 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 - Approved

No issues found. Checked for bugs, security issues, and CLAUDE.md compliance.

@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 significantly expands the test suite for CommunityService, adding comprehensive validation for community names containing various accented characters, ligatures, and diacritics. It also introduces tests for adding players with accented names to the roster. The review feedback suggests further strengthening these tests by verifying case-insensitive duplicate prevention—particularly important given Lua 5.1's lack of native UTF-8 support in string.lower()—and ensuring roster size assertions are consistently applied to prevent unexpected entries.

Comment on lines +215 to +220
it("should add accented name and normalize with realm", function()
local ok = WHLSN:AddCommunityPlayer("Müzaka")
assert.is_true(ok)
assert.equals(1, #WHLSN.db.profile.communityRoster)
assert.equals("Müzaka-Illidan", WHLSN.db.profile.communityRoster[1].name)
end)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The test suite for AddCommunityPlayer should include a case-insensitivity check for accented names to ensure the duplicate prevention logic works correctly for these characters. In Lua 5.1, string.lower() is not UTF-8 aware, which often leads to bugs where names like Müzaka and MÜZAKA are treated as distinct entries. Adding this test case would verify if the implementation handles this correctly or if a more robust normalization (like a custom UTF-8 lower function) is needed.

        it("should add accented name and normalize with realm", function()
            local ok = WHLSN:AddCommunityPlayer("Müzaka")
            assert.is_true(ok)
            assert.equals(1, #WHLSN.db.profile.communityRoster)
            assert.equals("Müzaka-Illidan", WHLSN.db.profile.communityRoster[1].name)

            -- Verify case-insensitive duplicate prevention for accented names
            local ok2 = WHLSN:AddCommunityPlayer("MÜZAKA")
            assert.is_false(ok2)
            assert.equals(1, #WHLSN.db.profile.communityRoster)
        end)

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

WoW enforces consistent character name casing (first letter uppercase, rest lowercase), so a case-differing accented name like MÜZAKA vs Müzaka is not a real-world scenario. Additionally, Lua 5.1's string.lower() isn't UTF-8 aware, so adding this assertion would actually fail — and fixing that with a custom UTF-8 lower function would be over-engineering for a case that can't occur in practice. Skipping this one.

Comment on lines +222 to +226
it("should add accented name with realm as-is", function()
local ok = WHLSN:AddCommunityPlayer("Élowen-Stormrage")
assert.is_true(ok)
assert.equals("Élowen-Stormrage", WHLSN.db.profile.communityRoster[1].name)
end)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

For consistency and thoroughness, this test should also verify that the roster size is exactly 1 after the addition, ensuring no unexpected entries were created.

        it("should add accented name with realm as-is", function()
            local ok = WHLSN:AddCommunityPlayer("Élowen-Stormrage")
            assert.is_true(ok)
            assert.equals(1, #WHLSN.db.profile.communityRoster)
            assert.equals("Élowen-Stormrage", WHLSN.db.profile.communityRoster[1].name)
        end)

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Good catch — added the roster size assertion in 87e20cd.

Addresses PR review comment — verify roster count after adding
an accented name with an explicit realm.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@TytaniumDev
TytaniumDev merged commit 8a17865 into main Mar 31, 2026
14 checks passed
@TytaniumDev
TytaniumDev deleted the test/accented-name-coverage branch March 31, 2026 04:01
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.

1 participant