[Tests] Improve branch coverage for DAO deserializeStringSet and updateTable

[DiamondDagger590]

Summary

• Add 12 new branch-coverage tests to PlayerStatisticDAOSerializationTest targeting previously uncovered paths in deserializeStringSet: whitespace handling, unquoted elements, unknown escape sequences, backslash at end of input, mixed quoted/unquoted parsing, and trailing whitespace
• Add negative-version updateTable tests to both PlayerStatisticDAOTest and PlayerSettingDAOTest covering the early-return branch when lastStoredVersion is negative

Coverage Impact

Class	Branch Coverage Before	Branch Coverage After
PlayerStatisticDAO	78.7%	93.3%
PlayerSettingDAO	78.6%	82.1%

Remaining uncovered branches are JaCoCo bytecode artifacts (try-with-resources null checks, switch exhaustiveness default branches) that are impractical to exercise.

Test plan

• All 14 new tests pass locally via ./gradlew test
• Full test suite passes with no regressions
• JaCoCo coverage report confirms branch coverage improvements
• Testing audit persona applied — no concerns found

---

Generated by Claude Code

Summary by CodeRabbit

• Tests
  • Added regression coverage for database upgrades when stored table versions are negative.
  • Expanded serialization coverage for string sets, including quoted values, separators, whitespace, escape sequences, and mixed formats.

[github-actions]

Claude finished @DiamondDagger590's task in 5m 10s —— View job

---

PR Review: No blocking issues found

• Gather context (PR diff, changed files)
• Route to review lenses
• Fan out to lenses
• Consolidate findings
• Post final review

This PR only adds tests (no src/main changes), so the applicable lens is testing. It ran and returned clean.

Summary: 14 new tests target genuinely uncovered branches — the lastStoredVersion < 0 path in PlayerStatisticDAO.updateTable / PlayerSettingDAO.updateTable (distinct from the existing "current version" and "version 0" cases), and 12 previously-uncovered parsing branches in PlayerStatisticDAO.deserializeStringSet (leading/trailing whitespace, whitespace-only input, unquoted elements via both the commaIdx == -1 and commaIdx != -1 paths, unknown escape sequences, and the backslash-at-end-of-input edge case). Each test's assertions were hand-traced against the actual character-by-character parsing logic in deserializeStringSet (lines 301-362) and confirmed correct, including the trickier ones (backslashAtEndOfInput_included, unknownEscapeSequence_bothCharsPreserved, multipleUnknownEscapes_allPreserved). All new tests carry @DisplayName, follow the existing naming convention, and contain real assertions — no test infrastructure conventions (ManagedExecutorExtension, RegistryResetExtension, MockBukkit) are implicated.

No blocking issues found.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: QUIET

Plan: Pro Plus

Run ID: 3c724bb8-1d65-4c7c-9f28-639f21851217

📥 Commits

Reviewing files that changed from the base of the PR and between 0fbf220 and 5915e17.

📒 Files selected for processing (3)

• src/test/java/com/diamonddagger590/mccore/database/table/impl/PlayerSettingDAOTest.java
• src/test/java/com/diamonddagger590/mccore/database/table/impl/PlayerStatisticDAOSerializationTest.java
• src/test/java/com/diamonddagger590/mccore/database/table/impl/PlayerStatisticDAOTest.java

---

📝 Walkthrough

Walkthrough

The pull request adds regression tests for negative table versions in two DAOs. It also adds nested branch-coverage tests for deserializeStringSet, including quoting, separators, whitespace, and escape handling.

Changes

DAO test coverage

Layer / File(s)	Summary
Negative-version migration tests src/test/.../PlayerSettingDAOTest.java, src/test/.../PlayerStatisticDAOTest.java	Tests verify that updateTable does not record version 1 when the stored table version is negative.
String-set deserialization coverage src/test/.../PlayerStatisticDAOSerializationTest.java	Nested tests cover whitespace, quoted and unquoted values, separators, mixed formats, unknown escapes, trailing backslashes, and trailing whitespace.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• DiamondDagger590/McCore#87: Adds related regression tests for negative table versions in DAO updateTable methods.
• DiamondDagger590/McCore#74: Adds related PlayerStatisticDAO branch and migration coverage.
• DiamondDagger590/McCore#36: Adds overlapping PlayerSettingDAO migration tests.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the added branch-coverage tests for deserializeStringSet and updateTable.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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

📝 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-kbv0tc

---

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.}