Add sanitization-based assertions for locale-specific timing text comparisons#303
Add sanitization-based assertions for locale-specific timing text comparisons#303
Conversation
📝 WalkthroughWalkthroughA single test file has been refactored to replace inline assertion logic with locale-specific helper methods and a new sanitization utility. The changes introduce Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
common/src/test/java/io/github/jy95/fds/translators/AbstractTimingEventTest.java (1)
74-96: Consider adding unit tests for the sanitize method.The sanitization logic is well-documented and correctly implements normalization for CLDR variations. However, since this utility method is critical to test correctness, consider adding dedicated unit tests to verify its behavior with various inputs (null, empty string, different Unicode characters, edge cases).
Example test cases to consider
// Example structure (not a diff, just illustrative) @Test void testSanitizeHandlesNarrowNoBreakSpace() { assertEquals("Jan 1 2024", sanitize("Jan\u202F1\u202F2024")); } @Test void testSanitizeRemovesPunctuation() { assertEquals("Jan 1 2024", sanitize("Jan. 1, 2024")); } @Test void testSanitizeHandlesNull() { assertEquals("", sanitize(null)); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
common/src/test/java/io/github/jy95/fds/translators/AbstractTimingEventTest.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: submit-pypi
- GitHub Check: Analyze (java-kotlin)
🔇 Additional comments (4)
common/src/test/java/io/github/jy95/fds/translators/AbstractTimingEventTest.java (4)
31-33: LGTM! Improved test resilience.The sanitization approach on both actual and expected values makes the test more robust against CLDR formatting variations while preserving semantic correctness checks.
39-48: LGTM! Clean locale mapping.The centralized expected-value mapping improves maintainability. The locale-specific strings appear correct for their respective formatting conventions.
55-57: LGTM! Consistent with testSingle approach.The changes maintain consistency with the single-day test pattern and correctly apply sanitization.
63-72: LGTM! Correct list formatting for multiple locales.The locale-specific list formatting (conjunctions and separators) appears correct for each supported language.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.