tests: use dir helpers on updater #2952
Conversation
ghost
requested review from
| ("0.20.8", "0.19.0", True), | ||
| ], | ||
| ) | ||
| def test_is_outdated(latest, current, result, updater): |
There was a problem hiding this comment.
move .is_outdated and .check to the unit tests, since they are testing a single unit, in this case, a method.
| updater.check() | ||
| updater.check() | ||
| updater.check() |
There was a problem hiding this comment.
Why do we do this 3 times? It would be nice to have a comment explaining this.
There was a problem hiding this comment.
indeed, I imagine that is to "test" the lock mechanism or something. @efiop , could you give more details about this?
There was a problem hiding this comment.
IIRC, to check that we hit the logic when there is no updater file, when there is one and once more. It is pretty old code, so sorry for it being weird like that. 🙂
There was a problem hiding this comment.
So, if all 3 .check() calls are noop then test will pass.
tests/unit/test_updater.py
Outdated
| if not os.getenv("TRAVIS_EVENT_TYPE") == "cron": | ||
| return | ||
|
|
||
| monkeypatch.delenv("CI") |
There was a problem hiding this comment.
This is not enough to make .check() run. There is also a DVC_TEST env var. Also, the fact that it ran smooth means that we are not checking anything here. Add a spy probably.
|
@MrOutis Test failing. Please take a look, |
tests/unit/test_updater.py
Outdated
| os.getenv("TRAVIS_EVENT_TYPE") != "cron", | ||
| reason = "Only run on travis CRON to avoid generating too much logs" | ||
| def test_check(updater, monkeypatch): | ||
| monkeypatch.setenv("DVC_TEST", "true") |
There was a problem hiding this comment.
There is also a DVC_TEST env var
Then I didn't get this comment, @Suor
There was a problem hiding this comment.
You are setting it to true, but it is already true. Basically test is noop since I changed Updater to not run in tests several months ago.
To fix this properly you need:
- add some mock/spy to check that
updater.check()is actually doing something - fix it not doing something by adjusting env
efiop
added
refactoring
3 participants
No description provided.