Fix/issue 440 create compreh tests#441
Conversation
|
I was going to suggest some more tests for the repo (after adding them for a PR using EntsoePandasClient over at OpenClimateFix), before a search brought me here - where I can see that the proposed suite of tests is already quite comprehensive. Therefore, @zljubisic I'd appreciate it if you could provide an update on the status of this PR, so I can open an Issue to add my tests if needed. Thanks. |
|
Hi @Shohail-Ismail. I have created these tests long time ago. At that time some other guy also created PR with some tests. |
|
Thanks @zljubisic, I've opened an Issue related to the specific tests I wanted to add - will let you know if there's anything further related to your tests. |
maurerle
left a comment
maurerle
left a comment
There was a problem hiding this comment.
Most of this is AI created garbage, so I would vote to just close this PR.
Mocking the output of a function and then checking if the mocked return value equals a return value does not get you far. The tests do not provide value.
- Add tests/conftest.py with XML builder helpers and Hypothesis strategies - Rewrite test_series_parsers.py: 24 tests covering resolution parsing, datetime index construction, position-to-timestamp mapping, A01/A03 curves - Rewrite test_parsers.py: 49 tests covering price/load/generation parsing, crossborder flows, net positions, unavailability, imbalance volumes - Rewrite test_mappings.py: 13 tests covering Area enum lookups, PSRTYPE/DOCSTATUS/BSNTYPE/NEIGHBOURS mapping validity - Rewrite test_misc.py: 15 tests covering time block contiguity, coverage, and timezone preservation - Rewrite test_decorators.py: 30 tests covering retry logic, year-limited and day-limited decorator behavior - Rewrite test_client.py: 19 tests covering datetime formatting, URL construction, and error response handling All 190 tests pass. Property-based tests use Hypothesis with max_examples=100. No mocks of return values - all tests feed real XML/data through the full parsing pipeline.
zljubisic
force-pushed
the
fix/issue-440-create-compreh-tests
branch
from
86ee5e6 to
23a0fc5
Compare
|
@maurerle I thought you will never show up. :) |
|
I find this PR very confusing. the amount of code is huge and it is unclear to me if any actual API calls are made? Reading parts of this it also seems that there are functions which do not do anything. They simply create a client and patch out the base request function but then dont do anything. What exactly does this test then? I am sorry but this is very unmaintainable for me if it is this verbose and unclear. I would like to have tests that are 1 per api call and makes very clear what it tests. it should not be this verbose. There is also a class called "Improved" instead of just replacing the code. this is very strange to me. Next to that the over verbose style of writing and amount of emojis indeed leads to me to believe this is AI slop. Please do not use AI to create this huge amount of slop as it makes it unworkable for others. if you want to continue on this please provide an explanation if you actually do an api call and how these tests work. please also cut down on the huge amount of code to something that is more simple and clear. thanks in advance! |
4 participants
I have added few tests to improve testing.
Feel free to add or change anything.
Closes: #440
@fboerman can you please check?