[Tests] Add LocalizationManager broadcastMessage and PAPI branch coverage

[DiamondDagger590]

Summary

• New LocalizationManagerBroadcastTest — 10 tests covering both broadcastMessage(Route) and broadcastMessage(Route, Map) overloads, exercising loaded player, unloaded player, console, mixed, and no-players-online paths
• 4 PAPI branch coverage tests added to LocalizationManagerTest — covers the CorePapiHook present+online and present+offline branches for both getLocalizedMessage(CorePlayer, Route) and getLocalizedMessages(CorePlayer, Route)
• 1 multi-locale message format test added to NoLocalizationContainsMessageExceptionTest — verifies all locale display names appear in the exception message when multiple locales are checked

Coverage Impact

Class	Line Coverage	Branch Coverage
LocalizationManager	100%	95.7%
NoLocalizationContainsMessageException	100%	—

Test plan

• All new tests pass (./gradlew test)
• Full test suite passes with no regressions
• Testing audit persona reviewed — addressed feedback (added content assertions to Route-only broadcast tests)
• JaCoCo coverage report confirms improvement

---

Generated by Claude Code

Summary by CodeRabbit

• Tests
  • Added coverage for localized exception messages containing multiple locale names.
  • Added tests validating localization broadcasts to online players, offline players, the console, and mixed recipients.
  • Added coverage for placeholder substitution in broadcast messages, including empty placeholder values.
  • Added tests confirming PlaceholderAPI translation for single and list-based localization messages.

[github-actions]

Claude finished @DiamondDagger590's task in 4m 34s —— View job

---

Reviewing PR #90

• Gather context (PR diff, changed files)
• Route to review lenses — test-only PR (src/test/java/**), so only review-testing applies
• Run review-testing lens
• Consolidate findings
• Post final review

---

No blocking issues found

This is a test-only PR adding coverage for LocalizationManager.broadcastMessage (both overloads) and PAPI branch handling, plus a multi-locale exception message test. Since no src/main files changed, only the testing lens applies (security/architecture/error-handling/performance/extensibility all key off src/main/**/*.java and had nothing to review). The new PAPI branch tests correctly exercise the hook-present+online vs. hook-present+offline paths against the actual LocalizationManager source, and the broadcastMessage tests assert real message content (not just non-null checks), consistent with the PR description's note that this was already addressed from prior audit feedback.

Nits (2)

• LocalizationManagerBroadcastTest.java:135-300 — Test method names inside the @Nested classes (e.g. loadedPlayerReceivesLocalizedMessage, consoleReceivesDefaultMessage) don't follow the project's methodUnderTest_expectedOutcome_whenCondition convention used elsewhere (e.g. MutexDAOTest's attemptCreateTable_returnsFalse_whenTableExists). Consider renaming, e.g. broadcastMessage_sendsToConsole_whenNoOnlinePlayers.
• LocalizationManagerTest.java:284,317 — Adds two more // --- Section --- decorative divider comments, extending a pre-existing pattern in this file that CLAUDE.md explicitly lists as an anti-pattern. Not a fresh violation (the file already uses this convention throughout), but worth avoiding going forward — @Nested + @DisplayName (as used in the new LocalizationManagerBroadcastTest.java) is the preferred grouping mechanism.

[coderabbitai]

📝 Walkthrough

Walkthrough

The pull request adds tests for localization broadcasts, PlaceholderAPI translation, and exception messages containing multiple locale display names.

Changes

Localization test coverage

Layer / File(s)	Summary
Localization broadcast coverage src/test/java/com/diamonddagger590/mccore/localization/LocalizationManagerBroadcastTest.java	Adds MockBukkit tests for player and console broadcasts, loaded and unloaded players, mixed recipients, empty recipients, and placeholder substitution.
PlaceholderAPI translation coverage src/test/java/com/diamonddagger590/mccore/localization/LocalizationManagerTest.java	Adds tests for single and list message translation for online players and unchanged messages for offline players.
Locale display-name assertion src/test/java/com/diamonddagger590/mccore/exception/localization/NoLocalizationContainsMessageExceptionTest.java	Verifies that an exception message contains the display names of English and French locales.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• DiamondDagger590/McCore#37: Both pull requests add tests for LocalizationManager localization behavior.
• DiamondDagger590/McCore#82: This pull request extends the LocalizationManagerBroadcastTest coverage with broadcast and PlaceholderAPI cases.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main additions for LocalizationManager broadcastMessage and PlaceholderAPI branch coverage, although it omits the exception test.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/eager-thompson-9ab00w

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Note

Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.

🟡 Other comments (1)

src/test/java/com/diamonddagger590/mccore/localization/LocalizationManagerBroadcastTest.java-133-148 (1)

133-148: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Test locale-specific delivery for loaded players.

Both tests use only the default English localization. They pass if broadcastMessage incorrectly resolves loaded players through the default locale.

• src/test/java/com/diamonddagger590/mccore/localization/LocalizationManagerBroadcastTest.java#L133-L148: Register a distinct non-default localization for the loaded player's locale and assert its distinct message.
• src/test/java/com/diamonddagger590/mccore/localization/LocalizationManagerBroadcastTest.java#L218-L233: Use a distinct non-default template and assert both locale resolution and placeholder substitution.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/test/java/com/diamonddagger590/mccore/localization/LocalizationManagerBroadcastTest.java`
around lines 133 - 148, Update loadedPlayerReceivesLocalizedMessage in
src/test/java/com/diamonddagger590/mccore/localization/LocalizationManagerBroadcastTest.java:133-148
to register a distinct non-default localization for the loaded player’s locale
and assert its distinct message. Also update the test at
src/test/java/com/diamonddagger590/mccore/localization/LocalizationManagerBroadcastTest.java:218-233
to use a distinct non-default template and assert both locale resolution and
placeholder substitution.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Other comments:
In
`@src/test/java/com/diamonddagger590/mccore/localization/LocalizationManagerBroadcastTest.java`:
- Around line 133-148: Update loadedPlayerReceivesLocalizedMessage in
src/test/java/com/diamonddagger590/mccore/localization/LocalizationManagerBroadcastTest.java:133-148
to register a distinct non-default localization for the loaded player’s locale
and assert its distinct message. Also update the test at
src/test/java/com/diamonddagger590/mccore/localization/LocalizationManagerBroadcastTest.java:218-233
to use a distinct non-default template and assert both locale resolution and
placeholder substitution.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: QUIET

Plan: Pro Plus

Run ID: 3dc93ee7-a211-4458-83a1-6fa26b7aa132

📥 Commits

Reviewing files that changed from the base of the PR and between 0fbf220 and 09b63eb.

📒 Files selected for processing (3)

• src/test/java/com/diamonddagger590/mccore/exception/localization/NoLocalizationContainsMessageExceptionTest.java
• src/test/java/com/diamonddagger590/mccore/localization/LocalizationManagerBroadcastTest.java
• src/test/java/com/diamonddagger590/mccore/localization/LocalizationManagerTest.java