Add coverage tests for issue 698 - #1629
Conversation
AI disclosure: I used OpenAI Codex (GPT-5.6) to identify uncovered code paths, explain the relevant project code, review the user-authored tests, and verify formatting, tests, and coverage.
|
This pull request did not pass quality checks and AI use is suspected. Please review Contribute and make any necessary amendments. |
|
Profile summary: Full profile |
|
This pull request did not pass quality checks and AI use is suspected. Please review Contribute and make any necessary amendments. |
Documentation build overview
45 files changed ·
|
|
Hello, thank you for reviewing my first PR. |
stevepiercy
left a comment
There was a problem hiding this comment.
Thank you for your first contribution! The work LGTM.
Let's get a technical review. There may be a preferred way to organize the tests in existing test files or parametrize them.
SashankBhamidi
left a comment
There was a problem hiding this comment.
Thanks for the contribution, and welcome. I ran all five tests and they pass.
One thing to address inline.
There was a problem hiding this comment.
The tests should be split into their natural homes rather than collected in a new file.
test_vunknown_reprbelongs insrc/icalendar/tests/test_issue_1445.py, that's where allvUnknowntests live.test_vcategory_hashandtest_vcategory_reprbelong insrc/icalendar/tests/prop/test_unit.pyalongside the existingvCategorytests at line 335.test_vweekday_rejects_invalid_jcal_valuebelongs insrc/icalendar/tests/prop/test_vWeekday.py.test_vdate_rejects_invalid_calendar_datebelongs insrc/icalendar/tests/prop/test_date_and_time.py.
Once you've moved them, this new file can be deleted.
There was a problem hiding this comment.
Thank you very much for the detailed guidance. I have moved each test to the existing test file you suggested and removed src/icalendar/tests/prop/test_issue_698_more_coverage.py. I also ran the five affected tests and the Ruff lint and formatting checks, and they pass. I appreciate your help in showing me the project's preferred test organization.
AI disclosure: I used OpenAI Codex (GPT-5.6) to apply the requested test organization changes, review the diff, and verify formatting and the five affected tests.
niccokunzmann
left a comment
There was a problem hiding this comment.
Thanks for your PR. I will wait for Sashank to have a look again.
Linked issue
Description
Adds focused tests for five uncovered paths involving unknown-property representation, category hashing and representation, invalid calendar dates, and invalid jCal weekday values. No production code is changed.
I used OpenAI Codex (GPT-5.6) to help identify uncovered paths, understand the relevant code, review the tests, and verify formatting, tests, and coverage.
Checklist