Skip to content

Conversation

@axunonb
Copy link
Collaborator

@axunonb axunonb commented Jun 6, 2025

  • Add checks for edge cases of periods to Period.CollidesWith
    • periods with Duration.Zero do not collide
    • periods with EffectiveDuration == null (only StartTime is set) throw as they are not considered as periods (should use Contains(CalDateTime) instead
    • contiguous periods are not evaluated as colliding any more
  • Remove redundant null check in Period.EffectiveEndTime
  • Enhance unit test coverage for Period and FreeBusy

Resolves #811

@codecov
Copy link

codecov bot commented Jun 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

❌ Your project status has failed because the head coverage (67%) is below the target coverage (80%). You can increase the head coverage or adjust the target coverage.

Impacted file tree graph

@@         Coverage Diff         @@
##           main   #812   +/-   ##
===================================
  Coverage    67%    67%           
===================================
  Files       106    106           
  Lines      4204   4211    +7     
  Branches    945    947    +2     
===================================
+ Hits       2822   2842   +20     
+ Misses     1048   1039    -9     
+ Partials    334    330    -4     
Files with missing lines Coverage Δ
Ical.Net/DataTypes/Period.cs 99% <100%> (+12%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@axunonb axunonb force-pushed the wip/axunonb/pr/period-collides-with branch from 34518e6 to 7e2b7f4 Compare June 6, 2025 14:41
* Add checks for edge cases of periodes
* Remove redundant null check in `Period.EffectiveEndTime`
* Enhance unit test coverage for `Period` and `FreeBusy`

Resolves #811
@axunonb axunonb force-pushed the wip/axunonb/pr/period-collides-with branch from 7e2b7f4 to 1db3c18 Compare June 6, 2025 14:59
@axunonb axunonb requested a review from minichma June 6, 2025 15:06
@axunonb axunonb marked this pull request as ready for review June 6, 2025 15:22
@sonarqubecloud
Copy link

@axunonb axunonb merged commit 682728d into main Jun 17, 2025
8 of 9 checks passed
@axunonb axunonb deleted the wip/axunonb/pr/period-collides-with branch June 17, 2025 07:13
@minichma
Copy link
Collaborator

@axunonb Sorry for not being able to review any earlier. Great increase of test coverage! The only detail to consider (not sure this is the case anyways) would be that Period instances that represent an RDATE of type DATE or DATE-TIME might not have a duration set. Not sure that could be a problem in the current implementation.

@axunonb
Copy link
Collaborator Author

axunonb commented Jun 18, 2025

Not sure that could be a problem in the current implementation.

In the FreeBusyStatus context I don't see a problem. With RDATE the method is not in use internally, but right, this could cause an issue.

Great increase of test coverage

Still too low over ical.net, unfortunately, putting any code changes at risk for unintentionally breaking things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

iCalendar semantics in Period.CollidesWith

3 participants