search

openmeteo / loggertodb

Insert meteorological station data to Enhydris
Other
1 stars 3 forks source link

Fix random filename with multi-file format (fixes #29) #46

Closed Shrhawk closed 1 year ago

aptiko commented 1 year ago

Hi! I think this is fine, and it's a good solution and well-written. Could you write a unit test for it? One that fails without the changes to the main code, but that succeeds with them.

Shrhawk commented 1 year ago

Hello @aptiko, Can you please review again. I have added the test case.

aptiko commented 1 year ago

I found out that the existing tests had started to fail after this fix, so I fixed them. I then decided that fixing the existing tests sufficiently tests for the new functionality and that no new testing was needed, so I removed your test case, which, by the way, shouldn't be in test_cli.py, this is supposed to only test stuff that is in cli.py. Your fix was in meteologgerstorage.py, and the tests for this are in tests/loggerstorage/, and this would need to be tested in test_multitextfileloggerstorage.py.

Merged in 9e071e6d438aff178c6a90b87980cc386901257e and subsequently the tests were fixed in 165123520964e196a23609d7453f795f3cb97aff.

BTW, could also please use your proper name in .gitconfig?