Skip to content

Update l10n test: check .po files for placeholders mismatch#3169

Merged
arkid15r merged 2 commits intovacanza:devfrom
KJhellico:upd-l10n-test
Dec 26, 2025
Merged

Update l10n test: check .po files for placeholders mismatch#3169
arkid15r merged 2 commits intovacanza:devfrom
KJhellico:upd-l10n-test

Conversation

@KJhellico
Copy link
Copy Markdown
Collaborator

@KJhellico KJhellico commented Dec 25, 2025

Proposed change

Update l10n test: check .po files for placeholders mismatch.
(I missed %s here).

Type of change

  • New country/market holidays support (thank you!)
  • Supported country/market holidays update (calendar discrepancy fix, localization)
  • Existing code/documentation/test/process quality improvement (best practice, cleanup, refactoring, optimization)
  • Dependency update (version deprecation/pin/upgrade)
  • Bugfix (non-breaking change which fixes an issue)
  • Breaking change (a code change causing existing functionality to break)
  • New feature (new holidays functionality in general)

Checklist

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Dec 25, 2025 •
edited
Loading

Summary by CodeRabbit

  • Tests
    • Enhanced localization quality assurance with improved validation ensuring translated content maintains consistent placeholder formatting across all localization files.

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

Adds placeholder validation to the localization test suite. The test now verifies that msgid and msgstr entries in PO files contain identical counts of placeholders matching the pattern %[a-zA-Z], using a Counter comparison. Introduces a regex pattern and imports Counter from collections.

Changes

Cohort / File(s) Summary
Test localization validation
tests/test_l10n.py		 Adds Counter import from collections. Defines new regex pattern placeholder_re to match placeholders (%[a-zA-Z]). Implements validation logic in test_localization that iterates over PO file entries and asserts placeholder count parity between msgid and msgstr.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

l10n, test

Suggested reviewers

  • arkid15r
  • PPsyrius

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title directly and clearly describes the main change: adding a check for placeholder mismatches in .po localization files.
Description check ✅ Passed The description relates to the changeset by explaining the motivation for the update and referencing a missed placeholder that prompted the test improvement.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6f1fc6c and dbe9272.

📒 Files selected for processing (1)
  • tests/test_l10n.py
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings 
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2608
File: holidays/locale/en_US/LC_MESSAGES/VC.po:76-76
Timestamp: 2025-06-10T05:08:07.939Z
Learning: For the holidays project, localization file formatting issues (like missing terminal periods in .po files) should be automatically fixed by running `make l10n` command (which is included in `make check`). Authors should be directed to use this automated tooling instead of manual formatting fixes.

Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2608
File: holidays/locale/en_VC/LC_MESSAGES/VC.po:61-66
Timestamp: 2025-06-10T05:07:29.372Z
Learning: For missing translator comments in .po localization files in the holidays repository, direct authors to run `make l10n` or `make check` commands instead of suggesting manual fixes, as these commands automatically handle translator comment generation.

Learnt from: KJhellico
Repo: vacanza/holidays PR: 2960
File: holidays/locale/ca/LC_MESSAGES/ES.po:97-100
Timestamp: 2025-09-26T13:44:12.652Z
Learning: When fixing localization issues in the holidays project, corrections should be made in both the Python source code comments and the en_US .po file msgstr entries, then all other language .po files should be regenerated to pick up the corrected en_US comments.

Learnt from: KJhellico
Repo: vacanza/holidays PR: 2676
File: holidays/locale/en_US/LC_MESSAGES/TN.po:17-28
Timestamp: 2025-06-26T15:34:35.476Z
Learning: In the holidays project, .po file header metadata updates (version numbers, revision dates, translator information) are legitimate changes when part of localization work and don't require `make l10n` regeneration. The `make l10n` command is primarily for formatting fixes and missing translator comments, not for intentional metadata updates.

