Skip to content

Conversation

@arkid15r
Copy link
Collaborator

Proposed change

No actual test logic change is intended -- just their structure.

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
Contributor

coderabbitai bot commented Aug 12, 2025

Summary by CodeRabbit

  • Tests

    • Added focused unit tests for holiday groups: Balinese Saka, Christian, International, Persian, Thai/Khmer, Tibetan, and Mandaean calendars, covering out-of-range handling, date additions, and invalid input errors.
    • Reorganized test coverage by replacing a monolithic suite with category-specific modules.
  • Documentation

    • Added a header documentation block describing the project in the test suite.
    • Corrected capitalization in a Tibetan holiday description.
  • Chores

    • Removed an obsolete consolidated holiday groups test file to streamline the test structure.

Walkthrough

Reorganized group-oriented tests: added a package header to tests/groups/__init__.py, split the previous consolidated tests into several new group-specific test modules under tests/groups/, removed the old tests/test_holiday_groups.py, and made a small docstring wording change in holidays/groups/tibetan.py.

Changes

Cohort / File(s) Summary
Docs header
tests/groups/__init__.py
Added a multi-line package header comment (library description, authors, website, MIT license); no executable changes.
New group-specific tests
tests/groups/test_balinese_saka.py, tests/groups/test_christian.py, tests/groups/test_international.py, tests/groups/test_mandaean.py, tests/groups/test_persian.py, tests/groups/test_thai.py, tests/groups/test_tibetan.py
Added separate unit test modules exercising each calendar/mixin: initialization patterns, internal add* helpers, out-of-range behavior assertions, and some error-case checks (ValueError on invalid inputs).
Removed legacy consolidated tests
tests/test_holiday_groups.py
Deleted the monolithic test module that previously contained combined group tests.
Minor docstring edit
holidays/groups/tibetan.py
Small capitalization/text change in the _add_descending_day_of_lord_buddha docstring only; no logic change.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~15 minutes

Possibly related PRs

  • Add Iraq holidays #2763 — Reorganizes group-level tests and adds calendar-specific tests (overlaps the moved/added group test files and Mandaean calendar usage).

Suggested reviewers

  • KJhellico
  • PPsyrius
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a 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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added the test label Aug 12, 2025
@arkid15r arkid15r enabled auto-merge August 12, 2025 23:27
@codecov
Copy link

codecov bot commented Aug 12, 2025

Codecov Report

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

Additional details and impacted files
@@            Coverage Diff            @@
##               dev     #2803   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          284       284           
  Lines        17049     17049           
  Branches      2253      2253           
=========================================
  Hits         17049     17049           

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d17ddae and 72d25b5.

📒 Files selected for processing (9)
  • tests/groups/__init__.py (1 hunks)
  • tests/groups/test_balinese_saka.py (1 hunks)
  • tests/groups/test_christian.py (1 hunks)
  • tests/groups/test_international.py (1 hunks)
  • tests/groups/test_mandaean.py (1 hunks)
  • tests/groups/test_persian.py (1 hunks)
  • tests/groups/test_thai.py (1 hunks)
  • tests/groups/test_tibetan.py (1 hunks)
  • tests/test_holiday_groups.py (0 hunks)
💤 Files with no reviewable changes (1)
  • tests/test_holiday_groups.py
🧰 Additional context used
🧠 Learnings (28)
📓 Common learnings
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:52-64
Timestamp: 2025-04-05T04:47:27.213Z
Learning: For holiday tests in the vacanza/holidays project, structure tests by individual holidays rather than by years. Each test method should focus on a specific holiday and test it across multiple years (from start_year through 2050) using helper methods like `assertHolidayName`. For fixed holidays, use generators like `(f"{year}-01-01" for year in range(1991, 2051))`. For movable holidays, specify individual dates for specific years followed by a range check.
Learnt from: PPsyrius
PR: vacanza/holidays#2615
File: tests/countries/test_anguilla.py:1-12
Timestamp: 2025-06-16T15:48:48.680Z
Learning: Test files in the holidays repository follow a standardized structure without module or class docstrings. All country test files use the same pattern: license header, imports, and class definition (`class Test{Country}(CommonCountryTests, TestCase):`) without docstrings. This is an established codebase convention that should be maintained for consistency.
Learnt from: PPsyrius
PR: vacanza/holidays#2386
File: tests/countries/test_nepal.py:499-536
Timestamp: 2025-04-05T06:49:06.217Z
Learning: In the holidays project, test files follow a dual testing approach: individual methods test specific holidays across multiple years, while comprehensive year-specific tests (e.g., `test_2025`) verify all holidays for a specific year in a single assertion. Both approaches serve different testing purposes and complement each other.
📚 Learning: 2025-06-19T05:54:49.792Z
Learnt from: PPsyrius
PR: vacanza/holidays#2642
File: holidays/countries/french_polynesia.py:1-12
Timestamp: 2025-06-19T05:54:49.792Z
Learning: The holidays library uses a standard file header format across all country implementation files consisting of a comprehensive comment block with project description, authors, website, and license information. Country files do not use module-level docstrings but instead rely on this header format followed by class-level docstrings.

