test: http: use random port (#2269)

Fixes #1878

Signed-off-by: Ruslan Kuprieiev <[email protected]>

tests/func/test_import_url.py

@@ -44,12 +44,8 @@ class TestDefaultOutput(TestDvc):
 
 
 class TestInterruptedDownload(TestDvc):
-    @property
-    def remote(self):
-        return "http://localhost:8000/"
-
-    def _prepare_interrupted_download(self):
-        import_url = urljoin(self.remote, self.FOO)
+    def _prepare_interrupted_download(self, port):
+        import_url = urljoin("http://localhost:{}/".format(port), self.FOO)
         import_output = "imported_file"
         tmp_file_name = import_output + ".part"
         tmp_file_path = os.path.realpath(
@@ -79,18 +75,16 @@ class TestInterruptedDownload(TestDvc):
 class TestShouldResumeDownload(TestInterruptedDownload):
     @patch("dvc.remote.http.RemoteHTTP.CHUNK_SIZE", 1)
     def test(self):
-        with StaticFileServer():
-            import_output, import_url, tmp_file_path = (
-                self._prepare_interrupted_download()
+        with StaticFileServer() as httpd:
+            output, url, file_path = self._prepare_interrupted_download(
+                httpd.server_port
             )
 
             m = mock_open()
             with patch("dvc.remote.http.open", m):
-                result = main(
-                    ["import-url", "--resume", import_url, import_output]
-                )
+                result = main(["import-url", "--resume", url, output])
                 self.assertEqual(result, 0)
-        m.assert_called_once_with(tmp_file_path, "ab")
+        m.assert_called_once_with(file_path, "ab")
         m_handle = m()
         expected_calls = [call(b"o"), call(b"o")]
         m_handle.write.assert_has_calls(expected_calls, any_order=False)
@@ -99,16 +93,16 @@ class TestShouldResumeDownload(TestInterruptedDownload):
 class TestShouldNotResumeDownload(TestInterruptedDownload):
     @patch("dvc.remote.http.RemoteHTTP.CHUNK_SIZE", 1)
     def test(self):
-        with StaticFileServer():
-            import_output, import_url, tmp_file_path = (
-                self._prepare_interrupted_download()
+        with StaticFileServer() as httpd:
+            output, url, file_path = self._prepare_interrupted_download(
+                httpd.server_port
             )
 
             m = mock_open()
             with patch("dvc.remote.http.open", m):
-                result = main(["import-url", import_url, import_output])
+                result = main(["import-url", url, output])
                 self.assertEqual(result, 0)
-        m.assert_called_once_with(tmp_file_path, "wb")
+        m.assert_called_once_with(file_path, "wb")
         m_handle = m()
         expected_calls = [call(b"f"), call(b"o"), call(b"o")]
         m_handle.write.assert_has_calls(expected_calls, any_order=False)

tests/func/test_repro.py

@@ -1142,36 +1142,18 @@ class TestReproExternalLOCAL(TestReproExternalBase):
 class TestReproExternalHTTP(TestReproExternalBase):
     _external_cache_id = None
 
-    @property
-    def remote(self):
-        return "http://localhost:8000/"
+    @staticmethod
+    def get_remote(port):
+        return "http://localhost:{}/".format(port)
 
     @property
     def local_cache(self):
         return os.path.join(self.dvc.dvc_dir, "cache")
 
-    @property
-    def external_cache_id(self):
-        if not self._external_cache_id:
-            self._external_cache_id = str(uuid.uuid4())
-
-        return self._external_cache_id
-
-    @property
-    def external_cache(self):
-        return urljoin(self.remote, self.external_cache_id)
-
     def test(self):
-        ret1 = main(["remote", "add", "mycache", self.external_cache])
-        ret2 = main(["remote", "add", "myremote", self.remote])
-        self.assertEqual(ret1, 0)
-        self.assertEqual(ret2, 0)
-
-        self.dvc = DvcRepo(".")
-
         # Import
-        with StaticFileServer():
-            import_url = urljoin(self.remote, self.FOO)
+        with StaticFileServer() as httpd:
+            import_url = urljoin(self.get_remote(httpd.server_port), self.FOO)
             import_output = "imported_file"
             import_stage = self.dvc.imp_url(import_url, import_output)
 
@@ -1180,8 +1162,8 @@ class TestReproExternalHTTP(TestReproExternalBase):
 
         self.dvc.remove("imported_file.dvc")
 
-        with StaticFileServer(handler="Content-MD5"):
-            import_url = urljoin(self.remote, self.FOO)
+        with StaticFileServer(handler="Content-MD5") as httpd:
+            import_url = urljoin(self.get_remote(httpd.server_port), self.FOO)
             import_output = "imported_file"
             import_stage = self.dvc.imp_url(import_url, import_output)
 
@@ -1189,8 +1171,20 @@ class TestReproExternalHTTP(TestReproExternalBase):
         self.assertTrue(filecmp.cmp(import_output, self.FOO, shallow=False))
 
         # Run --deps
-        with StaticFileServer():
-            run_dependency = urljoin(self.remote, self.BAR)
+        with StaticFileServer() as httpd:
+            remote = self.get_remote(httpd.server_port)
+
+            cache_id = str(uuid.uuid4())
+            cache = urljoin(remote, cache_id)
+
+            ret1 = main(["remote", "add", "mycache", cache])
+            ret2 = main(["remote", "add", "myremote", remote])
+            self.assertEqual(ret1, 0)
+            self.assertEqual(ret2, 0)
+
+            self.dvc = DvcRepo(".")
+
+            run_dependency = urljoin(remote, self.BAR)
             run_output = "remote_file"
             cmd = 'open("{}", "w+")'.format(run_output)
 
@@ -1204,19 +1198,18 @@ class TestReproExternalHTTP(TestReproExternalBase):
             )
             self.assertTrue(run_stage is not None)
 
-        self.assertTrue(os.path.exists(run_output))
+            self.assertTrue(os.path.exists(run_output))
 
-        # Pull
-        self.dvc.remove(import_stage.path, outs_only=True)
-        self.assertFalse(os.path.exists(import_output))
+            # Pull
+            self.dvc.remove(import_stage.path, outs_only=True)
+            self.assertFalse(os.path.exists(import_output))
 
-        shutil.move(self.local_cache, self.external_cache_id)
-        self.assertFalse(os.path.exists(self.local_cache))
+            shutil.move(self.local_cache, cache_id)
+            self.assertFalse(os.path.exists(self.local_cache))
 
-        with StaticFileServer():
             self.dvc.pull(import_stage.path, remote="mycache")
 
-        self.assertTrue(os.path.exists(import_output))
+            self.assertTrue(os.path.exists(import_output))
 
 
 class TestReproShell(TestDvc):

tests/utils/httpd.py

@@ -30,34 +30,22 @@ class ContentMD5Handler(SimpleHTTPRequestHandler):
 
 
 class StaticFileServer:
-    _server_lock = threading.Lock()
+    _lock = threading.Lock()
 
     def __init__(self, handler="etag"):
-        self._server_lock.acquire()
+        self._lock.acquire()
         handler_class = ETagHandler if handler == "etag" else ContentMD5Handler
-        self.bind_port_by_any_means(handler_class)
-
-    def bind_port_by_any_means(self, handler_class):
-        import time
-
-        # shutdowning/closing socket/server does not unbind port in time,
-        # locking the server also does not bring results, hence
-        # this method
-        for i in range(10000):
-            try:
-                self.httpd = HTTPServer(("localhost", 8000), handler_class)
-            except Exception:
-                time.sleep(0.01)
-                continue
-            break
+        self._httpd = HTTPServer(("localhost", 0), handler_class)
+        self._thread = None
 
     def __enter__(self):
-        self.server_thread = threading.Thread(target=self.httpd.serve_forever)
-        self.server_thread.daemon = True
-        self.server_thread.start()
+        self._thread = threading.Thread(target=self._httpd.serve_forever)
+        self._thread.daemon = True
+        self._thread.start()
+        return self._httpd
 
     def __exit__(self, *args):
-        self.httpd.socket.close()
-        self.httpd.shutdown()
-        self.httpd.server_close()
-        self._server_lock.release()
+        self._httpd.socket.close()
+        self._httpd.shutdown()
+        self._httpd.server_close()
+        self._lock.release()