tests: use dir_helpers on repo #2950
Conversation
tests/unit/test_repo.py
Outdated
| def test_return_true_on_dvc_internal_file(self): | ||
| path = os.path.join("path", "to", ".dvc", "file") | ||
| self.assertTrue(path) | ||
| def test_destroy(tmp_dir, dvc): |
There was a problem hiding this comment.
moved .destroy, .collect, .status to the unit test module. the reason behind it is that we are testing the behavior of a single unit, in this case, a single method.
There was a problem hiding this comment.
Still look like functional tests to me. We recreate the whole scenario and call a method, which is a mirror of cli command.
There was a problem hiding this comment.
@Suor , for future references, how do you make the distinction between functional and unit tests?
ghost
requested review from
Suor,
efiop and
pared
and removed request for
Suor
|
@MrOutis Please check tests, they are failing. |
|
@efiop , thanks for the notice |
|
@MrOutis tests are still failing |
tests/unit/test_repo.py
Outdated
| def test_return_true_on_dvc_internal_file(self): | ||
| path = os.path.join("path", "to", ".dvc", "file") | ||
| self.assertTrue(path) | ||
| def test_destroy(tmp_dir, dvc): |
There was a problem hiding this comment.
Still look like functional tests to me. We recreate the whole scenario and call a method, which is a mirror of cli command.
|
|
||
| def test_destroy(tmp_dir, dvc): | ||
| dvc.config.set("cache", "type", "symlink") | ||
|
|
There was a problem hiding this comment.
dvc.cache = Cache(dvc)
setting cache type does not reload it, and last check is always true
tests/func/test_repo.py
Outdated
|
|
||
| self.create(DvcIgnore.DVCIGNORE_FILE, self.DATA_DIR) | ||
| assert stages() == { | ||
| os.path.join("file.dvc"), |
There was a problem hiding this comment.
is there need for os.path.join here?
There was a problem hiding this comment.
no need, but just to keep it consistent 😅
| run_copy("bar", "buzz") | ||
| assert collect_outs("buzz.dvc", with_deps=True) == {"foo", "bar", "buzz"} | ||
| assert collect_outs("buzz.dvc", with_deps=False) == {"buzz"} |
There was a problem hiding this comment.
The "buzz" was added in a branch originally. And it was a collect with a branch assigned.
There was a problem hiding this comment.
I think it doesn't make sense to do the branching, but I'll do it just to not be a blocker for this PR
tests/func/test_repo.py
Outdated
| self.assertEqual(0, ret) | ||
| def test_stages(tmp_dir, dvc): | ||
| def stages(): | ||
| return set(stage.relpath for stage in Repo(str(tmp_dir)).stages) |
tests/unit/test_repo.py
Outdated
| self.assertTrue(path) | ||
| def test_is_dvc_internal(tmp_dir, dvc): | ||
| assert dvc.is_dvc_internal(fspath(tmp_dir / ".dvc" / "file")) | ||
| assert not dvc.is_dvc_internal("file") |
There was a problem hiding this comment.
The test was quite different before:
- testing relative paths always
- testing
.dvcnot in the repo root (still considered internal, don't know why) - testing tricky dirname "to-non-.dvc" (not sure if that is important)
There was a problem hiding this comment.
@pared , where those intended? I'll leave them as it was before for now
efiop
added
refactoring
3 participants
No description provided.