Learnt from: KJhellico
Repo: vacanza/holidays PR: 2651
File: holidays/locale/nl/LC_MESSAGES/BQ.po:29-95
Timestamp: 2025-06-25T20:55:00.642Z
Learning: In the holidays library, Dutch locale files (.po) with `X-Source-Language: nl` should have empty msgstr entries when the target language is also Dutch. The library uses fallback=True with gettext, which returns the original msgid when msgstr is empty. This is the correct pattern for native language files and does not cause blank holiday names.

Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2944
File: holidays/countries/myanmar.py:108-111
Timestamp: 2025-09-17T15:53:16.940Z
Learning: In the holidays package, explanatory comments about year restrictions or policy context should be placed above conditional blocks or at method level, never directly above holiday name comments that precede tr() function calls, as this would include the explanatory text in .po localization files when running `make l10n`.

Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2537
File: holidays/countries/finland.py:195-199
Timestamp: 2025-05-09T18:34:33.990Z
Learning: In the holidays library, localization (l10n) is handled through separate .po files for different languages rather than combining multiple translations in a single string. The code should use the default language with tr() function, and translations are provided in language-specific .po files.

Learnt from: KJhellico
Repo: vacanza/holidays PR: 2398
File: tests/countries/test_guinea.py:237-239
Timestamp: 2025-04-02T18:38:35.164Z
Learning: In the vacanza/holidays project, the method assertLocalizedHolidays in country test files should be called with positional parameters rather than named parameters to maintain consistency with the rest of the codebase.

Learnt from: KJhellico
Repo: vacanza/holidays PR: 2860
File: holidays/countries/burkina_faso.py:27-30
Timestamp: 2025-08-26T21:24:41.827Z
Learning: Countries in the holidays library that don't have localization support yet should use plain English strings for labels (e.g., `estimated_label = "%s (estimated)"`), while only countries with existing .po translation files should use `tr()` wrapping. Check for the presence of .po files in holidays/locale to determine if a country has localization support.

Learnt from: KJhellico
Repo: vacanza/holidays PR: 2960
File: holidays/locale/ca/LC_MESSAGES/ES.po:97-100
Timestamp: 2025-09-26T13:44:12.652Z
Learning: In the holidays project, localization fixes should be made in the source Python code files, not directly in .po files. After fixing comments or translations in the source code, the .po files should be regenerated using the proper localization workflow.

Learnt from: KJhellico
Repo: vacanza/holidays PR: 2607
File: scripts/l10n/l10n_helper.py:147-150
Timestamp: 2025-06-06T15:22:39.950Z
Learning: In the holidays project localization system, all .po files are generated from the same .pot template for all supported languages, ensuring that all msgids exist for all languages. This makes defensive programming for missing language keys in po_data[msgid] unnecessary in the normal workflow.

Learnt from: KJhellico
Repo: vacanza/holidays PR: 2653
File: holidays/locale/th/LC_MESSAGES/TW.po:17-21
Timestamp: 2025-06-21T18:06:50.027Z
Learning: KJhellico's username includes a tilde character (~) as part of their nickname (appears as "~Jhellico" in Last-Translator headers), which is intentional formatting and not an error.
📚 Learning: 2025-11-08T05:36:32.788Z 
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2881
File: tests/countries/test_faroe_islands.py:16-16
Timestamp: 2025-11-08T05:36:32.788Z
Learning: In PR #2881 onwards, the vacanza/holidays project moved to centralized alias testing via the `check_aliases` method in `tests/common.py`. Individual country test files no longer need to import country code aliases (e.g., FO, FRO for Faroe Islands) or define `test_country_aliases` methods. The common test framework automatically validates all aliases by dynamically importing them from the registry and calling `assertAliases`.

Applied to files:

  • tests/test_l10n.py
