From fe1d7e9f1da76ca5d8fcb320a0b1884d5f26eec5 Mon Sep 17 00:00:00 2001
From: Saugat Pachhai <suagatchhetri@outlook.com>
Date: Thu, 9 Jul 2020 18:29:26 +0000
Subject: [PATCH] params: fix skipping of params dvc.lock when it's a falsy
 value (#4185)

The falsy values were being skipped when merging params values
from dvc.yaml and dvc.lock, showing the values as always changed.

These values were never added to `info`, but were found in
`self.params` making them appear as changed.

Fixes #4184
---
 dvc/dependency/param.py              |  5 ++---
 tests/func/test_run_multistage.py    |  2 +-
 tests/unit/dependency/test_params.py | 30 +++++++++++++++++++++++-----
 3 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/dvc/dependency/param.py b/dvc/dependency/param.py
index 4df6087f52..12860e3475 100644
--- a/dvc/dependency/param.py
+++ b/dvc/dependency/param.py
@@ -45,9 +45,8 @@ def fill_values(self, values=None):
         if not values:
             return
         for param in self.params:
-            value = values.get(param)
-            if value:
-                self.info[param] = value
+            if param in values:
+                self.info[param] = values[param]
 
     def save(self):
         super().save()
diff --git a/tests/func/test_run_multistage.py b/tests/func/test_run_multistage.py
index daf9c0c2ee..056052342a 100644
--- a/tests/func/test_run_multistage.py
+++ b/tests/func/test_run_multistage.py
@@ -244,7 +244,7 @@ def test_run_params_default(tmp_dir, dvc):
         params=["nested.nested1.nested2"],
         cmd="cat params.yaml",
     )
-    isinstance(stage.deps[0], ParamsDependency)
+    assert isinstance(stage.deps[0], ParamsDependency)
     assert stage.deps[0].params == ["nested.nested1.nested2"]
 
     lockfile = stage.dvcfile._lockfile
diff --git a/tests/unit/dependency/test_params.py b/tests/unit/dependency/test_params.py
index b4edf53d6e..2667feabb6 100644
--- a/tests/unit/dependency/test_params.py
+++ b/tests/unit/dependency/test_params.py
@@ -4,6 +4,7 @@
 from dvc.dependency import ParamsDependency, loadd_from, loads_params
 from dvc.dependency.param import BadParamFileError, MissingParamsError
 from dvc.stage import Stage
+from dvc.utils.yaml import load_yaml
 
 PARAMS = {
     "foo": 1,
@@ -11,6 +12,7 @@
     "baz": "str",
     "qux": None,
 }
+DEFAULT_PARAMS_FILE = ParamsDependency.DEFAULT_PARAMS_FILE
 
 
 def test_loads_params(dvc):
@@ -63,7 +65,7 @@ def test_loadd_from(dvc):
 def test_dumpd_with_info(dvc):
     dep = ParamsDependency(Stage(dvc), None, PARAMS)
     assert dep.dumpd() == {
-        "path": "params.yaml",
+        "path": DEFAULT_PARAMS_FILE,
         "params": PARAMS,
     }
 
@@ -71,7 +73,7 @@ def test_dumpd_with_info(dvc):
 def test_dumpd_without_info(dvc):
     dep = ParamsDependency(Stage(dvc), None, list(PARAMS.keys()))
     assert dep.dumpd() == {
-        "path": "params.yaml",
+        "path": DEFAULT_PARAMS_FILE,
         "params": list(PARAMS.keys()),
     }
 
@@ -82,7 +84,7 @@ def test_read_params_nonexistent_file(dvc):
 
 
 def test_read_params_unsupported_format(tmp_dir, dvc):
-    tmp_dir.gen("params.yaml", b"\0\1\2\3\4\5\6\7")
+    tmp_dir.gen(DEFAULT_PARAMS_FILE, b"\0\1\2\3\4\5\6\7")
     dep = ParamsDependency(Stage(dvc), None, ["foo"])
     with pytest.raises(BadParamFileError):
         dep.read_params()
@@ -90,7 +92,8 @@ def test_read_params_unsupported_format(tmp_dir, dvc):
 
 def test_read_params_nested(tmp_dir, dvc):
     tmp_dir.gen(
-        "params.yaml", yaml.dump({"some": {"path": {"foo": ["val1", "val2"]}}})
+        DEFAULT_PARAMS_FILE,
+        yaml.dump({"some": {"path": {"foo": ["val1", "val2"]}}}),
     )
     dep = ParamsDependency(Stage(dvc), None, ["some.path.foo"])
     assert dep.read_params() == {"some.path.foo": ["val1", "val2"]}
@@ -103,7 +106,24 @@ def test_save_info_missing_config(dvc):
 
 
 def test_save_info_missing_param(tmp_dir, dvc):
-    tmp_dir.gen("params.yaml", "bar: baz")
+    tmp_dir.gen(DEFAULT_PARAMS_FILE, "bar: baz")
     dep = ParamsDependency(Stage(dvc), None, ["foo"])
     with pytest.raises(MissingParamsError):
         dep.save_info()
+
+
+@pytest.mark.parametrize(
+    "param_value",
+    ["", "false", "[]", "{}", "null", "no", "off"]
+    # we use pyyaml to load params.yaml, which only supports YAML 1.1
+    # so, some of the above are boolean values
+)
+def test_params_with_false_values(tmp_dir, dvc, param_value):
+    key = "param"
+    dep = ParamsDependency(Stage(dvc), DEFAULT_PARAMS_FILE, [key])
+    (tmp_dir / DEFAULT_PARAMS_FILE).write_text(f"{key}: {param_value}")
+
+    dep.fill_values(load_yaml(DEFAULT_PARAMS_FILE))
+
+    with dvc.state:
+        assert dep.status() == {}