Applied to files:

  • tests/groups/__init__.py
📚 Learning: 2025-06-16T11:42:28.293Z
Learnt from: PPsyrius
PR: vacanza/holidays#2638
File: holidays/countries/svalbard_and_jan_mayen.py:1-12
Timestamp: 2025-06-16T11:42:28.293Z
Learning: In the holidays library codebase, country implementation files in holidays/countries/ follow a standard convention of NOT having module-level docstrings. They start with the license header comment block, followed by imports, then class definitions. This is consistent across all country implementations like austria.py, belgium.py, canada.py, etc.

Applied to files:

  • tests/groups/__init__.py
📚 Learning: 2025-06-16T15:48:48.680Z
Learnt from: PPsyrius
PR: vacanza/holidays#2615
File: tests/countries/test_anguilla.py:1-12
Timestamp: 2025-06-16T15:48:48.680Z
Learning: Test files in the holidays repository follow a standardized structure without module or class docstrings. All country test files use the same pattern: license header, imports, and class definition (`class Test{Country}(CommonCountryTests, TestCase):`) without docstrings. This is an established codebase convention that should be maintained for consistency.

Applied to files:

  • tests/groups/__init__.py
📚 Learning: 2025-04-26T19:55:09.581Z
Learnt from: KJhellico
PR: vacanza/holidays#2501
File: scripts/docs/gen_index.py:1-12
Timestamp: 2025-04-26T19:55:09.581Z
Learning: The holidays project uses a standard header for all Python (.py) files in the repository that describes the overall holidays library, including authors, website and license information, regardless of the specific purpose of individual script files.

Applied to files:

  • tests/groups/__init__.py
📚 Learning: 2025-06-21T16:30:12.749Z
Learnt from: KJhellico
PR: vacanza/holidays#2654
File: holidays/countries/cape_verde.py:1-12
Timestamp: 2025-06-21T16:30:12.749Z
Learning: The holidays project does not use module docstrings in country holiday files. All country files start directly with the copyright header comment block without module docstrings, maintaining a consistent coding style across the project.

Applied to files:

  • tests/groups/__init__.py
📚 Learning: 2025-06-22T21:33:17.854Z
Learnt from: KJhellico
PR: vacanza/holidays#2671
File: holidays/countries/libya.py:1-1
Timestamp: 2025-06-22T21:33:17.854Z
Learning: In the holidays library, country files do not use module docstrings. The established pattern is: license header comment block, imports, then class definitions with class docstrings. Module docstrings should not be added to country files as they don't follow this convention.

Applied to files:

  • tests/groups/__init__.py
📚 Learning: 2025-06-25T10:36:39.909Z
Learnt from: PPsyrius
PR: vacanza/holidays#2678
File: tests/countries/test_united_states_virgin_islands.py:59-84
Timestamp: 2025-06-25T10:36:39.909Z
Learning: In the holidays library, test methods typically do not have docstrings. Only special test methods that need specific explanation (like edge cases or unique behaviors) have docstrings. Regular test methods like test_l10n_default, test_l10n_th, test_government_holidays, etc. should not have docstrings added.

Applied to files:

  • tests/groups/__init__.py
📚 Learning: 2025-04-24T04:59:38.436Z
Learnt from: PPsyrius
PR: vacanza/holidays#2489
File: holidays/countries/sao_tome_and_principe.py:1-12
Timestamp: 2025-04-24T04:59:38.436Z
Learning: The holidays library uses a standard file header across country implementation files that includes copyright information, authors, website, and license details. New country implementation files should follow this same header format.

Applied to files:

  • tests/groups/__init__.py
📚 Learning: 2025-06-22T21:36:18.015Z
Learnt from: KJhellico
PR: vacanza/holidays#2671
File: tests/countries/test_libya.py:19-24
Timestamp: 2025-06-22T21:36:18.015Z
Learning: In the vacanza/holidays project, test classes for countries do not use docstrings. All test classes follow the same pattern: class declaration directly followed by classmethod def setUpClass(cls) without any docstrings for the class or methods.