📚 Learning: 2025-08-26T21:24:41.827Z 
Learnt from: KJhellico
Repo: vacanza/holidays PR: 2860
File: holidays/countries/burkina_faso.py:27-30
Timestamp: 2025-08-26T21:24:41.827Z
Learning: Countries in the holidays library that don't have localization support yet should use plain English strings for labels (e.g., `estimated_label = "%s (estimated)"`), while only countries with existing .po translation files should use `tr()` wrapping. Check for the presence of .po files in holidays/locale to determine if a country has localization support.

Applied to files:

  • tests/test_l10n.py
📚 Learning: 2025-05-09T18:34:33.990Z 
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2537
File: holidays/countries/finland.py:195-199
Timestamp: 2025-05-09T18:34:33.990Z
Learning: In the holidays library, localization (l10n) is handled through separate .po files for different languages rather than combining multiple translations in a single string. The code should use the default language with tr() function, and translations are provided in language-specific .po files.

Applied to files:

  • tests/test_l10n.py
📚 Learning: 2025-09-26T13:44:12.652Z 
Learnt from: KJhellico
Repo: vacanza/holidays PR: 2960
File: holidays/locale/ca/LC_MESSAGES/ES.po:97-100
Timestamp: 2025-09-26T13:44:12.652Z
Learning: In the holidays project, localization fixes should be made in the source Python code files, not directly in .po files. After fixing comments or translations in the source code, the .po files should be regenerated using the proper localization workflow.

Applied to files:

  • tests/test_l10n.py
🪛 Ruff (0.14.10)
tests/test_l10n.py

88-88: Use a regular assert instead of unittest-style assertEqual

Replace assertEqual(...) with assert ...

(PT009)

⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: Test Python 3.10 on windows-latest
  • GitHub Check: Test Python 3.12 on windows-latest
  • GitHub Check: Test Python 3.14 on windows-latest
  • GitHub Check: Test Python 3.13 on windows-latest
  • GitHub Check: Test Python 3.11 on windows-latest
🔇 Additional comments (3)
tests/test_l10n.py (3)

16-16: Import looks good.

Counter is the right tool for comparing placeholder frequencies between msgid and msgstr.

87-94: Placeholder validation logic looks solid.

The Counter comparison ensures both the count and types of placeholders match between source and translation. Good placement after the obsolete entries check, and the error message provides clear context for debugging.

41-41: The regex pattern %[a-zA-Z] is sufficient for this codebase. The locale files only use simple single-letter placeholders (%s, %Y, %d, %m), not complex formats like %.2f, %1$s, or %(name)s. No changes needed.

Likely an incorrect or invalid review comment.

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot added the test label Dec 25, 2025
@codecov
Copy link
Copy Markdown

codecov bot commented Dec 25, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (22b7f89) to head (4dfea5b).
⚠️ Report is 1 commits behind head on dev.

Additional details and impacted files 
@@            Coverage Diff            @@
##               dev     #3169   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          306       306           
  Lines        18264     18264           
  Branches      2330      2330           
=========================================
  Hits         18264     18264

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

arkid15r
arkid15r approved these changes Dec 26, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@arkid15r arkid15r left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@arkid15r arkid15r enabled auto-merge December 26, 2025 03:12
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Dec 26, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@arkid15r arkid15r added this pull request to the merge queue Dec 26, 2025
Merged via the queue into vacanza:dev with commit a3ac34f Dec 26, 2025
33 checks passed
@KJhellico KJhellico deleted the upd-l10n-test branch December 26, 2025 11:52
@arkid15r arkid15r mentioned this pull request Jan 5, 2026
Merged
@coderabbitai coderabbitai bot mentioned this pull request Feb 18, 2026
Merged
9 tasks
@coderabbitai coderabbitai bot mentioned this pull request Mar 6, 2026
Merged
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@arkid15r arkid15r arkid15r approved these changes

@coderabbitai coderabbitai[bot] coderabbitai[bot] approved these changes

@PPsyrius PPsyrius Awaiting requested review from PPsyrius PPsyrius is a code owner

Assignees

No one assigned

Labels

test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@KJhellico @arkid15r