Repository: conan-io/conan

Issue:
Enhance graph dependency resolution speed
Hello,

I have a big dependency graph that is managed in our CI system. About a hundred packages are handled and depends on each other by using version range like `[^1.66.0]` to keep, according to semver, the last backward compatible package.

The fact is that as all packages requirements are expressed that way.
I observed that conan, for each package, make a query to resolve the range and that can take a lot of time.

Observed times are with conan <= 1.4 a `conan info . -bo ... -bo ... [...]` on the graph would take about fifteen minutes (depending on the number of `-bo` that are passed).
On conan 1.6, it is twice as before.

As a way to enhance the resolution, can conan create a cache so they are resolved only once and future calls to the same range will be answered quickly ?

Discussion:
memsharded: Would also love to have the feedback of @claasd, IIRC, you had big dependency graphs and using version ranges too, so this fix might be good for you too.
claasd: Hi,

we completely change from version ranges to aliases once they became available. So sorry, can't help you here. We are quite happy with the alias feature, where we for example use OpenSSL/1.1.0 as alias for OpenSSL/1.1.0i, or MyPkg/1.0.latest as alias for MyPkg/1.0.2.

Best,
Claas
memsharded: Sounds good, thanks for the feedback @claasd !

Patch:
diff --git a/conans/client/graph/graph_builder.py b/conans/client/graph/graph_builder.py
index a802a879b..db30049d5 100644
--- a/conans/client/graph/graph_builder.py
+++ b/conans/client/graph/graph_builder.py
@@ -50,7 +50,7 @@ class DepsGraphBuilder(object):
 
         # After resolving ranges,
         for req in conanfile.requires.values():
-            alias = aliased.get(req.conan_reference, None)
+            alias = aliased.get(req.conan_reference)
             if alias:
                 req.conan_reference = alias
 
@@ -140,7 +140,7 @@ class DepsGraphBuilder(object):
 
     def _config_node(self, node, down_reqs, down_ref, down_options):
         """ update settings and option in the current ConanFile, computing actual
-        requirement values, cause they can be overriden by downstream requires
+        requirement values, cause they can be overridden by downstream requires
         param settings: dict of settings values => {"os": "windows"}
         """
         try:
diff --git a/conans/client/graph/range_resolver.py b/conans/client/graph/range_resolver.py
index c92c5fe23..45c488f2b 100644
--- a/conans/client/graph/range_resolver.py
+++ b/conans/client/graph/range_resolver.py
@@ -27,6 +27,7 @@ class RangeResolver(object):
         self._output = output
         self._client_cache = client_cache
         self._remote_search = remote_search
+        self._cached_remote_found = {}
 
     def resolve(self, require, base_conanref, update, remote_name):
         version_range = require.version_range
@@ -69,13 +70,16 @@ class RangeResolver(object):
     def _resolve_local(self, search_ref, version_range):
         local_found = search_recipes(self._client_cache, search_ref)
         if local_found:
-            resolved_version = self._resolve_version(version_range, local_found)
-            if resolved_version:
-                return resolved_version
+            return self._resolve_version(version_range, local_found)
 
     def _resolve_remote(self, search_ref, version_range, remote_name):
+        remote_cache = self._cached_remote_found.setdefault(remote_name, {})
         # We should use ignorecase=False, we want the exact case!
-        remote_found = self._remote_search.search_remotes(search_ref, remote_name)
+        remote_found = remote_cache.get(search_ref)
+        if remote_found is None:
+            remote_found = self._remote_search.search_remotes(search_ref, remote_name)
+            # Empty list, just in case it returns None
+            remote_cache[search_ref] = remote_found or []
         if remote_found:
             return self._resolve_version(version_range, remote_found)
 
diff --git a/conans/client/rest/rest_client_common.py b/conans/client/rest/rest_client_common.py
index e9b38a7dc..ecfa18bdd 100644
--- a/conans/client/rest/rest_client_common.py
+++ b/conans/client/rest/rest_client_common.py
@@ -212,16 +212,6 @@ class RestCommonMethods(object):
         url = self._recipe_url(conan_reference) + "/remove_files"
         return self._post_json(url, payload)
 