Applied to files:

  • tests/groups/__init__.py
📚 Learning: 2025-03-04T11:32:45.095Z
Learnt from: PPsyrius
PR: vacanza/holidays#2323
File: holidays/countries/macau.py:407-461
Timestamp: 2025-03-04T11:32:45.095Z
Learning: In the holidays library, the standard approach for organizing static holidays is to use separate dictionaries for different categories (`government`, `mandatory`, and `public`), which are utilized by syntactic sugar methods.

Applied to files:

  • tests/groups/__init__.py
📚 Learning: 2025-08-11T13:48:45.905Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/calendars/ethiopian.py:13-13
Timestamp: 2025-08-11T13:48:45.905Z
Learning: The holidays library does not use `__all__` declarations in calendar modules (holidays/calendars/). Calendar files follow a standard pattern of defining constants and functions directly without explicit exports, similar to the convention used in country modules.

Applied to files:

  • tests/groups/__init__.py
📚 Learning: 2025-03-04T11:32:45.095Z
Learnt from: PPsyrius
PR: vacanza/holidays#2323
File: holidays/countries/macau.py:407-461
Timestamp: 2025-03-04T11:32:45.095Z
Learning: In the holidays library, the standard approach for organizing static holidays is to use separate dictionaries for different categories (like `special_government_holidays`, `special_mandatory_holidays`, and `special_public_holidays`), which are utilized by syntactic sugar methods to pick up the appropriate holidays based on the selected category.

Applied to files:

  • tests/groups/__init__.py
📚 Learning: 2025-03-19T16:54:58.657Z
Learnt from: PPsyrius
PR: vacanza/holidays#2354
File: holidays/countries/fiji.py:146-159
Timestamp: 2025-03-19T16:54:58.657Z
Learning: In the holidays library implementation, explicit holiday dates (like Diwali in Fiji) are only defined for historical years with official sources (2016-2025). Future dates beyond the explicitly defined range are automatically calculated by methods like `_add_diwali`, which provide approximations when official dates aren't yet available.

Applied to files:

  • tests/groups/__init__.py
📚 Learning: 2025-08-03T13:48:11.910Z
Learnt from: KJhellico
PR: vacanza/holidays#2777
File: holidays/countries/gambia.py:120-122
Timestamp: 2025-08-03T13:48:11.910Z
Learning: When reviewing holiday implementations in the holidays library, defer to the maintainers' choice of start years for specific holiday policies, as they likely have access to more reliable primary sources and official documentation than what can be found through web searches.

Applied to files:

  • tests/groups/__init__.py
📚 Learning: 2025-07-10T03:36:16.461Z
Learnt from: PPsyrius
PR: vacanza/holidays#2706
File: holidays/countries/cayman_islands.py:80-97
Timestamp: 2025-07-10T03:36:16.461Z
Learning: In the holidays library, date dictionaries that map years to specific dates (like queens_birthday_dates, spring_bank_dates, thanksgiving_day_dates, etc.) are typically defined within the _populate_public_holidays method rather than as module-level constants. This is the established library-wide pattern seen across multiple country implementations including United Kingdom, United States, Sri Lanka, and others.

Applied to files:

  • tests/groups/__init__.py
📚 Learning: 2025-04-05T04:47:27.213Z
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:52-64
Timestamp: 2025-04-05T04:47:27.213Z
Learning: For holiday tests in the vacanza/holidays project, structure tests by individual holidays rather than by years. Each test method should focus on a specific holiday and test it across multiple years (from start_year through 2050) using helper methods like `assertHolidayName`. For fixed holidays, use generators like `(f"{year}-01-01" for year in range(1991, 2051))`. For movable holidays, specify individual dates for specific years followed by a range check.

Applied to files:

  • tests/groups/test_christian.py
  • tests/groups/test_international.py
  • tests/groups/test_balinese_saka.py
  • tests/groups/test_mandaean.py
  • tests/groups/test_tibetan.py
  • tests/groups/test_persian.py
  • tests/groups/test_thai.py
📚 Learning: 2025-04-05T06:49:06.217Z
Learnt from: PPsyrius
PR: vacanza/holidays#2386
File: tests/countries/test_nepal.py:499-536
Timestamp: 2025-04-05T06:49:06.217Z
Learning: In the holidays project, test files follow a dual testing approach: individual methods test specific holidays across multiple years, while comprehensive year-specific tests (e.g., `test_2025`) verify all holidays for a specific year in a single assertion. Both approaches serve different testing purposes and complement each other.

