Write test for get_inode#2673
Conversation
|
@efiop I thought I would delete https://github.com/iterative/dvc/blob/1baf020de7c3b0024807f30bc7e9eec39a37948c/dvc/state.py#L373 because we were replacing |
|
@efiop The |
There was a problem hiding this comment.
You are not mocking anything in this test
| # Mocking a file to get the inode. |
There was a problem hiding this comment.
@efiop I am creating a file FOO so that I can mock a path. Isn't this called mocking?
There was a problem hiding this comment.
Mocking is when you simulate something, and here you are testing on a real file, so I wouldn't call that mocking.
There was a problem hiding this comment.
@efiop Oh okay. But how does it delete the file later?
There was a problem hiding this comment.
@algomaster99 Sorry, missed this. Yes, it deletes it. See dvc_repo fixture in tests/conftest.py.
Yes, it should accept both str and path_info too, but let's fix it in a separate PR 🙂
Got it. Yes, this PR is mergable by itself so let's finish this and proceed with |
efiop
left a comment
There was a problem hiding this comment.
Looks good! Let's remove that comment and we will be ready to merge 🙂
Have you followed the guidelines in our
Contributing document?
Does your PR affect documented changes or does it add new functionality
that should be documented? If yes, have you created a PR for
dvc.org documenting it or at
least opened an issue for it? If so, please add a link to it.
With reference to #2137
Testing if
get_inodecan take both - Path-like objects and strings.