From 2b3c30cb0c68bc9fb8934bd331de07cc6d8d9fe1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pawe=C5=82=20Redzy=C5=84ski?= <pawelredzynski@gmail.com>
Date: Tue, 22 Sep 2020 21:53:55 +0200
Subject: [PATCH] metrics: accept any viable target in DvcRepo (#4590)

---
 dvc/repo/metrics/show.py        | 35 ++++++++++++++++--------------
 tests/func/metrics/test_diff.py | 14 ++++++++++++
 tests/func/metrics/test_show.py | 38 +++++++++++++++++++++++++++++++++
 3 files changed, 71 insertions(+), 16 deletions(-)

diff --git a/dvc/repo/metrics/show.py b/dvc/repo/metrics/show.py
index 0913a59719..d0e7b07ae5 100644
--- a/dvc/repo/metrics/show.py
+++ b/dvc/repo/metrics/show.py
@@ -11,28 +11,31 @@
 
 
 def _collect_metrics(repo, targets, recursive):
-    metrics = set()
 
+    if targets:
+        target_infos = [
+            PathInfo(os.path.abspath(target)) for target in targets
+        ]
+        tree = RepoTree(repo)
+
+        rec_files = []
+        if recursive:
+            for target_info in target_infos:
+                if tree.isdir(target_info):
+                    rec_files.extend(list(tree.walk_files(target_info)))
+
+        result = [t for t in target_infos if tree.isfile(t)]
+        result.extend(rec_files)
+
+        return result
+
+    metrics = set()
     for stage in repo.stages:
         for out in stage.outs:
             if not out.metric:
                 continue
-
             metrics.add(out.path_info)
-
-    if not targets:
-        return list(metrics)
-
-    target_infos = [PathInfo(os.path.abspath(target)) for target in targets]
-
-    def _filter(path_info):
-        for info in target_infos:
-            func = path_info.isin_or_eq if recursive else path_info.__eq__
-            if func(info):
-                return True
-        return False
-
-    return list(filter(_filter, metrics))
+    return list(metrics)
 
 
 def _extract_metrics(metrics, path, rev):
diff --git a/tests/func/metrics/test_diff.py b/tests/func/metrics/test_diff.py
index c7ea99bf33..8d00c2d814 100644
--- a/tests/func/metrics/test_diff.py
+++ b/tests/func/metrics/test_diff.py
@@ -195,3 +195,17 @@ def _gen(val):
         "Path    Metric    Old      New      Change\n"
         "m.yaml  foo       1.23457  3.45679  2.22222"
     )
+
+
+def test_metrics_diff_non_metrics(tmp_dir, scm, dvc):
+    def _gen(val):
+        tmp_dir.scm_gen({"some_file.yaml": f"foo: {val}"}, commit=str(val))
+
+    _gen(1)
+    _gen(2)
+    _gen(3)
+
+    result = dvc.metrics.diff(targets=["some_file.yaml"], a_rev="HEAD~2")
+    assert result == {
+        "some_file.yaml": {"foo": {"old": 1, "new": 3, "diff": 2}}
+    }
diff --git a/tests/func/metrics/test_show.py b/tests/func/metrics/test_show.py
index 69414b4e96..bc93f4aefe 100644
--- a/tests/func/metrics/test_show.py
+++ b/tests/func/metrics/test_show.py
@@ -110,3 +110,41 @@ def test_missing_cache(tmp_dir, dvc, run_copy_metrics):
     remove(stage.outs[0].cache_path)
 
     assert dvc.metrics.show() == {"": {"metrics.yaml": 1.1}}
+
+
+def test_show_non_metric(tmp_dir, dvc):
+    tmp_dir.gen("metrics.yaml", "foo: 1.1")
+
+    assert dvc.metrics.show(targets=["metrics.yaml"]) == {
+        "": {"metrics.yaml": {"foo": 1.1}}
+    }
+
+
+def test_show_non_metric_branch(tmp_dir, scm, dvc):
+    tmp_dir.scm_gen("metrics.yaml", "foo: 1.1", commit="init")
+    with tmp_dir.branch("branch", new=True):
+        tmp_dir.scm_gen("metrics.yaml", "foo: 2.2", commit="other")
+
+    assert dvc.metrics.show(targets=["metrics.yaml"], revs=["branch"]) == {
+        "workspace": {"metrics.yaml": {"foo": 1.1}},
+        "branch": {"metrics.yaml": {"foo": 2.2}},
+    }
+
+
+def test_non_metric_and_recurisve_show(tmp_dir, dvc, run_copy_metrics):
+    tmp_dir.gen(
+        {"metrics_t.yaml": "foo: 1.1", "metrics": {"metric1.yaml": "bar: 1.2"}}
+    )
+
+    metric2 = os.fspath(tmp_dir / "metrics" / "metric2.yaml")
+    run_copy_metrics("metrics_t.yaml", metric2, metrics=[metric2])
+
+    assert dvc.metrics.show(
+        targets=["metrics_t.yaml", "metrics"], recursive=True
+    ) == {
+        "": {
+            os.path.join("metrics", "metric1.yaml"): {"bar": 1.2},
+            os.path.join("metrics", "metric2.yaml"): {"foo": 1.1},
+            "metrics_t.yaml": {"foo": 1.1},
+        }
+    }