Applied to files:

  • tests/groups/test_christian.py
  • tests/groups/test_international.py
  • tests/groups/test_balinese_saka.py
  • tests/groups/test_mandaean.py
  • tests/groups/test_tibetan.py
  • tests/groups/test_persian.py
  • tests/groups/test_thai.py
📚 Learning: 2025-07-09T21:16:35.145Z
Learnt from: KJhellico
PR: vacanza/holidays#2623
File: tests/countries/test_christmas_island.py:136-146
Timestamp: 2025-07-09T21:16:35.145Z
Learning: In Christmas Island's holiday implementation, the test_christmas_day method cannot use assertNoNonObservedHoliday because in some years observed Christmas Day overlaps with Boxing Day when both holidays are moved due to weekend conflicts, causing the standard non-observed holiday check to fail inappropriately.

Applied to files:

  • tests/groups/test_christian.py
  • tests/groups/test_international.py
📚 Learning: 2025-04-05T04:29:38.042Z
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:85-86
Timestamp: 2025-04-05T04:29:38.042Z
Learning: For testing holiday implementations in the vacanza/holidays repository, recommend using `from tests.common import CommonCountryTests` as the base class instead of directly using `unittest.TestCase` to maintain consistency with project conventions and leverage common test utilities.

Applied to files:

  • tests/groups/test_christian.py
  • tests/groups/test_international.py
  • tests/groups/test_balinese_saka.py
  • tests/groups/test_mandaean.py
  • tests/groups/test_tibetan.py
  • tests/groups/test_persian.py
  • tests/groups/test_thai.py
📚 Learning: 2025-04-05T04:50:40.752Z
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:31-49
Timestamp: 2025-04-05T04:50:40.752Z
Learning: For Turkmenistan holiday tests, use this class structure: `class TestTurkmenistan(CommonCountryTests, TestCase)` with imports `from unittest import TestCase`, `from holidays.countries import Turkmenistan, TM, TKM`, and `from tests.common import CommonCountryTests`. Ensure to call `super().setUp()` in the setUp method.

Applied to files:

  • tests/groups/test_christian.py
  • tests/groups/test_international.py
  • tests/groups/test_balinese_saka.py
  • tests/groups/test_mandaean.py
  • tests/groups/test_tibetan.py
  • tests/groups/test_persian.py
  • tests/groups/test_thai.py
📚 Learning: 2025-08-12T17:16:54.458Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: tests/calendars/test_julian.py:35-36
Timestamp: 2025-08-12T17:16:54.458Z
Learning: In the vacanza/holidays project calendar tests (Thai, Ethiopian, Julian, etc.), the established testing pattern for validation methods is to use simple for loops like `for year in known_data_dict:` followed by `self.assertEqual(expected, actual)` without using unittest's subTest feature. This pattern is consistently maintained across all calendar test files.

Applied to files:

  • tests/groups/test_christian.py
  • tests/groups/test_balinese_saka.py
  • tests/groups/test_mandaean.py
  • tests/groups/test_tibetan.py
  • tests/groups/test_persian.py
  • tests/groups/test_thai.py
📚 Learning: 2025-07-02T18:17:53.342Z
Learnt from: KJhellico
PR: vacanza/holidays#2608
File: tests/countries/test_saint_vincent_and_the_grenadines.py:162-178
Timestamp: 2025-07-02T18:17:53.342Z
Learning: In the Saint Vincent and the Grenadines holidays implementation, New Year's Day is added without observed rules using `_add_new_years_day()` and should not include observed rule testing in its test method. Only holidays explicitly wrapped with `_add_observed()` have observed behavior.

Applied to files:

  • tests/groups/test_international.py
  • tests/groups/test_balinese_saka.py
  • tests/groups/test_tibetan.py
  • tests/groups/test_persian.py
  • tests/groups/test_thai.py
📚 Learning: 2025-05-06T21:07:11.577Z
Learnt from: KJhellico
PR: vacanza/holidays#2530
File: tests/countries/test_andorra.py:23-28
Timestamp: 2025-05-06T21:07:11.577Z
Learning: In the holidays project, test methods for country holidays follow a standard form where year ranges are explicitly recreated in each test method rather than being stored as class variables, to maintain consistency across all country tests.

Applied to files:

  • tests/groups/test_balinese_saka.py
  • tests/groups/test_mandaean.py
  • tests/groups/test_tibetan.py
  • tests/groups/test_persian.py
