+
Skip to content

Fix iCal exporter tests #2689

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 30, 2025
Merged

Fix iCal exporter tests #2689

merged 1 commit into from
Jun 30, 2025

Conversation

KJhellico
Copy link
Collaborator

@KJhellico KJhellico commented Jun 30, 2025 •
edited
Loading

Proposed change

Fix iCal exporter tests (1, 2).
Libya PR branch - fail,
Libya PR branch + this fix - success.

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 coderabbitai
Copy link
Contributor

coderabbitai bot commented Jun 30, 2025 •
edited
Loading

Summary by CodeRabbit

  • Tests
    • Updated tests to explicitly specify language settings for holiday objects, improving accuracy of localized holiday name handling.

Walkthrough

The test suite was updated to explicitly set language codes when creating holiday objects for Thailand, the United States, and Ukraine. This change ensures that localization is consistently applied during test execution.

Changes

Files Change Summary
tests/test_ical.py Added explicit language codes ("th", "en_US", "uk") when instantiating holiday objects in setup and a specific test.

Suggested labels

test

📜 Recent review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 7d11a21 and a059c85.

📒 Files selected for processing (1)
  • tests/test_ical.py (2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings 
Learnt from: KJhellico
PR: vacanza/holidays#2631
File: tests/countries/test_sint_maarten.py:62-0
Timestamp: 2025-06-14T21:12:07.224Z
Learning: KJhellico prefers to focus on completing and reviewing the main holiday implementation code before doing detailed reviews of the corresponding test files.

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.

Learnt from: PPsyrius
PR: vacanza/holidays#2362
File: tests/test_ical.py:31-40
Timestamp: 2025-03-23T16:59:25.794Z
Learning: In the holidays library's `TestIcalExporter` class, initializing `ICalExporter` with `None` is intentional. Using a mock object instead breaks tests because the constructor performs attribute validation with `getattr(self.holidays, "language", None)` which returns a mock with a mock object instead of `None`, causing validation failures. The approach isolates file operation tests from iCal generation logic.

Learnt from: PPsyrius
PR: vacanza/holidays#2608
File: holidays/countries/saint_vincent_and_the_grenadines.py:30-30
Timestamp: 2025-06-07T07:37:55.516Z
Learning: For the vacanza/holidays repository, defer to the ruff configuration and pre-commit checks for coding standards rather than making manual suggestions about line length or style issues. If the automated checks pass, the code follows the project's standards.

Learnt from: PPsyrius
PR: vacanza/holidays#2362
File: holidays/ical.py:46-67
Timestamp: 2025-03-23T10:44:39.256Z
Learning: The ICalExporter class in the holidays library intentionally restricts language codes to two-letter ISO 639-1 codes as required by iCalendar specifications. This validation is comprehensively tested in test_language_code.

Learnt from: KJhellico
PR: vacanza/holidays#2614
File: README.md:108-108
Timestamp: 2025-06-14T20:13:29.536Z
Learning: In the vacanza/holidays repository, the team prefers to manually update the country count in README.md when adding new countries, with tests in place to verify the count stays accurate. They are satisfied with this approach and do not want it automated.

Learnt from: PPsyrius
PR: vacanza/holidays#2362
File: tests/test_ical.py:31-40
Timestamp: 2025-03-23T16:59:25.794Z
Learning: In the holidays library tests, `TestIcalExporter` initializes `self.exporter = ICalExporter(None)` intentionally and then mocks the `generate` method, which is a valid test pattern for isolating file operation tests from the actual iCal generation logic. Replacing `None` with a mock object breaks the tests.

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.

Learnt from: PPsyrius
PR: vacanza/holidays#2642
File: tests/countries/test_france.py:24-27
Timestamp: 2025-06-18T10:10:46.158Z
Learning: The holidays library prioritizes full test coverage over performance in test suites. Modern library-wide standards favor comprehensive testing across all subdivisions and wide year ranges (e.g., 1803-2050 for France) to ensure thorough validation, even if it creates many test objects.

Learnt from: PPsyrius
PR: vacanza/holidays#2362
File: holidays/ical.py:53-80
Timestamp: 2025-03-26T08:55:08.917Z
Learning: The holidays library intentionally limits language code support to ISO 639-1 and ISO 639-2 standards (2-3 letter language codes with optional region) to maintain consistency with other modules like `common_holidays` and `financial_holidays`, rather than implementing full RFC 5646 compliance.

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.

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.

Learnt from: PPsyrius
PR: vacanza/holidays#2489
File: holidays/countries/sao_tome_and_principe.py:22-26
Timestamp: 2025-04-23T14:55:35.504Z
Learning: References in holidays classes should only be included if they're used for test case cross-checks or provide historical context about when holidays were added/removed, not just for the sake of having more references.

Learnt from: PPsyrius
PR: vacanza/holidays#2489
File: holidays/countries/sao_tome_and_principe.py:22-26
Timestamp: 2025-04-23T14:55:35.504Z
Learning: References in holidays classes should only be included if they're used for test case cross-checks or provide historical context about when holidays were added/removed, not just for the sake of having more references.

Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:52-64
Timestamp: 2025-04-05T04:33:53.254Z
Learning: For Turkmenistan holiday tests, recommend using CommonCountryTests as the base class rather than unittest.TestCase to follow project conventions, be consistent with other country test files, and gain access to common test utilities.

Learnt from: KJhellico
PR: vacanza/holidays#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.
tests/test_ical.py (20) 
Learnt from: PPsyrius
PR: vacanza/holidays#2362
File: tests/test_ical.py:31-40
Timestamp: 2025-03-23T16:59:25.794Z
Learning: In the holidays library's `TestIcalExporter` class, initializing `ICalExporter` with `None` is intentional. Using a mock object instead breaks tests because the constructor performs attribute validation with `getattr(self.holidays, "language", None)` which returns a mock with a mock object instead of `None`, causing validation failures. The approach isolates file operation tests from iCal generation logic.

Learnt from: PPsyrius
PR: vacanza/holidays#2362
File: holidays/ical.py:46-67
Timestamp: 2025-03-23T10:44:39.256Z
Learning: The ICalExporter class in the holidays library intentionally restricts language codes to two-letter ISO 639-1 codes as required by iCalendar specifications. This validation is comprehensively tested in test_language_code.

Learnt from: PPsyrius
PR: vacanza/holidays#2362
File: tests/test_ical.py:54-75
Timestamp: 2025-03-22T09:31:16.025Z
Learning: The ICalExporter class in holidays/ical.py already handles empty language strings, likely treating them similar to None values which default to "EN".

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.

Learnt from: PPsyrius
PR: vacanza/holidays#2362
File: tests/test_ical.py:31-40
Timestamp: 2025-03-23T16:59:25.794Z
Learning: In the holidays library tests, `TestIcalExporter` initializes `self.exporter = ICalExporter(None)` intentionally and then mocks the `generate` method, which is a valid test pattern for isolating file operation tests from the actual iCal generation logic. Replacing `None` with a mock object breaks the tests.

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.

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#2362
File: holidays/ical.py:27-52
Timestamp: 2025-03-27T04:46:17.044Z
Learning: In the ICalExporter class, the language tag (`;LANGUAGE=`) should only be included in the SUMMARY field when explicitly requested (`show_language=True`) and a valid language is available. The default behavior (`show_language=False`) doesn't include the language tag.

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.

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.

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.

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.

Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:52-64
Timestamp: 2025-04-05T04:33:53.254Z
Learning: For Turkmenistan holiday tests, recommend using CommonCountryTests as the base class rather than unittest.TestCase to follow project conventions, be consistent with other country test files, and gain access to common test utilities.

Learnt from: PPsyrius
PR: vacanza/holidays#2601
File: tests/countries/test_mongolia.py:128-156
Timestamp: 2025-06-15T11:52:39.572Z
Learning: In the vacanza/holidays project tests, when testing holidays that span multiple consecutive days across many years (like Mongolia's National Festival spanning July 11-13), prefer explicit for loops over complex nested generator expressions with unpacking. The explicit loops are more readable, easier to maintain, and better communicate the testing intent even though the Big O complexity is equivalent.

Learnt from: KJhellico
PR: vacanza/holidays#2386
File: holidays/countries/nepal.py:203-208
Timestamp: 2025-05-01T19:13:44.664Z
Learning: Multiple holidays on the same date are properly handled in the holidays library. When multiple `_add_...` methods are called with different holiday names for the same date, both holidays are preserved rather than the latter overwriting the former. This happens because the `__setitem__` method in `HolidayBase` merges multiple holidays on the same date by combining their names.

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

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.

Learnt from: PPsyrius
PR: vacanza/holidays#2629
File: tests/countries/test_namibia.py:22-23
Timestamp: 2025-06-14T10:58:43.636Z
Learning: In the vacanza/holidays project, country test files consistently use range(start_year, 2050) which intentionally excludes 2050 and stops at 2049. This is a library-wide implementation pattern, not an off-by-one error.

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

Learnt from: KJhellico
PR: vacanza/holidays#2601
File: README.md:108-108
Timestamp: 2025-06-09T19:39:38.818Z
Learning: The correct way to count supported countries in the holidays library is by counting unique country module files in holidays/countries/ (excluding __init__.py) or by counting entries in the COUNTRIES registry, not by counting individual class definitions which include alias classes.
🧬 Code Graph Analysis (1)
tests/test_ical.py (1)
holidays/utils.py (1)
  • country_holidays (32-228)
⏰ Context from checks skipped due to timeout of 300000ms (8)
  • GitHub Check: Test Python 3.13 on windows-latest
  • GitHub Check: Test Python 3.12 on windows-latest
  • GitHub Check: Test Python 3.10 on windows-latest
  • GitHub Check: Test Python 3.13 on ubuntu-latest
  • GitHub Check: Test Python 3.9 on windows-latest
  • GitHub Check: Test Python 3.11 on windows-latest
  • GitHub Check: Test Python 3.10 on ubuntu-latest
  • GitHub Check: CodeQL
🔇 Additional comments (2)
tests/test_ical.py (2)

48-49: Good consistency fix.

Adding explicit language parameters aligns these holiday objects with the existing pattern and ensures predictable localization behavior across all test fixtures.

253-253: Test reliability improvement.

Explicit language specification prevents environment-dependent behavior and ensures the Ukrainian holiday test runs consistently across different systems.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate Unit Tests
  • Create PR with Unit Tests
  • Post Copyable Unit Tests in 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.
    • Explain this complex logic.
    • 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. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • 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 src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

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

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai auto-generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

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.

Documentation and Community

  • 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 Jun 30, 2025
@sonarqubecloud SonarQubeCloud
Copy link

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

@codecov Codecov
Copy link

codecov bot commented Jun 30, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (7d11a21) to head (a059c85).
Report is 1 commits behind head on dev.

Additional details and impacted files 
@@            Coverage Diff            @@
##               dev     #2689   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          253       253           
  Lines        15201     15201           
  Branches      2095      2095           
=========================================
  Hits         15201     15201

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

PPsyrius
PPsyrius approved these changes Jun 30, 2025
View reviewed changes
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 reviewed Jun 30, 2025
View reviewed changes
Copy link
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 added this pull request to the merge queue Jun 30, 2025
Merged via the queue into vacanza:dev with commit 5176c05 Jun 30, 2025
33 checks passed
@KJhellico KJhellico deleted the fix-tests branch June 30, 2025 16:40
@KJhellico KJhellico mentioned this pull request Jul 7, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@arkid15r arkid15r arkid15r left review comments

@PPsyrius PPsyrius PPsyrius approved these changes

Assignees

No one assigned

Labels

test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@KJhellico @arkid15r @PPsyrius
点击 这是indexloc提供的php浏览器服务，不要输入任何密码和下载