tests: remove redundant native ssh tests #3104
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
efiop
force-pushed
the
ssh_dir_out
branch
2 times, most recently
from
47c8a6d
to
5383af8
Compare
efiop
commented
Jan 10, 2020
| @@ -156,14 +156,12 @@ def _remove_file(self, path): | |||
| def _remove_dir(self, path): | |||
| for root, dirs, files in self.walk(path, topdown=False): | |||
| for fname in files: | |||
| path = posixpath.join(root, fname) | |||
There was a problem hiding this comment.
This was a pretty nasty bug 🙁 Added a test
efiop
commented
Jan 10, 2020
| url_info = URLInfo(remote_url) | ||
| mkdir_cmd = "mkdir dir-out;cd dir-out;echo 1 > 1.txt; echo 2 > 2.txt" |
There was a problem hiding this comment.
This actually didn't work because of the bug in remove. That is why you need to use && to ensure that no error occurs. Same complaint as when people don't use set -e \n set -x in their bash scripts 🙂
ghost
approved these changes
Jan 11, 2020
We already have mocked ones, which are more reliable as they use their own paramiko server instead of relying on the sshd running on the test machine. Native tests are also quite flaky and fail sometimes due to network issues.
efiop
force-pushed
the
ssh_dir_out
branch
3 times, most recently
from
85f09b7
to
b6955d7
Compare
ghost
self-requested a review
It is already available in the standard hooks.
|
@efiop , sorry for the late re-review, looks good, tho 😅 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We already have mocked ones, which are more reliable as they use their
own paramiko server instead of relying on the sshd running on the test
machine. Native tests are also quite flaky and fail sometimes due to
network issues.
❗ Have you followed the guidelines in the Contributing to DVC list?
📖 Check this box if this PR does not require documentation updates, or if it does and you have created a separate PR in dvc.org with such updates (or at least opened an issue about it in that repo). Please link below to your PR (or issue) in the dvc.org repo.
❌ Have you checked DeepSource, CodeClimate, and other sanity checks below? We consider their findings recommendatory and don't expect everything to be addressed. Please review them carefully and fix those that actually improve code or fix bugs.
Thank you for the contribution - we'll try to review it as soon as possible. 🙏