📚 Learning: 2025-04-04T10:52:41.546Z
Learnt from: KJhellico
PR: vacanza/holidays#2398
File: holidays/countries/guinea.py:106-110
Timestamp: 2025-04-04T10:52:41.546Z
Learning: In the Guinea holidays implementation, observed Eid al-Fitr cases are properly covered by the test_eid_al_fitr_day() method, which tests both the regular holiday dates and the observed dates when the holiday falls on a non-working day (for years >= 2023).

Applied to files:

  • tests/groups/test_balinese_saka.py
  • tests/groups/test_mandaean.py
  • tests/groups/test_persian.py
📚 Learning: 2025-04-04T10:52:41.546Z
Learnt from: KJhellico
PR: vacanza/holidays#2398
File: holidays/countries/guinea.py:106-110
Timestamp: 2025-04-04T10:52:41.546Z
Learning: In the Guinea holidays implementation, observed Eid al-Fitr cases are covered by the test_eid_al_fitr_day() method, which tests both regular holiday dates and the observed dates when the holiday falls on a non-working day (for years >= 2023).

Applied to files:

  • tests/groups/test_balinese_saka.py
📚 Learning: 2025-05-09T18:36:09.607Z
Learnt from: PPsyrius
PR: vacanza/holidays#2537
File: tests/countries/test_finland.py:23-26
Timestamp: 2025-05-09T18:36:09.607Z
Learning: The holidays project prioritizes complete historical coverage in tests, verifying holidays from their first year of observance (e.g., 1853 for Finland) through future projections, rather than using shorter sliding windows.

Applied to files:

  • tests/groups/test_balinese_saka.py
📚 Learning: 2025-08-12T17:16:54.458Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: tests/calendars/test_julian.py:35-36
Timestamp: 2025-08-12T17:16:54.458Z
Learning: In the vacanza/holidays project, test methods that iterate over dictionaries of test data (like calendar drift tests) follow the established pattern without using unittest's subTest, as evidenced by the Thai calendar tests and maintained consistently across similar test structures.

Applied to files:

  • tests/groups/test_tibetan.py
  • tests/groups/test_thai.py
🧬 Code Graph Analysis (7)
tests/groups/test_christian.py (1)
holidays/groups/christian.py (1)
  • _add_christmas_day_three (228-235)
tests/groups/test_international.py (2)
holidays/groups/international.py (1)
  • _add_childrens_day (56-76)
holidays/holiday_base.py (1)
  • HolidayBase (57-1296)
tests/groups/test_balinese_saka.py (3)
holidays/groups/balinese_saka.py (2)
  • BalineseSakaCalendarHolidays (19-45)
  • _add_nyepi (35-45)
holidays/holiday_base.py (1)
  • HolidayBase (57-1296)
tests/test_holiday_groups.py (1)
  • TestBalineseSakaCalendarHolidays (28-42)
tests/groups/test_mandaean.py (5)
holidays/groups/mandaean.py (6)
  • MandaeanHolidays (20-107)
  • _add_dehwa_daimana_day (70-78)
  • _add_parwanaya_day (48-56)
  • _add_great_feast_day (92-98)
  • _add_dehwa_hanina_day (80-87)
  • _add_mandaean_holiday (40-46)
holidays/holiday_base.py (1)
  • HolidayBase (57-1296)
tests/groups/test_international.py (1)
  • TestHolidays (21-24)
holidays/calendars/mandaean.py (1)
  • mandaean_to_gregorian (39-59)
tests/test_holiday_groups.py (1)
  • TestMandaeanCalendarHolidays (161-183)
tests/groups/test_tibetan.py (6)
tests/groups/test_balinese_saka.py (1)
  • TestHolidays (22-27)
tests/groups/test_mandaean.py (1)
  • TestHolidays (22-27)
tests/groups/test_christian.py (2)
  • TestHolidays (24-30)
  • _populate (29-30)
tests/groups/test_international.py (1)
  • TestHolidays (21-24)
tests/groups/test_persian.py (1)
  • TestHolidays (22-27)
tests/groups/test_thai.py (1)
  • TestHolidays (23-28)
tests/groups/test_persian.py (4)
holidays/groups/persian.py (8)
  • PersianCalendarHolidays (20-169)
  • _add_nowruz_day (111-121)
  • _add_islamic_republic_day (50-60)
  • _add_natures_day (100-109)
  • _add_death_of_khomeini_day (28-37)
  • _add_khordad_uprising_day (75-86)
  • _add_islamic_revolution_day (62-73)
  • _add_oil_nationalization_day (147-157)
holidays/holiday_base.py (1)
  • HolidayBase (57-1296)
