Test Restructuring

[niccokunzmann]

Hi,

I had a look at the tests and I would like to discuss some restructuring - just discuss it for now - before I make changes.

If you look at
https://github.com/niccokunzmann/python-recurring-ical-events/tree/master/test
you see that

• there is a folder for all ICS files.
• these can be accessed quite conveniently as a fixture argument from pytest
• I encourage people to give the ICS files to the problems that they face - TDD

And, allowing parametrization, I run over 600 test cases both testing pytz and zoneinfo in the same tests - this is something for the transition.

So, I wonder what your input would be about restructuring the tests going forward. I am a fan of TDD and I will have a look but I also do not know how much time I have when...

My intention is to let you know about my wishes for icalendar and what I can offer, related to #360.

[mauritsvanrees]

These ideas sound good to me.

I am not always a fan of TDD, but it can be very useful. Certainly adding a test case that fails with current code and passes with new code in a PR, probing that a specific problem is indeed fixed, is very useful. It prevents regressions from happening in the future.

[jacadzaca]

@niccokunzmann I have some spare time, can I restructure the test according to what you've outlined?

[niccokunzmann]

Yes, let's keep it a conversation!

[jacadzaca]

@niccokunzmann I did some refactoring today, I'm by no means done, but I think I should consult whether I'm doing it long the lines of what you've in mind. See this branch for changes I've made

I also identified some tests that I think are redundant etc. here's the list:

Duplicates

• test_issue_53 (linked discussion says that test_case_meetup is enough?)
• test_encodes_unicode (test_encoding.py seems to provide sufficient coverage?)
• test_issue_101 (same as test_encodes_unicode?)
• test_issue_237 (same as test_encodes_unicode?)
• test_issue_345 (seems better than test_unit_tools.py, so we should replace)

Test that I don't know what they check

• test_issue_70_decode_rrule_attribute_error (no clue what does the test check)
• test_issue_82_encodes_base64
• test_issue_335_identify_UTC fails on my machine with:
  AssertionError: assert 'DTSTART;VALUE=DATE-TIME:20211117T150915Z' in 'BEGIN:VEVENT\r\nDTSTART;VALUE=DATE-TIME:20211117T140915Z\r\nEND:VEVENT\r\n'
  wrong timezones there?

Test that seem to only serve as documentation/examples

• test_issue_116

Test that seem to check if the parser works (I left these in the test_fixed_issues.py

• test_issue_100
• test_issue_142
• test_issue_143
• test_issue_157
• test_issue_168
• test_issue_178 (do we even want to support custom components??? RFC states that there can be no custom copoments I think)
• test_issue_184

Next, I plan to create a 'big' parameterized test for issues in the Test that seem to check if the parser works section, do you think that's a good idea?

[niccokunzmann]

I would say one pr for removing duplicates or better one for each duplicate. It is good to test documentation and that examples work. The objective of this is for me to - get to know the tests - not reduce what they test in code AND referenced understanding - make one good example to advertise - create a how to test the code documentation What is nice for me is to split them up so that one test has only one assertion, using pytest fixtures and functions and alike. But that might be interesting to approach.

Test that seem to check if the parser works

Could be in a parsing.py file. Important for me is that the references to issues do not get removed. So, yes, I like it. Let's make small PRs where possible.