tests: use mocker instead of unittest.mock where possible

Had the "pleasure" of finding a mocker.patch leak into other tests and one
of the suspicions is that mixing unittest.mock and mocker might be somehow
related.

In any case, we should use mocker where possible anyway.

tests/func/test_add.py

@@ -4,7 +4,6 @@ import os
 import shutil
 import stat
 import textwrap
-from unittest.mock import call
 
 import pytest
 
@@ -631,8 +630,8 @@ def test_add_optimization_for_hardlink_on_empty_files(tmp_dir, dvc, mocker):
     stages = dvc.add(["foo", "bar", "lorem", "ipsum"])
 
     assert m.call_count == 8
-    assert m.call_args != call(tmp_dir / "foo")
-    assert m.call_args != call(tmp_dir / "bar")
+    assert m.call_args != mocker.call(tmp_dir / "foo")
+    assert m.call_args != mocker.call(tmp_dir / "bar")
 
     for stage in stages[:2]:
         # hardlinks are not created for empty files

tests/func/test_analytics.py

@@ -1,5 +1,4 @@
 import os
-from unittest.mock import call
 
 import pytest
 
@@ -40,7 +39,7 @@ def test_collect_and_send_report(mocker, dvc, mock_daemon):
 
     assert mock_daemon.call_count == 1
     assert mock_post.call_count == 1