tests/groups/test_international.py (1)
  • TestHolidays (21-24)
tests/test_holiday_groups.py (1)
  • TestPersianCalendarHolidays (84-104)
tests/groups/test_thai.py (2)
holidays/groups/thai.py (2)
  • _add_makha_bucha (117-133)
  • _add_visakha_bucha (189-206)
tests/groups/test_international.py (1)
  • TestHolidays (21-24)
⏰ 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). (3)
  • GitHub Check: Test build on ubuntu-latest
  • GitHub Check: Test build on windows-latest
  • GitHub Check: Test build on macos-latest
🔇 Additional comments (12)
tests/groups/__init__.py (1)

1-11: Header comment only is fine here.

Stays consistent with repo conventions (header comment, no module docstrings). No runtime effect.

tests/groups/test_christian.py (1)

23-37: LGTM on the behavior check for Christmas Day + 2.

Default calendar behavior (Gregorian) → Dec 27th for 2022 is correct. Pattern matches existing group tests.

tests/groups/test_international.py (1)

20-34: Solid coverage of both valid (NOV) and invalid variation.

Behavior matches InternationalHolidays._add_childrens_day contract and HolidayBase sugar methods.

tests/groups/test_balinese_saka.py (1)

19-33: LGTM: out-of-range Nyepi correctly results in zero additions.

The end_year guard behavior is affirmed without changing semantics from the prior aggregated test.

tests/groups/test_mandaean.py (1)

19-41: LGTM: out-of-range and invalid date inputs produce no additions.

Mirrors the original aggregated tests’ intent and relies on _mandaean conversion returning None for invalid month/day.

tests/groups/test_tibetan.py (3)

19-28: Good split; construction pattern is consistent.

The nested TestHolidays class correctly initializes the mixin before super().init and mirrors the pattern used in the other group tests.


29-41: All helper methods verified in holidays/groups/tibetan.py

All invoked private helpers in the test are present and stable—no accidental name drift detected. No further action needed.


22-26: Set an explicit end_year in the Tibetan test harness

I didn’t find any start_year/end_year in TibetanCalendarHolidays or _TibetanLunisolar, so the test runs without a fixed horizon. Other calendar group tests define end_year (e.g., Thai 2158, Persian 2102) to lock in their supported range and avoid flakiness.

• File: tests/groups/test_tibetan.py
• Location: the class TestHolidays(HolidayBase, TibetanCalendarHolidays) block
• Action: add

    end_year = <MAX_SUPPORTED_YEAR>

after the class declaration.

Please confirm the intended max year supported by _TibetanLunisolar and update the test accordingly.

tests/groups/test_persian.py (2)

19-39: Direct lift from the consolidated test; looks correct.

The end_year, populate year, invoked helpers, and assertion match the prior pattern for Persian. No logic change introduced.


31-39: All Persian helper methods are present and correctly named
The following methods called in tests/groups/test_persian.py all have matching definitions in holidays/groups/persian.py, so no changes are needed:

  • _add_nowruz_day
  • _add_islamic_republic_day
  • _add_natures_day
  • _add_death_of_khomeini_day
  • _add_khordad_uprising_day
  • _add_islamic_revolution_day
  • _add_oil_nationalization_day
tests/groups/test_thai.py (2)

20-29: LGTM; matches the established group test harness pattern.

Explicit end_year and mixin initialization are consistent with the new per-group test suite.


32-46: Sanity check passed: Thai holiday helpers and KHMER_CALENDAR are properly defined

  • KHMER_CALENDAR is defined in holidays/calendars/thai.py and re-exported in holidays/calendars/__init__.py.
  • All _add_* helper methods listed in tests/groups/test_thai.py exist in holidays/groups/thai.py.
  • Both _add_makha_bucha(self, name, calendar=None) and _add_visakha_bucha(self, name, calendar=None) accept an optional calendar argument.

No further changes needed here.

@sonarqubecloud
Copy link

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (1)
tests/groups/test_tibetan.py (1)

38-38: Use repository-standard spelling: “Thimphu Drubchen”.

Align the label with the helper name and group docs.

Apply:

-        test_holidays._add_thimphu_drubchen_day("Thimphu Drubchoe")
+        test_holidays._add_thimphu_drubchen_day("Thimphu Drubchen")
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 72d25b5 and e59f208.

📒 Files selected for processing (2)
  • holidays/groups/tibetan.py (1 hunks)
  • tests/groups/test_tibetan.py (1 hunks)
