Add test for ssh connection

[ag613915cao]

This commit intends to mock SSH server that not depends on
OS which the test running on for testing purposes.

Related #1704

[efiop]

I'm probably missing something, but are you sure your _client() is indeed using mockssh's client? It looks to me that remote.ssh that you are using is trying to connect to the machine itself and not to the mockssh's server.

[efiop]

Also, by the looks of it, you've probably wanted to use pytest's fixtures here, right? 🙂 If so, please feel free to do so, we are simply in the middle of migrating from unittest to pytest, so we have a legacy unittest tests and we tend to write new tests with pytest.

[efiop]

Ah, right. Indeed, the port is not 22. Thanks! 🙂

Let's use fixtures for your tests, since it is the most natural approach. Btw, if you are testing SSHConnection itself, let's move your tests to tests/unit/remote/ssh/test_connection.py.

[ag613915cao]

Sure. I will do it ;)

[ag613915cao]

@efiop I updated the point as your expectation

[ag613915cao]

@efiop BTW, Do you know why the CI failed? And give me some hints about the detailed source of it (I clicked on the Details but no useful info to move forward)?

Updated: Nevermind, I see the log already

[efiop]

@ag613915cao Please checkout this guide https://dvc.org/doc/user-guide/contributing . Your tests on travis fail because it your code is not formatted with black.

[efiop]

Your appveyor test fails:

_____________________ TestRemoteSSHConnection.test_isdir ______________________
[gw3] win32 -- Python 2.7.16 c:\python27\python.exe
self = <tests.unit.remote.ssh.test_ssh.TestRemoteSSHConnection testMethod=test_isdir>
    def test_isdir(self):
        client = self._client()
        path = '/root'
        isdir = client.isdir(path=path)
>       self.assertTrue(isdir)
E       AssertionError: False is not true
tests\unit\remote\ssh\test_ssh.py:113: AssertionError
---------------------------- Captured stdout call -----------------------------
DEBUG: Establishing ssh connection with '127.0.0.1' through port '1198' as user 'user'
------------------------------ Captured log call ------------------------------
__init__.py                 70 DEBUG    Establishing ssh connection with '127.0.0.1' through port '1198' as user 'user'
---------- coverage: platform win32, python 2.7.16-final-0 -----------

because /root does not exist on windows 🙂

[efiop]

@ag613915cao Still not formatted by black. Try running checks manually

black dvc
black tests
flake8 dvc
flake8 tests

[ag613915cao]

@ag613915cao Still not formatted by black. Try running checks manually

Updated: All the gates are GREEN now :)

[efiop]

@ag613915cao Btw, your PR doesn't fix #1704 . You are adding new unit tests that use mockssh and the ticket is about enabling func tests that we already have(e.g. tests/func/test_data_sync.py) on windows.

[efiop]

You are testing SSHConnection here, so you don't have to(and probably shouldn't) instantiate it through RemoteSSH, since you could instantiate SSHConnection directly.

[ag613915cao]

I updated the point. I'm new our community and need a little bit of time to fully understand source code base

[efiop]

It is not a good idea to use __name__ for your regular variables.

[ag613915cao]

Yup :)

[ag613915cao]

@ag613915cao Btw, your PR doesn't fix #1704 . You are adding new unit tests that use mockssh and the ticket is about enabling func tests that we already have(e.g. tests/func/test_data_sync.py) on windows.

Right. the ticket is intended to enable test ssh on windows and setup env on appveyor. I see appveyor is already set up. So, the ticket should be closed?

[efiop]

@ag613915cao Not really. SSH test is not enabled on windows: https://github.com/iterative/dvc/blob/master/tests/func/test_data_cloud.py#L104 .

[ag613915cao]

@ag613915cao Not really. SSH test is not enabled on windows: https://github.com/iterative/dvc/blob/master/tests/func/test_data_cloud.py#L104 .

I see. So, you want me to solve it in this commit?

[efiop]

@ag613915cao Let's do that in a separate PR 🙂

[efiop]

@ag613915cao Thank you! 🎉

[ag613915cao]

@ag613915cao Not really. SSH test is not enabled on windows: https://github.com/iterative/dvc/blob/master/tests/func/test_data_cloud.py#L104 .

I pushed new one for this point: #1911