-    @handle_return_deserializer()
-    def _remove_package_files(self, package_reference, files):
-        """ Remove package files """
-        self.check_credentials()
-        payload = {"files": [filename.replace("\\", "/") for filename in files]}
-        url = "%s/conans/%s/packages/%s/remove_files" % (self.remote_api_url,
-                                                         "/".join(package_reference.conan),
-                                                         package_reference.package_id)
-        return self._post_json(url, payload)
-
     def _post_json(self, url, payload):
         response = self.requester.post(url,
                                        auth=self.auth,
diff --git a/conans/client/rest/rest_client_v1.py b/conans/client/rest/rest_client_v1.py
index c419b04fb..768472291 100644
--- a/conans/client/rest/rest_client_v1.py
+++ b/conans/client/rest/rest_client_v1.py
@@ -276,7 +276,9 @@ class RestV1Methods(RestCommonMethods):
             self._output.writeln("")
             self._upload_files(urls, files_to_upload, self._output, retry, retry_wait)
         if deleted:
-            self._remove_package_files(package_reference, deleted)
+            raise Exception("This shouldn't be happening, deleted files "
+                            "in local package present in remote: %s.\n Please, report it at "
+                            "https://github.com/conan-io/conan/issues " % str(deleted))
 
         logger.debug("====> Time rest client upload_package: %f" % (time.time() - t1))
         return files_to_upload or deleted
diff --git a/conans/client/rest/uploader_downloader.py b/conans/client/rest/uploader_downloader.py
index b52a9aac6..cff66cbfa 100644
--- a/conans/client/rest/uploader_downloader.py
+++ b/conans/client/rest/uploader_downloader.py
@@ -115,7 +115,7 @@ class Downloader(object):
         self.requester = requester
         self.verify = verify
 
-    def download(self, url, file_path=None, auth=None, retry=1, retry_wait=0, overwrite=False,
+    def download(self, url, file_path=None, auth=None, retry=3, retry_wait=0, overwrite=False,
                  headers=None):
 
         if file_path and not os.path.isabs(file_path):
@@ -130,10 +130,19 @@ class Downloader(object):
                 # the dest folder before
                 raise ConanException("Error, the file to download already exists: '%s'" % file_path)
 
+        return call_with_retry(self.output, retry, retry_wait, self._download_file, url, auth,
+                               headers, file_path)
+
+    def _download_file(self, url, auth, headers, file_path):
         t1 = time.time()
-        ret = bytearray()
-        response = call_with_retry(self.output, retry, retry_wait, self._download_file, url, auth, headers)
-        if not response.ok:  # Do not retry if not found or whatever controlled error
+
+        try:
+            response = self.requester.get(url, stream=True, verify=self.verify, auth=auth,
+                                          headers=headers)
+        except Exception as exc:
+            raise ConanException("Error downloading file %s: '%s'" % (url, exception_message_safe(exc)))
+
+        if not response.ok:
             if response.status_code == 404:
                 raise NotFoundException("Not found: %s" % url)
             elif response.status_code == 401:
@@ -141,61 +150,10 @@ class Downloader(object):
             raise ConanException("Error %d downloading file %s" % (response.status_code, url))
 
         try:
-            total_length = response.headers.get('content-length')
-
-            if total_length is None:  # no content length header
-                if not file_path:
-                    ret += response.content
-                else:
-                    total_length = len(response.content)
-                    progress = human_readable_progress(total_length, total_length)
-                    print_progress(self.output, 50, progress)
-                    save_append(file_path, response.content)
-            else:
-                total_length = int(total_length)
-                encoding = response.headers.get('content-encoding')
-                gzip = (encoding == "gzip")
-                # chunked can be a problem: https://www.greenbytes.de/tech/webdav/rfc2616.html#rfc.section.4.4
-                # It will not send content-length or should be ignored
-
-                def download_chunks(file_handler=None, ret_buffer=None):
-                    """Write to a buffer or to a file handler"""
-                    chunk_size = 1024 if not file_path else 1024 * 100
-                    download_size = 0
-                    last_progress = None
-                    for data in response.iter_content(chunk_size=chunk_size):
-                        download_size += len(data)
-                        if ret_buffer is not None:
-                            ret_buffer.extend(data)
-                        if file_handler is not None:
-                            file_handler.write(to_file_bytes(data))
-
-                        units = progress_units(download_size, total_length)
-                        progress = human_readable_progress(download_size, total_length)
-                        if last_progress != units:  # Avoid screen refresh if nothing has change
-                            if self.output:
-                                print_progress(self.output, units, progress)
-                            last_progress = units
-                    return download_size
-
-                if file_path:
-                    mkdir(os.path.dirname(file_path))
-                    with open(file_path, 'wb') as handle:
-                        dl_size = download_chunks(file_handler=handle)
-                else:
-                    dl_size = download_chunks(ret_buffer=ret)
-
-                if dl_size != total_length and not gzip:
-                    raise ConanException("Transfer interrupted before "
-                                         "complete: %s < %s" % (dl_size, total_length))
-
+            data = self._download_data(response, file_path)
             duration = time.time() - t1
             log_download(url, duration)
-
-            if not file_path:
-                return bytes(ret)
-            else:
-                return
+            return data
         except Exception as e:
             logger.debug(e.__class__)
             logger.debug(traceback.format_exc())
@@ -203,14 +161,62 @@ class Downloader(object):
             raise ConanConnectionError("Download failed, check server, possibly try again\n%s"
                                        % str(e))
 
-    def _download_file(self, url, auth, headers):
-        try:
-            response = self.requester.get(url, stream=True, verify=self.verify, auth=auth,
-                                          headers=headers)
-        except Exception as exc:
-            raise ConanException("Error downloading file %s: '%s'" % (url, exception_message_safe(exc)))
+    def _download_data(self, response, file_path):
+        ret = bytearray()
+        total_length = response.headers.get('content-length')
 
-        return response
+        if total_length is None:  # no content length header
+            if not file_path:
+                ret += response.content
+            else:
+                total_length = len(response.content)
+                progress = human_readable_progress(total_length, total_length)
+                print_progress(self.output, 50, progress)
+                save_append(file_path, response.content)
+        else:
+            total_length = int(total_length)
+            encoding = response.headers.get('content-encoding')
+            gzip = (encoding == "gzip")
+            # chunked can be a problem: https://www.greenbytes.de/tech/webdav/rfc2616.html#rfc.section.4.4
+            # It will not send content-length or should be ignored
+
+            def download_chunks(file_handler=None, ret_buffer=None):
+                """Write to a buffer or to a file handler"""
+                chunk_size = 1024 if not file_path else 1024 * 100
+                download_size = 0
+                last_progress = None
+                for data in response.iter_content(chunk_size):
+                    download_size += len(data)
+                    if ret_buffer is not None:
+                        ret_buffer.extend(data)
+                    if file_handler is not None:
+                        file_handler.write(to_file_bytes(data))
+
+                    units = progress_units(download_size, total_length)
+                    progress = human_readable_progress(download_size, total_length)
+                    if last_progress != units:  # Avoid screen refresh if nothing has change
+                        if self.output:
+                            print_progress(self.output, units, progress)
+                        last_progress = units
+                return download_size
+
+            if file_path:
+                mkdir(os.path.dirname(file_path))
+                with open(file_path, 'wb') as handle:
+                    dl_size = download_chunks(file_handler=handle)
+            else:
+                dl_size = download_chunks(ret_buffer=ret)
+
+            response.close()
+
+            if dl_size != total_length and not gzip:
+                raise ConanException("Transfer interrupted before "
+                                     "complete: %s < %s" % (dl_size, total_length))
+
+        if not file_path:
+            return bytes(ret)
+        else:
+            return
 
 
 def progress_units(progress, total):
diff --git a/conans/client/tools/oss.py b/conans/client/tools/oss.py
index ffa8b9d21..9e492a9ed 100644
--- a/conans/client/tools/oss.py
+++ b/conans/client/tools/oss.py
@@ -122,9 +122,12 @@ class OSInfo(object):
 
     @property
     def with_zypper(self):
-        return self.is_linux and self.linux_distro in \
-            ("opensuse", "sles")
-
+        if not self.is_linux:
+            return False
+        if "opensuse" in self.linux_distro or "sles" in self.linux_distro:
+            return True
+        return False
+    
     @staticmethod
     def get_win_os_version():
         """
diff --git a/conans/client/tools/scm.py b/conans/client/tools/scm.py
index d43c385e5..de4cb9184 100644
--- a/conans/client/tools/scm.py
+++ b/conans/client/tools/scm.py
@@ -47,7 +47,7 @@ class Git(object):
     def _configure_ssl_verify(self):
         return self.run("config http.sslVerify %s" % ("true" if self._verify_ssl else "false"))
 
-    def clone(self, url, branch=None, submodule=None):
+    def clone(self, url, branch=None):
         url = self.get_url_with_credentials(url)
         if os.path.exists(url):
             url = url.replace("\\", "/")  # Windows local directory
@@ -67,6 +67,12 @@ class Git(object):
             output = self.run('clone "%s" . %s' % (url, branch_cmd))
             output += self._configure_ssl_verify()
 
+        return output
+
+    def checkout(self, element, submodule=None):
+        self._check_git_repo()
+        output = self.run('checkout "%s"' % element)
+
         if submodule:
             if submodule == "shallow":
                 output += self.run("submodule sync")
@@ -77,13 +83,8 @@ class Git(object):
             else:
                 raise ConanException("Invalid 'submodule' attribute value in the 'scm'. "
                                      "Unknown value '%s'. Allowed values: ['shallow', 'recursive']" % submodule)
-
-        return output
-
-    def checkout(self, element):
-        self._check_git_repo()
         # Element can be a tag, branch or commit
-        return self.run('checkout "%s"' % element)
+        return output
 
     def excluded_files(self):
         try:
diff --git a/conans/model/requires.py b/conans/model/requires.py
index ab8d03095..535771892 100644
--- a/conans/model/requires.py
+++ b/conans/model/requires.py
@@ -120,7 +120,7 @@ class Requirements(OrderedDict):
                 # update dependency
                 other_ref = other_req.conan_reference
                 if other_ref and other_ref != req.conan_reference:
-                    output.info("%s requirement %s overriden by %s to %s "
+                    output.info("%s requirement %s overridden by %s to %s "
                                 % (own_ref, req.conan_reference, down_ref or "your conanfile",
                                    other_ref))
                     req.conan_reference = other_ref
diff --git a/conans/model/scm.py b/conans/model/scm.py
index e3a47e2e2..e7ff64aca 100644
--- a/conans/model/scm.py
+++ b/conans/model/scm.py
@@ -65,10 +65,10 @@ class SCM(object):
         return self.repo.excluded_files()
 
     def clone(self):
-        return self.repo.clone(self._data.url, submodule=self._data.submodule)
+        return self.repo.clone(self._data.url)
 
     def checkout(self):
-        return self.repo.checkout(self._data.revision)
+        return self.repo.checkout(self._data.revision, submodule=self._data.submodule)
 
     def get_remote_url(self):
         return self.repo.get_remote_url()


Test patch:
diff --git a/conans/test/functional/client_certs_test.py b/conans/test/functional/client_certs_test.py
index a571e2ecf..377e860b4 100644
--- a/conans/test/functional/client_certs_test.py
+++ b/conans/test/functional/client_certs_test.py
@@ -2,7 +2,7 @@ import os
 import unittest
 
 from conans import tools
-from conans.test.utils.tools import TestClient, TestServer
+from conans.test.utils.tools import TestClient
 
 
 class ClientCertsTest(unittest.TestCase):
diff --git a/conans/test/functional/download_retries_test.py b/conans/test/functional/download_retries_test.py
new file mode 100644
index 000000000..58c2f91fc
--- /dev/null
+++ b/conans/test/functional/download_retries_test.py
@@ -0,0 +1,59 @@
+import unittest
+
+from conans.paths import CONANFILE
+from conans.test.utils.tools import TestClient, TestServer
+
+
+class DownloadRetriesTest(unittest.TestCase):
+
+    def test_recipe_download_retry(self):
+        test_server = TestServer()
+        client = TestClient(servers={"default": test_server},
+                            users={"default": [("lasote", "mypass")]})
+
+        conanfile = '''from conans import ConanFile
+class MyConanfile(ConanFile):
+    pass
+'''
+        client.save({CONANFILE: conanfile})
+        client.run("create . Pkg/0.1@lasote/stable")
+        client.run("upload '*' -c --all")
+
+        class Response(object):
+            ok = None
+            status_code = None
+            charset = None
+            headers = {}
+
+            def __init__(self, ok, status_code):
+                self.ok = ok
+                self.status_code = status_code
+
+            @property
+            def content(self):
+                if not self.ok:
+                    raise Exception("Bad boy")
+                else:
+                    return b'{"conanfile.py": "path/to/fake/file"}'
+
+            text = content
+
+        class BuggyRequester(object):
+
+            def __init__(self, *args, **kwargs):
+                pass
+
+            def get(self, *args, **kwargs):
+                if "path/to/fake/file" not in args[0]:
+                    return Response(True, 200)
+                else:
+                    return Response(False, 200)
+
+        # The buggy requester will cause a failure only downloading files, not in regular requests
+        client = TestClient(servers={"default": test_server},
+                            users={"default": [("lasote", "mypass")]},
+                            requester_class=BuggyRequester)
+        error = client.run("install Pkg/0.1@lasote/stable", ignore_error=True)
+        self.assertTrue(error)
+        self.assertEquals(str(client.out).count("Waiting 0 seconds to retry..."), 2)
+        self.assertEquals(str(client.out).count("ERROR: Error 200 downloading"), 3)
diff --git a/conans/test/functional/require_override_test.py b/conans/test/functional/require_override_test.py
index 1a7b209e4..fa7c259cf 100644
--- a/conans/test/functional/require_override_test.py
+++ b/conans/test/functional/require_override_test.py
@@ -75,5 +75,5 @@ class RequireOverrideTest(unittest.TestCase):
                                                       "libC/1.0@user/channel",
                                                       ("libA/1.0@user/channel", "override")])
             self.client.run("create . user/channel")
-            self.assertIn("libA/2.0@user/channel overriden by project/1.0@user/channel",
+            self.assertIn("libA/2.0@user/channel overridden by project/1.0@user/channel",
                           self.client.out)
diff --git a/conans/test/model/transitive_reqs_test.py b/conans/test/model/transitive_reqs_test.py
index b902dab53..7b0f08c53 100644
--- a/conans/test/model/transitive_reqs_test.py
+++ b/conans/test/model/transitive_reqs_test.py
@@ -465,7 +465,7 @@ class ChatConan(ConanFile):
         self.retriever.conan(bye_ref, bye_content2)
         deps_graph = self.root(chat_content)
 
-        self.assertIn("Hello/1.2@user/testing requirement Say/0.1@user/testing overriden by "
+        self.assertIn("Hello/1.2@user/testing requirement Say/0.1@user/testing overridden by "
                       "your conanfile to Say/0.2@user/testing", self.output)
         self.assertNotIn("Conflict", self.output)
         self.assertEqual(4, len(deps_graph.nodes))
@@ -1539,7 +1539,7 @@ class LibDConan(ConanFile):
 
         with self.assertRaisesRegexp(ConanException, "Conflict in LibB/0.1@user/testing"):
             self.root(self.consumer_content)
-        self.assertIn("LibB/0.1@user/testing requirement LibA/0.1@user/testing overriden by "
+        self.assertIn("LibB/0.1@user/testing requirement LibA/0.1@user/testing overridden by "
                       "LibD/0.1@user/testing to LibA/0.2@user/testing", str(self.output))
         self.assertEqual(1, str(self.output).count("LibA requirements()"))
         self.assertEqual(1, str(self.output).count("LibA configure()"))
diff --git a/conans/test/model/version_ranges_test.py b/conans/test/model/version_ranges_test.py
index 2c4838c19..3e0122605 100644
--- a/conans/test/model/version_ranges_test.py
+++ b/conans/test/model/version_ranges_test.py
@@ -1,6 +1,6 @@
 import os
 import unittest
-from collections import namedtuple
+from collections import namedtuple, Counter
 
 from conans.test.utils.tools import TestBufferConanOutput
 from conans.paths import CONANFILE, SimplePaths
@@ -141,8 +141,10 @@ def _get_edges(graph):
 class MockSearchRemote(object):
     def __init__(self, packages=None):
         self.packages = packages or []
+        self.count = Counter()
 
     def search_remotes(self, pattern, ignorecase):  # @UnusedVariable
+        self.count[pattern] += 1
         return self.packages
 
 
@@ -168,9 +170,9 @@ class SayConan(ConanFile):
             say_ref = ConanFileReference.loads("Say/%s@memsharded/testing" % v)
             self.retriever.conan(say_ref, say_content)
 
-    def root(self, content):
+    def root(self, content, update=False):
         root_conan = self.retriever.root(content)
-        deps_graph = self.builder.load_graph(root_conan, False, False, None)
+        deps_graph = self.builder.load_graph(root_conan, update, update, None)
         return deps_graph
 
     def test_local_basic(self):
@@ -205,7 +207,63 @@ class SayConan(ConanFile):
             say_ref = ConanFileReference.loads("Say/%s@memsharded/testing" % v)
             remote_packages.append(say_ref)
         self.remote_search.packages = remote_packages
-        self.test_local_basic()
+        for expr, solution in [(">0.0", "2.2.1"),
+                               (">0.1,<1", "0.3"),
+                               (">0.1,<1||2.1", "2.1"),
+                               ("", "2.2.1"),
+                               ("~0", "0.3"),
+                               ("~=1", "1.2.1"),
+                               ("~1.1", "1.1.2"),
+                               ("~=2", "2.2.1"),
+                               ("~=2.1", "2.1"),
+                               ]:
+            deps_graph = self.root(hello_content % expr, update=True)
+            self.assertEqual(self.remote_search.count, {'Say/*@memsharded/testing': 1})
+            self.assertEqual(2, len(deps_graph.nodes))
+            hello = _get_nodes(deps_graph, "Hello")[0]
+            say = _get_nodes(deps_graph, "Say")[0]
+            self.assertEqual(_get_edges(deps_graph), {Edge(hello, say)})
+
+            self.assertEqual(hello.conan_ref, None)
+            conanfile = hello.conanfile
+            self.assertEqual(conanfile.version, "1.2")
+            self.assertEqual(conanfile.name, "Hello")
+            say_ref = ConanFileReference.loads("Say/%s@memsharded/testing" % solution)
+            self.assertEqual(conanfile.requires, Requirements(str(say_ref)))
+
+    def test_remote_optimized(self):
+        self.resolver._local_search = None
+        remote_packages = []
+        for v in ["0.1", "0.2", "0.3", "1.1", "1.1.2", "1.2.1", "2.1", "2.2.1"]:
+            say_ref = ConanFileReference.loads("Say/%s@memsharded/testing" % v)
+            remote_packages.append(say_ref)
+        self.remote_search.packages = remote_packages
+
+        dep_content = """from conans import ConanFile
+class Dep1Conan(ConanFile):
+    requires = "Say/[%s]@memsharded/testing"
+"""
+        dep_ref = ConanFileReference.loads("Dep1/0.1@memsharded/testing")
+        self.retriever.conan(dep_ref, dep_content % ">=0.1")
+        dep_ref = ConanFileReference.loads("Dep2/0.1@memsharded/testing")
+        self.retriever.conan(dep_ref, dep_content % ">=0.1")
+
+        hello_content = """from conans import ConanFile
+class HelloConan(ConanFile):
+    name = "Hello"
+    requires = "Dep1/0.1@memsharded/testing", "Dep2/0.1@memsharded/testing"
+"""
+        deps_graph = self.root(hello_content, update=True)
+        self.assertEqual(4, len(deps_graph.nodes))
+        hello = _get_nodes(deps_graph, "Hello")[0]
+        say = _get_nodes(deps_graph, "Say")[0]
+        dep1 = _get_nodes(deps_graph, "Dep1")[0]
+        dep2 = _get_nodes(deps_graph, "Dep2")[0]
+        self.assertEqual(_get_edges(deps_graph), {Edge(hello, dep1), Edge(hello, dep2),
+                                                  Edge(dep1, say), Edge(dep2, say)})
+
+        # Most important check: counter of calls to remote
+        self.assertEqual(self.remote_search.count, {'Say/*@memsharded/testing': 1})
 
     @parameterized.expand([("", "0.3", None, None),
                            ('"Say/1.1@memsharded/testing"', "1.1", False, False),
@@ -242,9 +300,9 @@ class ChatConan(ConanFile):
         chat = _get_nodes(deps_graph, "Chat")[0]
         edges = {Edge(hello, say), Edge(chat, hello)}
         if override is not None:
-            self.assertIn("override", self.output)
+            self.assertIn("overridden", self.output)
         else:
-            self.assertNotIn("override", self.output)
+            self.assertNotIn("overridden", self.output)
         if override is False:
             edges = {Edge(hello, say), Edge(chat, say), Edge(chat, hello)}
 
diff --git a/conans/test/util/tools_test.py b/conans/test/util/tools_test.py
index 21a951435..86576451b 100644
--- a/conans/test/util/tools_test.py
+++ b/conans/test/util/tools_test.py
@@ -1125,7 +1125,7 @@ class GitToolTest(unittest.TestCase):
     def test_clone_submodule_git(self):
         subsubmodule, _ = create_local_git_repo({"subsubmodule": "contents"})
         submodule, _ = create_local_git_repo({"submodule": "contents"}, submodules=[subsubmodule])
-        path, _ = create_local_git_repo({"myfile": "contents"}, submodules=[submodule])
+        path, commit = create_local_git_repo({"myfile": "contents"}, submodules=[submodule])
 
         def _create_paths():
             tmp = temp_folder()
@@ -1147,13 +1147,15 @@ class GitToolTest(unittest.TestCase):
         # Check invalid value
         tmp, submodule_path, subsubmodule_path = _create_paths()
         git = Git(tmp)
+        git.clone(path)
         with self.assertRaisesRegexp(ConanException, "Invalid 'submodule' attribute value in the 'scm'."):
-            git.clone(path, submodule="invalid")
+            git.checkout(commit, submodule="invalid")
 
         # Check shallow
         tmp, submodule_path, subsubmodule_path = _create_paths()
         git = Git(tmp)
-        git.clone(path, submodule="shallow")
+        git.clone(path)
+        git.checkout(commit, submodule="shallow")
         self.assertTrue(os.path.exists(os.path.join(tmp, "myfile")))
         self.assertTrue(os.path.exists(os.path.join(submodule_path, "submodule")))
         self.assertFalse(os.path.exists(os.path.join(subsubmodule_path, "subsubmodule")))
@@ -1161,7 +1163,8 @@ class GitToolTest(unittest.TestCase):
         # Check recursive
         tmp, submodule_path, subsubmodule_path = _create_paths()
         git = Git(tmp)
-        git.clone(path, submodule="recursive")
+        git.clone(path)
+        git.checkout(commit, submodule="recursive")
         self.assertTrue(os.path.exists(os.path.join(tmp, "myfile")))
         self.assertTrue(os.path.exists(os.path.join(submodule_path, "submodule")))
         self.assertTrue(os.path.exists(os.path.join(subsubmodule_path, "subsubmodule")))
diff --git a/conans/test/utils/tools.py b/conans/test/utils/tools.py
index 7c882db0d..f4ea46fcd 100644
--- a/conans/test/utils/tools.py
+++ b/conans/test/utils/tools.py
@@ -69,6 +69,9 @@ class TestingResponse(object):
     def __init__(self, test_response):
         self.test_response = test_response
 
+    def close(self):
+        pass  # Compatibility with close() method of a requests when stream=True
+
     @property
     def headers(self):
         return self.test_response.headers