🧰 Additional context used
🧠 Learnings (15)
📓 Common learnings
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:52-64
Timestamp: 2025-04-05T04:47:27.213Z
Learning: For holiday tests in the vacanza/holidays project, structure tests by individual holidays rather than by years. Each test method should focus on a specific holiday and test it across multiple years (from start_year through 2050) using helper methods like `assertHolidayName`. For fixed holidays, use generators like `(f"{year}-01-01" for year in range(1991, 2051))`. For movable holidays, specify individual dates for specific years followed by a range check.
📚 Learning: 2025-04-05T04:47:27.213Z
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:52-64
Timestamp: 2025-04-05T04:47:27.213Z
Learning: For holiday tests in the vacanza/holidays project, structure tests by individual holidays rather than by years. Each test method should focus on a specific holiday and test it across multiple years (from start_year through 2050) using helper methods like `assertHolidayName`. For fixed holidays, use generators like `(f"{year}-01-01" for year in range(1991, 2051))`. For movable holidays, specify individual dates for specific years followed by a range check.

Applied to files:

  • tests/groups/test_tibetan.py
📚 Learning: 2025-04-05T06:49:06.217Z
Learnt from: PPsyrius
PR: vacanza/holidays#2386
File: tests/countries/test_nepal.py:499-536
Timestamp: 2025-04-05T06:49:06.217Z
Learning: In the holidays project, test files follow a dual testing approach: individual methods test specific holidays across multiple years, while comprehensive year-specific tests (e.g., `test_2025`) verify all holidays for a specific year in a single assertion. Both approaches serve different testing purposes and complement each other.

Applied to files:

  • tests/groups/test_tibetan.py
📚 Learning: 2025-07-24T15:21:31.632Z
Learnt from: PPsyrius
PR: vacanza/holidays#2750
File: tests/countries/test_germany.py:46-46
Timestamp: 2025-07-24T15:21:31.632Z
Learning: In the holidays project test files, the standard method name for testing the absence of holidays is `test_no_holidays`, not more descriptive names like `test_no_holidays_before_1990`. This is a consistent naming convention across country test files like France and Germany.

Applied to files:

  • tests/groups/test_tibetan.py
📚 Learning: 2025-07-02T18:21:59.302Z
Learnt from: KJhellico
PR: vacanza/holidays#2608
File: tests/countries/test_saint_vincent_and_the_grenadines.py:162-178
Timestamp: 2025-07-02T18:21:59.302Z
Learning: In the Saint Vincent and the Grenadines holiday tests, for holidays without observed rules that only require a single assertHolidayName call, pass the holiday name directly as a string literal rather than storing it in a variable first for cleaner, more concise code.

Applied to files:

  • tests/groups/test_tibetan.py
📚 Learning: 2025-04-02T18:38:35.164Z
Learnt from: KJhellico
PR: vacanza/holidays#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.

Applied to files:

  • tests/groups/test_tibetan.py
📚 Learning: 2025-08-11T10:14:28.499Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:328-343
Timestamp: 2025-08-11T10:14:28.499Z
Learning: For Ethiopian holidays in the `holidays/groups/christian.py` file, docstring wording should maintain source-accurate phrasing (e.g., "in coincidence of" for Ethiopian New Year/Enkutatash), even when it might read awkwardly in English, to ensure consistency with official Ethiopian documentation.

Applied to files:

  • tests/groups/test_tibetan.py
📚 Learning: 2025-04-03T16:58:27.175Z
Learnt from: Wasif-Shahzad
PR: vacanza/holidays#2409
File: holidays/countries/qatar.py:27-46
Timestamp: 2025-04-03T16:58:27.175Z
Learning: In the holidays library, method names like `_add_holiday_2nd_tue_of_feb()` and `_add_holiday_1st_sun_of_mar()` use calendar constants internally through parent class implementations even when these constants don't appear directly in the file. Removing imports that seem unused based on a simple text search could break functionality.

Applied to files:

  • tests/groups/test_tibetan.py
📚 Learning: 2025-06-14T20:43:15.370Z
Learnt from: KJhellico
PR: vacanza/holidays#2631
File: holidays/countries/sint_maarten.py:94-95
Timestamp: 2025-06-14T20:43:15.370Z
Learning: The `_add_*` helper methods in the holidays library (such as `_add_christmas_day_two()`, `_add_labor_day()`, etc.) don't have default holiday names, so the name parameter should always be explicitly specified when calling these methods.

Applied to files:

  • tests/groups/test_tibetan.py
📚 Learning: 2025-04-05T04:50:40.752Z
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:31-49
Timestamp: 2025-04-05T04:50:40.752Z
Learning: For Turkmenistan holiday tests, use this class structure: `class TestTurkmenistan(CommonCountryTests, TestCase)` with imports `from unittest import TestCase`, `from holidays.countries import Turkmenistan, TM, TKM`, and `from tests.common import CommonCountryTests`. Ensure to call `super().setUp()` in the setUp method.

Applied to files:

  • tests/groups/test_tibetan.py
📚 Learning: 2025-08-12T17:16:54.458Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: tests/calendars/test_julian.py:35-36
Timestamp: 2025-08-12T17:16:54.458Z
Learning: In the vacanza/holidays project calendar tests (Thai, Ethiopian, Julian, etc.), the established testing pattern for validation methods is to use simple for loops like `for year in known_data_dict:` followed by `self.assertEqual(expected, actual)` without using unittest's subTest feature. This pattern is consistently maintained across all calendar test files.

Applied to files:

  • tests/groups/test_tibetan.py
📚 Learning: 2025-04-05T04:29:38.042Z
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:85-86
Timestamp: 2025-04-05T04:29:38.042Z
Learning: For testing holiday implementations in the vacanza/holidays repository, recommend using `from tests.common import CommonCountryTests` as the base class instead of directly using `unittest.TestCase` to maintain consistency with project conventions and leverage common test utilities.

Applied to files:

  • tests/groups/test_tibetan.py
📚 Learning: 2025-05-06T21:07:11.577Z
Learnt from: KJhellico
PR: vacanza/holidays#2530
File: tests/countries/test_andorra.py:23-28
Timestamp: 2025-05-06T21:07:11.577Z
Learning: In the holidays project, test methods for country holidays follow a standard form where year ranges are explicitly recreated in each test method rather than being stored as class variables, to maintain consistency across all country tests.

Applied to files:

  • tests/groups/test_tibetan.py
📚 Learning: 2025-08-12T17:16:54.458Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: tests/calendars/test_julian.py:35-36
Timestamp: 2025-08-12T17:16:54.458Z
Learning: In the vacanza/holidays project, test methods that iterate over dictionaries of test data (like calendar drift tests) follow the established pattern without using unittest's subTest, as evidenced by the Thai calendar tests and maintained consistently across similar test structures.

Applied to files:

  • tests/groups/test_tibetan.py
📚 Learning: 2025-07-02T18:17:53.342Z
Learnt from: KJhellico
PR: vacanza/holidays#2608
File: tests/countries/test_saint_vincent_and_the_grenadines.py:162-178
Timestamp: 2025-07-02T18:17:53.342Z
Learning: In the Saint Vincent and the Grenadines holidays implementation, New Year's Day is added without observed rules using `_add_new_years_day()` and should not include observed rule testing in its test method. Only holidays explicitly wrapped with `_add_observed()` have observed behavior.

Applied to files:

  • tests/groups/test_tibetan.py
🧬 Code Graph Analysis (1)
tests/groups/test_tibetan.py (2)
holidays/groups/tibetan.py (12)
  • TibetanCalendarHolidays (20-153)
  • _add_blessed_rainy_day (42-48)
  • _add_birth_of_guru_rinpoche (50-56)
  • _add_buddha_first_sermon (58-64)
  • _add_buddha_parinirvana (66-72)
  • _add_day_of_offering (74-80)
  • _add_death_of_zhabdrung (82-88)
  • _add_descending_day_of_lord_buddha (90-96)
  • _add_losar (98-104)
  • _add_thimphu_drubchen_day (115-121)
  • _add_thimphu_tshechu_day (123-129)
  • _add_tibetan_winter_solstice (147-153)
tests/groups/test_international.py (1)
  • TestHolidays (21-24)
⏰ 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). (1)
  • GitHub Check: Test Python 3.14 on windows-latest
🔇 Additional comments (2)
holidays/groups/tibetan.py (1)

92-93: Docstring casing fix aligns naming; LGTM.

This matches test labels and keeps terminology consistent across the codebase.

tests/groups/test_tibetan.py (1)

22-26: Mixin init order matches established pattern; LGTM.

Calling TibetanCalendarHolidays.init before super().init mirrors other group tests (e.g., International).

Copy link
Collaborator

@PPsyrius PPsyrius 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 added this pull request to the merge queue Aug 13, 2025
Merged via the queue into vacanza:dev with commit 890f889 Aug 13, 2025
36 checks passed
@arkid15r arkid15r deleted the ark/restructure-groups-tests branch August 13, 2025 02:12
@arkid15r arkid15r mentioned this pull request Aug 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants