-
-
Notifications
You must be signed in to change notification settings - Fork 561
Split tests/test_holiday_groups.py #2803
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Summary by CodeRabbit
WalkthroughReorganized group-oriented tests: added a package header to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
Codecov Report✅ All modified and coverable lines are covered by tests. 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. 🚀 New features to boost your workflow:
|
There was a problem hiding this 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
📒 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.pytests/groups/test_international.pytests/groups/test_balinese_saka.pytests/groups/test_mandaean.pytests/groups/test_tibetan.pytests/groups/test_persian.pytests/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.pytests/groups/test_international.pytests/groups/test_balinese_saka.pytests/groups/test_mandaean.pytests/groups/test_tibetan.pytests/groups/test_persian.pytests/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.pytests/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.pytests/groups/test_international.pytests/groups/test_balinese_saka.pytests/groups/test_mandaean.pytests/groups/test_tibetan.pytests/groups/test_persian.pytests/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.pytests/groups/test_international.pytests/groups/test_balinese_saka.pytests/groups/test_mandaean.pytests/groups/test_tibetan.pytests/groups/test_persian.pytests/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.pytests/groups/test_balinese_saka.pytests/groups/test_mandaean.pytests/groups/test_tibetan.pytests/groups/test_persian.pytests/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.pytests/groups/test_balinese_saka.pytests/groups/test_tibetan.pytests/groups/test_persian.pytests/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.pytests/groups/test_mandaean.pytests/groups/test_tibetan.pytests/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.pytests/groups/test_mandaean.pytests/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.pytests/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.pyAll 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 harnessI didn’t find any
start_year/end_yearinTibetanCalendarHolidaysor_TibetanLunisolar, so the test runs without a fixed horizon. Other calendar group tests defineend_year(e.g., Thai 2158, Persian 2102) to lock in their supported range and avoid flakiness.• File: tests/groups/test_tibetan.py
• Location: theclass TestHolidays(HolidayBase, TibetanCalendarHolidays)block
• Action: addend_year = <MAX_SUPPORTED_YEAR>after the class declaration.
Please confirm the intended max year supported by
_TibetanLunisolarand 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_CALENDARis defined inholidays/calendars/thai.pyand re-exported inholidays/calendars/__init__.py.- All
_add_*helper methods listed intests/groups/test_thai.pyexist inholidays/groups/thai.py.- Both
_add_makha_bucha(self, name, calendar=None)and_add_visakha_bucha(self, name, calendar=None)accept an optionalcalendarargument.No further changes needed here.
|
There was a problem hiding this 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
📒 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).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🛠️
Proposed change
No actual test logic change is intended -- just their structure.
Type of change
holidaysfunctionality in general)Checklist
make checklocally; all checks and tests passed.