fix(diff): do not show missing cache entries on diff result (#10845)

Author: skshetry

dvc/repo/diff.py

@@ -45,9 +45,11 @@ def meta_cmp_key(meta):
         with_renames=True,
         meta_cmp_key=meta_cmp_key,
         roots=data_keys,
-        # we need to get unknown and unchanged entries to tell whether
-        # the object is missing from the cache or not.
-        with_unknown=with_missing,
+        # Include unknown entries from missing dir entry, so that they don't
+        # get reported as added/modified/deleted.
+        # Also return unchanged entries so that we can check if they are missing
+        # from cache.
+        with_unknown=True,
         with_unchanged=with_missing,
     ):
         if (change.old and change.old.isdir and not change.old.hash_info) or (

tests/func/test_diff.py

@@ -2,7 +2,6 @@
 import os
 
 import pytest
-from funcy import first
 
 from dvc.exceptions import DvcException
 from dvc.utils.fs import remove
@@ -96,6 +95,81 @@ def test_no_cache_entry(tmp_dir, scm, dvc):
     }
 
 
+def test_diff_no_cache(tmp_dir, scm, dvc):
+    (stage,) = tmp_dir.dvc_gen({"dir": {"file": "file content"}}, commit="first")
+    scm.tag("v1")
+    dvc.cache.local.clear()
+    old_digest = stage.outs[0].hash_info.value
+    dir_path = os.path.join("dir", "")
+
+    default_result = {
+        "added": [],
+        "deleted": [],
+        "modified": [],
+        "renamed": [],
+        "not in cache": [],
+    }
+
+    assert dvc.diff("v1") == default_result | {
+        "not in cache": [{"path": dir_path, "hash": old_digest}],
+    }
+    assert dvc.diff("HEAD", "v1") == {}
+    assert dvc.diff("v1", "HEAD") == {}
+
+    (stage,) = tmp_dir.dvc_gen(
+        {"dir": {"file": "modified file content"}}, commit="first"
+    )
+    scm.tag("v2")
+    new_digest = stage.outs[0].hash_info.value
+
+    assert dvc.diff("v2") == {}
+    assert dvc.diff("v1") == default_result | {
+        "modified": [
+            {"path": dir_path, "hash": {"old": old_digest, "new": new_digest}}
+        ],
+        "not in cache": [{"path": dir_path, "hash": old_digest}],
+    }
+    assert dvc.diff("v1", "v2") == default_result | {
+        "modified": [
+            {"path": dir_path, "hash": {"old": old_digest, "new": new_digest}}
+        ],
+    }
+
+    remove(dvc.cache.local.path)
+    # drop the cache so that we can test as if we don't know what entries are
+    # in the missing cache entry.
+    dvc.drop_data_index()
+
+    assert dvc.diff("v2") == default_result | {
+        "not in cache": [{"path": dir_path, "hash": new_digest}],
+    }
+    assert dvc.diff("v1") == default_result | {
+        "modified": [
+            {"path": dir_path, "hash": {"old": old_digest, "new": new_digest}}
+        ],
+        "not in cache": [{"path": dir_path, "hash": old_digest}],
+    }
+    assert dvc.diff("v2", "v1") == default_result | {
+        "modified": [
+            {"path": dir_path, "hash": {"old": new_digest, "new": old_digest}}
+        ],
+    }
+    assert dvc.diff("v1", "v2") == default_result | {
+        "modified": [
+            {"path": dir_path, "hash": {"old": old_digest, "new": new_digest}}
+        ],
+    }
+    assert dvc.diff() == default_result | {
+        "not in cache": [{"path": dir_path, "hash": new_digest}],
+    }
+
+    remove(str(tmp_dir / "dir"))
+    assert dvc.diff() == default_result | {
+        "deleted": [{"path": dir_path, "hash": new_digest}],
+        "not in cache": [{"path": dir_path, "hash": new_digest}],
+    }
+
+
 @pytest.mark.parametrize("delete_data", [True, False])
 def test_deleted(tmp_dir, scm, dvc, delete_data):
     tmp_dir.dvc_gen("file", "text", commit="add file")
@@ -256,54 +330,6 @@ def test_directories(tmp_dir, scm, dvc):
     }
 
 
-def test_diff_no_cache(tmp_dir, scm, dvc):
-    tmp_dir.dvc_gen({"dir": {"file": "file content"}}, commit="first")
-    scm.tag("v1")
-
-    tmp_dir.dvc_gen({"dir": {"file": "modified file content"}}, commit="second")
-    scm.tag("v2")
-
-    remove(dvc.cache.local.path)
-
-    diff = dvc.diff("v1")
-    assert diff["added"] == []
-    assert diff["deleted"] == []
-    assert first(diff["modified"])["path"] == os.path.join("dir", "")
-    assert diff["renamed"] == []
-    assert diff["not in cache"] == [
-        {
-            "hash": "61d9c7f006e1ae7138fec5e574676ee2.dir",
-            "path": os.path.join("dir", ""),
-        }
-    ]
-
-    diff = dvc.diff("v1", "v2")
-    assert diff["added"] == []
-    assert diff["deleted"] == []
-    assert diff["renamed"] == []
-    assert first(diff["modified"])["path"] == os.path.join("dir", "")
-    assert diff["not in cache"] == []
-
-    (tmp_dir / "dir" / "file").unlink()
-    remove(str(tmp_dir / "dir"))
-    diff = dvc.diff()
-    assert diff["added"] == []
-    assert diff["deleted"] == [
-        {
-            "path": os.path.join("dir", ""),
-            "hash": "f0f7a307d223921557c929f944bf5303.dir",
-        }
-    ]
-    assert diff["renamed"] == []
-    assert diff["modified"] == []
-    assert diff["not in cache"] == [
-        {
-            "path": os.path.join("dir", ""),
-            "hash": "f0f7a307d223921557c929f944bf5303.dir",
-        }
-    ]
-
-
 def test_diff_dirty(tmp_dir, scm, dvc):
     tmp_dir.dvc_gen(
         {"file": "file_content", "dir": {"dir_file1": "dir file content"}},