Skip to content
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

Tests for tasks and journals #98

Closed
wants to merge 5 commits into from
Closed

Conversation

tobixen
Copy link
Collaborator

@tobixen tobixen commented Nov 3, 2022

Test for #97 - some of those tests are as of now intentionally broken, but at least 2/3 of the remaining tests should pass once #97 is fixed.

(I'm a bit uncertain on the recurring task with DUE date set but no DTSTART - RRULE is linked to DTSTART according to the RFC, so perhaps that ical data should be considered invalid - though, in my head one should consider the missing DTSTART to be equal to the DUE if no DTSTART is given).

@niccokunzmann
Copy link
Owner

, in my head one should consider the missing DTSTART to be equal to the DUE if no DTSTART is given).

I think, I do the same with the events. They all have a DTEND. This module is about ease and consistency, so people do not need to worry much about the edge cases.

@@ -360,6 +360,10 @@ def __init__(self, calendar, keep_recurrence_attributes=False):
self.repetitions = []
for event in calendar.walk("VEVENT"):
self.repetitions.append(RepeatedEvent(event, keep_recurrence_attributes))
for event in calendar.walk("VTODO"):
self.repetitions.append(RepeatedTodo(event, keep_recurrence_attributes))
for event in calendar.walk("VJOURNAL"):
Copy link
Owner

Choose a reason for hiding this comment

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

Does walk(("VEVENT", "VTODO")) work? - not a blocker for merge

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh, did I really commit this? It's not part of the "broken tests" and it's anyway broken. I'll move it over to a separate pull request.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Does walk(("VEVENT", "VTODO")) work? - not a blocker for merge

Apparently not:

(Pdb) self.walk(('VEVENT', 'VTODO'))
*** AttributeError: 'tuple' object has no attribute 'upper'

@niccokunzmann
Copy link
Owner

Can this be closed now? What do you think?

@tobixen
Copy link
Collaborator Author

tobixen commented Nov 16, 2022

Probably. Would be nice with more test code. At least, I'd like to run the caldav test suite again to see if anything is amiss.

@tobixen

This comment was marked as outdated.

@tobixen
Copy link
Collaborator Author

tobixen commented Nov 16, 2022

(nevermind my previous message, that was a bug at my side)

@tobixen
Copy link
Collaborator Author

tobixen commented Nov 16, 2022

The tests at my side still doesn't pass (I thought I had checked that ... but perhaps not after the last changes).

I need to sleep. Hope to get time to look more into it tomorrow.

@niccokunzmann
Copy link
Owner

niccokunzmann commented Nov 16, 2022

Yes. Sleep helps :) It is nice it we can reproduce the behavior here. If you like, you can open issues whenever you encounter something. I will not work now because I am too tired.. 😴

@tobixen
Copy link
Collaborator Author

tobixen commented Nov 17, 2022

It helped sleeping. For one thing, this is a pull request which was fully included in pull request #100, so of course it can be closed. For the second thing, I managed to get all tests passing at my side.

@tobixen tobixen closed this Nov 17, 2022
@niccokunzmann
Copy link
Owner

niccokunzmann commented Nov 18, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants