Refactor test_encoding create_from_ical #438
Conversation
|
Merging #439 first may lead to less merge conflicts |
src/icalendar/tests/test_encoding.py
Outdated
| ('issue_64_event_with_non_unicode_summary', 'SUMMARY', 'åäö'), | ||
| # Unicode characters in summary | ||
| ('issue_64_event_with_unicode_summary', 'SUMMARY', 'abcdef'), |
There was a problem hiding this comment.
I noticed that src/icalendar/tests/events/issue_64_event_with_non_unicode_summary.ics contains (as the name suggest) not unicode characters but this tests for 'åäö' and then the unicode test only checks ascii characters. This should be the other way around? Why are the tests not failing?
There was a problem hiding this comment.
I think by non-unicode I've meant non-ascii, so I'll fix that.
There was a problem hiding this comment.
The test where actually wrong. issue_64_event_with_non_ascii_summary.ics was called issue_64_event_with_unicode_summary.ics and contained
BEGIN:VEVENT
SUMMARY:åäö
END:VEVENT
and originally the test was ('issue_64_event_with_unicode_summary', 'SUMMARY', 'abcdef'),, where summary clearly didn't match. I looked into the action log to figure out, why the action wasn't failing. After pulling the code myself to debug it, I noticed that they where indeed not equal, but you forgot to assert the equality.
Can you please add the assert for this test.
src/icalendar/tests/test_encoding.py
Outdated
|
|
||
| def test_event_from_ical_respects_unicode(test_input, field, expected_value, events): | ||
| event = events[test_input] | ||
| event[field].to_ical().decode('utf-8') == expected_value |
jacadzaca
force-pushed
the
test_restructure_test_encoding_create_from_ical
branch
from
916464e
to
33c380e
Compare
|
Thanks for this one @jacadzaca and @angatha for the review! It is nice to see that you are bringing the project forward without the maintainers being needed and I liked seeing the review process - that builds trust in me that the changes have a conversation and more eyes looking at them :) |
Restructure
create_from_icalfromtest_encodingalong the lines of #374