RepoTree: work with dirty repos (#4005)

Currently `RepoTree.open` will throw FileNotFoundError if you have a
file that is tracked by dvc, but which has no md5 or a cache for which
is missing, even though you have the file itself in your workspace.

This corresponds very nicely to the `--no-commit` or `--no-exec`
scenario, where you might not want to commit your artifacts to cache
just yet.

Fixes #3974

dvc/repo/tree.py

@@ -61,6 +61,10 @@ class DvcTree(BaseTree):
         except OutputNotFoundError as exc:
             raise FileNotFoundError from exc
 
+        # NOTE: this handles both dirty and checkout-ed out at the same time
+        if self.repo.tree.exists(path):
+            return self.repo.tree.open(path, mode=mode, encoding=encoding)
+
         if len(outs) != 1 or (
             outs[0].is_dir_checksum and path == outs[0].path_info
         ):
@@ -252,13 +256,9 @@ class RepoTree(BaseTree):
             encoding = None
 
         if self.dvctree and self.dvctree.exists(path):
-            try:
-                return self.dvctree.open(
-                    path, mode=mode, encoding=encoding, **kwargs
-                )
-            except FileNotFoundError:
-                if self.isdvc(path):
-                    raise
+            return self.dvctree.open(
+                path, mode=mode, encoding=encoding, **kwargs
+            )
         return self.repo.tree.open(path, mode=mode, encoding=encoding)
 
     def exists(self, path):

tests/func/metrics/test_diff.py

@@ -153,3 +153,20 @@ def test_no_commits(tmp_dir):
     assert Git().no_commits
 
     assert Repo.init().metrics.diff() == {}
+
+
+def test_metrics_diff_dirty(tmp_dir, scm, dvc, run_copy_metrics):
+    def _gen(val):
+        tmp_dir.gen({"m_temp.yaml": str(val)})
+        run_copy_metrics("m_temp.yaml", "m.yaml", metrics=["m.yaml"])
+        dvc.scm.commit(str(val))
+
+    _gen(1)
+    _gen(2)
+    _gen(3)
+
+    tmp_dir.gen({"m.yaml": "4"})
+
+    expected = {"m.yaml": {"": {"old": 3, "new": 4, "diff": 1}}}
+
+    assert dvc.metrics.diff() == expected

tests/func/params/test_diff.py

@@ -24,6 +24,20 @@ def test_diff(tmp_dir, scm, dvc):
     }
 
 
+def test_diff_dirty(tmp_dir, scm, dvc):
+    tmp_dir.gen("params.yaml", "foo: bar")
+    dvc.run(cmd="echo params.yaml", params=["foo"], single_stage=True)
+    scm.add(["params.yaml", "Dvcfile"])
+    scm.commit("bar")
+
+    tmp_dir.scm_gen("params.yaml", "foo: baz", commit="baz")
+    tmp_dir.gen("params.yaml", "foo: qux")
+
+    assert dvc.params.diff() == {
+        "params.yaml": {"foo": {"old": "baz", "new": "qux"}}
+    }
+
+
 def test_diff_new(tmp_dir, scm, dvc):
     tmp_dir.gen("params.yaml", "foo: bar")
     dvc.run(cmd="echo params.yaml", params=["foo"], single_stage=True)

tests/func/plots/test_diff.py

@@ -39,3 +39,17 @@ def test_diff_dirty(tmp_dir, scm, dvc, run_copy_metrics):
     ]
     assert plot_content["encoding"]["x"]["field"] == PlotData.INDEX_FIELD
     assert plot_content["encoding"]["y"]["field"] == "y"
+
+    _write_json(tmp_dir, [{"y": 7}, {"y": 8}], "metric.json")
+
+    plot_string = dvc.plots.diff(props={"fields": {"y"}})["metric.json"]
+
+    plot_content = json.loads(plot_string)
+    assert plot_content["data"]["values"] == [
+        {"y": 3, PlotData.INDEX_FIELD: 0, "rev": "HEAD"},
+        {"y": 5, PlotData.INDEX_FIELD: 1, "rev": "HEAD"},
+        {"y": 7, PlotData.INDEX_FIELD: 0, "rev": "workspace"},
+        {"y": 8, PlotData.INDEX_FIELD: 1, "rev": "workspace"},
+    ]
+    assert plot_content["encoding"]["x"]["field"] == PlotData.INDEX_FIELD
+    assert plot_content["encoding"]["y"]["field"] == "y"

tests/func/test_api.py

@@ -114,6 +114,10 @@ def test_missing(remote_url, tmp_dir, dvc):
     # Remove cache to make foo missing
     remove(dvc.cache.local.cache_dir)
 
+    api.read("foo")
+
+    remove("foo")
+
     with pytest.raises(FileMissingError):
         api.read("foo")
 

tests/func/test_update.py

@@ -52,9 +52,9 @@ def test_update_import_after_remote_updates_to_dvc(tmp_dir, dvc, erepo_dir):
     new_rev = None
     with erepo_dir.branch("branch", new=False), erepo_dir.chdir():
         erepo_dir.scm.repo.index.remove(["version"])
-        erepo_dir.dvc_gen("version", "updated")
-        erepo_dir.scm.add(["version", "version.dvc"])
-        erepo_dir.scm.commit("upgrade to DVC tracking")
+        erepo_dir.dvc_gen(
+            "version", "updated", commit="upgrade to DVC tracking"
+        )
         new_rev = erepo_dir.scm.get_rev()
 
     assert old_rev != new_rev

tests/unit/repo/test_repo_tree.py

@@ -26,6 +26,24 @@ def test_open(tmp_dir, dvc):
         assert fobj.read() == "foo"
 
 
+def test_open_dirty_hash(tmp_dir, dvc):
+    tmp_dir.dvc_gen("file", "file")
+    (tmp_dir / "file").write_text("something")
+
+    tree = RepoTree(dvc)
+    with tree.open("file", "r") as fobj:
+        assert fobj.read() == "something"
+
+
+def test_open_dirty_no_hash(tmp_dir, dvc):
+    tmp_dir.gen("file", "file")
+    (tmp_dir / "file.dvc").write_text("outs:\n- path: file\n")
+
+    tree = RepoTree(dvc)
+    with tree.open("file", "r") as fobj:
+        assert fobj.read() == "file"
+
+
 def test_open_in_history(tmp_dir, scm, dvc):
     tmp_dir.gen("foo", "foo")
     dvc.add("foo")

tests/unit/repo/test_tree.py

@@ -26,6 +26,24 @@ def test_open(tmp_dir, dvc):
         assert fobj.read() == "foo"
 
 
+def test_open_dirty_hash(tmp_dir, dvc):
+    tmp_dir.dvc_gen("file", "file")
+    (tmp_dir / "file").write_text("something")
+
+    tree = DvcTree(dvc)
+    with tree.open("file", "r") as fobj:
+        assert fobj.read() == "something"
+
+
+def test_open_dirty_no_hash(tmp_dir, dvc):
+    tmp_dir.gen("file", "file")
+    (tmp_dir / "file.dvc").write_text("outs:\n- path: file\n")
+
+    tree = DvcTree(dvc)
+    with tree.open("file", "r") as fobj:
+        assert fobj.read() == "file"
+
+
 def test_open_in_history(tmp_dir, scm, dvc):
     tmp_dir.gen("foo", "foo")
     dvc.add("foo")