-    assert mock_post.call_args == call(
+    assert mock_post.call_args == mocker.call(
         "https://analytics.dvc.org",
         json=ANY(dict),
         headers={"content-type": "application/json"},

tests/func/test_external_repo.py

@@ -1,5 +1,4 @@
 import os
-from unittest.mock import ANY
 
 from dvc.repo.open_repo import CLONES
 from dvc.repo.open_repo import _external_repo as external_repo
@@ -140,7 +139,9 @@ def test_shallow_clone_branch(erepo_dir, mocker):
         with repo.dvcfs.open("file") as fd:
             assert fd.read() == "branch"
 
-    clone_spy.assert_called_with(url, ANY, shallow_branch="branch", progress=ANY)
+    clone_spy.assert_called_with(
+        url, mocker.ANY, shallow_branch="branch", progress=mocker.ANY
+    )
 
     path, _ = CLONES[url]
     CLONES[url] = (path, True)
@@ -165,7 +166,9 @@ def test_shallow_clone_tag(erepo_dir, mocker):
         with repo.dvcfs.open("file") as fd:
             assert fd.read() == "foo"
 
-    clone_spy.assert_called_with(url, ANY, shallow_branch="v1", progress=ANY)
+    clone_spy.assert_called_with(
+        url, mocker.ANY, shallow_branch="v1", progress=mocker.ANY
+    )
 
     path, _ = CLONES[url]
     CLONES[url] = (path, True)

tests/func/test_remote.py

@@ -2,7 +2,6 @@ import errno
 import itertools
 import os
 import stat
-from unittest.mock import patch
 
 import configobj
 import pytest
@@ -128,7 +127,7 @@ def test_upper_case_remote(tmp_dir, dvc, local_cloud):
     assert ret == 0
 
 
-def test_dir_hash_should_be_key_order_agnostic(tmp_dir, dvc):
+def test_dir_hash_should_be_key_order_agnostic(tmp_dir, dvc, mocker):
     from dvc_data.hashfile.build import build
     from dvc_data.hashfile.tree import Tree
 
@@ -138,24 +137,27 @@ def test_dir_hash_should_be_key_order_agnostic(tmp_dir, dvc):
 
     tree = Tree.from_list([{"relpath": "1", "md5": "1"}, {"relpath": "2", "md5": "2"}])
     tree.digest()
-    with patch("dvc_data.hashfile.build._build_tree", return_value=(None, tree)):
-        _, _, obj = build(dvc.cache.local, path, dvc.cache.local.fs, "md5")
-        hash1 = obj.hash_info
+
+    mocker.patch("dvc_data.hashfile.build._build_tree", return_value=(None, tree))
+
+    _, _, obj = build(dvc.cache.local, path, dvc.cache.local.fs, "md5")
+    hash1 = obj.hash_info
 
     # remove the raw dir obj to force building the tree on the next build call
     dvc.cache.local.fs.remove(dvc.cache.local.oid_to_path(hash1.as_raw().value))
 
     tree = Tree.from_list([{"md5": "1", "relpath": "1"}, {"md5": "2", "relpath": "2"}])
     tree.digest()
-    with patch("dvc_data.hashfile.build._build_tree", return_value=(None, tree)):
-        _, _, obj = build(dvc.cache.local, path, dvc.cache.local.fs, "md5")
-        hash2 = obj.hash_info
+
+    # NOTE: _build_tree already patched above
+    _, _, obj = build(dvc.cache.local, path, dvc.cache.local.fs, "md5")
+    hash2 = obj.hash_info
 
     assert hash1 == hash2
 
 
 def test_partial_push_n_pull(  # noqa: C901
-    tmp_dir, dvc, tmp_path_factory, local_remote
+    tmp_dir, dvc, tmp_path_factory, local_remote, mocker
 ):
     from dvc_objects.fs import generic
 
@@ -190,14 +192,15 @@ def test_partial_push_n_pull(  # noqa: C901
                 del to_info[i]
         return original(from_fs, from_info, to_fs, to_info, **kwargs)
 
-    with patch.object(generic, "transfer", unreliable_upload):
-        with pytest.raises(UploadError) as upload_error_info:
-            dvc.push()
-        assert upload_error_info.value.amount == 2
+    mock_upload = mocker.patch.object(generic, "transfer", unreliable_upload)
+    with pytest.raises(UploadError) as upload_error_info:
+        dvc.push()
+    assert upload_error_info.value.amount == 2
 
-        assert not odb.exists(foo.hash_info.value)
-        assert odb.exists(bar.hash_info.value)
-        assert not odb.exists(baz.hash_info.value)
+    assert not odb.exists(foo.hash_info.value)
+    assert odb.exists(bar.hash_info.value)
+    assert not odb.exists(baz.hash_info.value)
+    mocker.stop(mock_upload)
 
     # Push everything and delete local cache
     dvc.push()
@@ -215,12 +218,13 @@ def test_partial_push_n_pull(  # noqa: C901
         for from_i, to_i in zip(from_info, to_info):
             on_error(from_i, to_i, Exception())
 
-    with patch.object(generic, "transfer", unreliable_download):
-        with pytest.raises(DownloadError) as download_error_info:
-            dvc.pull()
-        # error count should be len(.dir + standalone file checksums)
-        # since files inside dir are ignored if dir cache entry is missing
-        assert download_error_info.value.amount == 2
+    mock_download = mocker.patch.object(generic, "transfer", unreliable_download)
+    with pytest.raises(DownloadError) as download_error_info:
+        dvc.pull()
+    # error count should be len(.dir + standalone file checksums)
+    # since files inside dir are ignored if dir cache entry is missing
+    assert download_error_info.value.amount == 2
+    mocker.stop(mock_download)
 
 
 def test_raise_on_too_many_open_files(

tests/unit/command/test_machine.py

@@ -1,5 +1,4 @@
 import os
-from unittest.mock import call
 
 import configobj
 import pytest
@@ -77,7 +76,7 @@ def test_status(tmp_dir, scm, dvc, mocker):
     m = mocker.patch.object(cmd.repo.machine, "status", autospec=True, return_value=[])
     assert cmd.run() == 0
     assert m.call_count == 2
-    m.assert_has_calls([call("foo"), call("myaws")])
+    m.assert_has_calls([mocker.call("foo"), mocker.call("myaws")])
 
 
 def test_destroy(tmp_dir, dvc, mocker):

tests/unit/fs/test_dvc.py

@@ -1,7 +1,6 @@
 import os
 import posixpath
 import shutil
-from unittest import mock
 
 import pytest
 
@@ -346,33 +345,35 @@ def test_subrepos(tmp_dir, scm, dvc, mocker):
 
         return f
 
-    with mock.patch.object(
+    mock_subrepo1 = mocker.patch.object(
         fs.fs, "_get_repo", side_effect=assert_fs_belongs_to_repo(subrepo1.dvc)
-    ):
-        assert fs.exists("dir/repo/foo") is True
-        assert fs.exists("dir/repo/bar") is False
+    )
+    assert fs.exists("dir/repo/foo") is True
+    assert fs.exists("dir/repo/bar") is False
 
-        assert fs.isfile("dir/repo/foo") is True
-        assert fs.isfile("dir/repo/dir1/bar") is True
-        assert fs.isfile("dir/repo/dir1") is False
+    assert fs.isfile("dir/repo/foo") is True
+    assert fs.isfile("dir/repo/dir1/bar") is True
+    assert fs.isfile("dir/repo/dir1") is False
 
-        assert fs.isdir("dir/repo/dir1") is True
-        assert fs.isdir("dir/repo/dir1/bar") is False
-        assert fs.isdvc("dir/repo/foo") is True
+    assert fs.isdir("dir/repo/dir1") is True
+    assert fs.isdir("dir/repo/dir1/bar") is False
+    assert fs.isdvc("dir/repo/foo") is True
+    mocker.stop(mock_subrepo1)
 
-    with mock.patch.object(
+    mock_subrepo2 = mocker.patch.object(
         fs.fs, "_get_repo", side_effect=assert_fs_belongs_to_repo(subrepo2.dvc)
-    ):
-        assert fs.exists("dir/repo2/lorem") is True
-        assert fs.exists("dir/repo2/ipsum") is False
+    )
+    assert fs.exists("dir/repo2/lorem") is True
+    assert fs.exists("dir/repo2/ipsum") is False
 
-        assert fs.isfile("dir/repo2/lorem") is True
-        assert fs.isfile("dir/repo2/dir2/ipsum") is True
-        assert fs.isfile("dir/repo2/dir2") is False
+    assert fs.isfile("dir/repo2/lorem") is True
+    assert fs.isfile("dir/repo2/dir2/ipsum") is True
+    assert fs.isfile("dir/repo2/dir2") is False
 
-        assert fs.isdir("dir/repo2/dir2") is True
-        assert fs.isdir("dir/repo2/dir2/ipsum") is False
-        assert fs.isdvc("dir/repo2/lorem") is True
+    assert fs.isdir("dir/repo2/dir2") is True
+    assert fs.isdir("dir/repo2/dir2/ipsum") is False
+    assert fs.isdvc("dir/repo2/lorem") is True
+    mocker.stop(mock_subrepo2)
 
 
 @pytest.mark.parametrize(

tests/unit/repo/experiments/queue/test_remove.py

@@ -1,5 +1,3 @@
-from unittest.mock import call
-
 from dvc.repo.experiments.queue.base import QueueDoneResult
 
 
@@ -37,7 +35,11 @@ def test_remove_queued(test_queue, mocker):
     assert test_queue.clear(queued=True) == queued_test
     remove_revs_mocker.assert_called_once_with(list(stash_dict.values()))
     reject_mocker.assert_has_calls(
-        [call("msg_queue1"), call("msg_queue2"), call("msg_queue3")]
+        [
+            mocker.call("msg_queue1"),
+            mocker.call("msg_queue2"),
+            mocker.call("msg_queue3"),
+        ]
     )
 
 
@@ -84,7 +86,9 @@ def test_remove_done(test_queue, mocker):
         "success2",
     ]
     remove_revs_mocker.assert_called_once_with([stash_dict["failed3"]])
-    purge_mocker.assert_has_calls([call("msg_failed3"), call("msg_success2")])
+    purge_mocker.assert_has_calls(
+        [mocker.call("msg_failed3"), mocker.call("msg_success2")]
+    )
 
     remove_revs_mocker.reset_mock()
     purge_mocker.reset_mock()
@@ -94,12 +98,12 @@ def test_remove_done(test_queue, mocker):
     )
     purge_mocker.assert_has_calls(
         [
-            call("msg_failed1"),
-            call("msg_failed2"),
-            call("msg_failed3"),
-            call("msg_success1"),
-            call("msg_success2"),
-            call("msg_success3"),
+            mocker.call("msg_failed1"),
+            mocker.call("msg_failed2"),
+            mocker.call("msg_failed3"),
+            mocker.call("msg_success1"),
+            mocker.call("msg_success2"),
+            mocker.call("msg_success3"),
         ],
         any_order=True,
     )

tests/unit/repo/test_open_repo.py

@@ -1,5 +1,4 @@
 import os
-from unittest.mock import call
 
 import pytest
 
@@ -34,7 +33,7 @@ def test_hook_is_called(tmp_dir, erepo_dir, mocker):
         paths = ["/" + path.replace("\\", "/") for path in subrepo_paths]
         spy.assert_has_calls(
             [
-                call(
+                mocker.call(
                     path,
                     fs=repo.fs,
                     scm=repo.scm,

tests/unit/test_ignore.py

@@ -1,21 +1,10 @@
 import os
-from unittest.mock import MagicMock, mock_open, patch
 
 import pytest
 
 from dvc.ignore import DvcIgnorePatterns
 
 
-def mock_dvcignore(dvcignore_path, patterns):
-    from dvc.fs import localfs
-
-    fs = MagicMock()
-    fs.path = localfs.path
-    fs.sep = localfs.sep
-    with patch.object(fs, "open", mock_open(read_data="\n".join(patterns))):
-        return DvcIgnorePatterns.from_file(dvcignore_path, fs, "mocked")
-
-
 @pytest.mark.parametrize(
     "file_to_ignore_relpath, patterns,  expected_match",
     [
@@ -174,13 +163,21 @@ def mock_dvcignore(dvcignore_path, patterns):
         ),
     ],
 )
-def test_match_ignore_from_file(file_to_ignore_relpath, patterns, expected_match):
+def test_match_ignore_from_file(
+    file_to_ignore_relpath, patterns, expected_match, mocker
+):
+    from dvc.fs import localfs
+
     dvcignore_path = os.path.join(
         os.path.sep, "full", "path", "to", "ignore", "file", ".dvcignore"
     )
     dvcignore_dirname = os.path.dirname(dvcignore_path)
 
-    ignore_file = mock_dvcignore(dvcignore_path, patterns)
+    fs = mocker.MagicMock()
+    fs.path = localfs.path
+    fs.sep = localfs.sep
+    mocker.patch.object(fs, "open", mocker.mock_open(read_data="\n".join(patterns)))
+    ignore_file = DvcIgnorePatterns.from_file(dvcignore_path, fs, "mocked")
 
     assert (
         ignore_file.matches(dvcignore_dirname, file_to_ignore_relpath) == expected_match