Repository: iterative/dvc

Issue:
run: running the same command twice on a callback stage doesn't reproduce
version: `0.35.7+8ef52f`

----
```bash
❯ dvc run -o hello 'echo hello > hello'
Running command:
	echo hello > hello
Saving 'hello' to cache '.dvc/cache'.
Saving information to 'hello.dvc'.

❯ dvc run -o hello 'echo hello > hello'
Stage is cached, skipping.
```

The second run should have run the command again, as it is a callback stage.


Discussion:

Patch:
diff --git a/dvc/stage.py b/dvc/stage.py
--- a/dvc/stage.py
+++ b/dvc/stage.py
@@ -534,7 +534,11 @@ def create(
 
         if validate_state:
             if os.path.exists(path):
-                if not ignore_build_cache and stage.is_cached:
+                if (
+                    not ignore_build_cache
+                    and stage.is_cached
+                    and not stage.is_callback
+                ):
                     logger.info("Stage is cached, skipping.")
                     return None
 


Test patch:
diff --git a/tests/func/test_run.py b/tests/func/test_run.py
--- a/tests/func/test_run.py
+++ b/tests/func/test_run.py
@@ -671,21 +671,22 @@ def test_cwd_is_ignored(self):
         )
 
 
-class TestRunDeterministicBase(TestDvc):
-    def setUp(self):
-        super(TestRunDeterministicBase, self).setUp()
+class DeterministicRunBaseFixture(object):
+    def __init__(self, repo_dir, dvc_repo):
         self.out_file = "out"
         self.stage_file = self.out_file + ".dvc"
-        self.cmd = "python {} {} {}".format(self.CODE, self.FOO, self.out_file)
-        self.deps = [self.FOO, self.CODE]
+        self.cmd = "python {} {} {}".format(
+            repo_dir.CODE, repo_dir.FOO, self.out_file
+        )
+        self.deps = [repo_dir.FOO, repo_dir.CODE]
         self.outs = [self.out_file]
         self.overwrite = False
         self.ignore_build_cache = False
+        self.dvc_repo = dvc_repo
+        self.stage = None
 
-        self._run()
-
-    def _run(self):
-        self.stage = self.dvc.run(
+    def run(self):
+        self.stage = self.dvc_repo.run(
             cmd=self.cmd,
             fname=self.stage_file,
             overwrite=self.overwrite,
@@ -693,70 +694,69 @@ def _run(self):
             deps=self.deps,
             outs=self.outs,
         )
+        return self.stage
 
 
-class TestRunDeterministic(TestRunDeterministicBase):
-    def test(self):
-        self._run()
+@pytest.fixture
+def deterministic_run(dvc_repo, repo_dir):
+    run_base = DeterministicRunBaseFixture(repo_dir, dvc_repo)
+    run_base.run()
+    yield run_base
 
 
-class TestRunDeterministicOverwrite(TestRunDeterministicBase):
-    def test(self):
-        self.overwrite = True
-        self.ignore_build_cache = True
-        self._run()
+def test_run_deterministic(deterministic_run):
+    deterministic_run.run()
 
 
-class TestRunDeterministicCallback(TestRunDeterministicBase):
-    def test(self):
-        self.stage.remove()
-        self.deps = []
-        self._run()
-        self._run()
+def test_run_deterministic_overwrite(deterministic_run):
+    deterministic_run.overwrite = True
+    deterministic_run.ignore_build_cache = True
+    deterministic_run.run()
 
 
-class TestRunDeterministicChangedDep(TestRunDeterministicBase):
-    def test(self):
-        os.unlink(self.FOO)
-        shutil.copy(self.BAR, self.FOO)
-        with self.assertRaises(StageFileAlreadyExistsError):
-            self._run()
+def test_run_deterministic_callback(deterministic_run):
+    deterministic_run.stage.remove()
+    deterministic_run.deps = []
+    deterministic_run.run()
+    with mock.patch("dvc.prompt.confirm", return_value=True):
+        assert deterministic_run.run()
 
 
-class TestRunDeterministicChangedDepsList(TestRunDeterministicBase):
-    def test(self):
-        self.deps = [self.BAR, self.CODE]
-        with self.assertRaises(StageFileAlreadyExistsError):
-            self._run()
+def test_run_deterministic_changed_dep(deterministic_run, repo_dir):
+    os.unlink(repo_dir.FOO)
+    shutil.copy(repo_dir.BAR, repo_dir.FOO)
+    with pytest.raises(StageFileAlreadyExistsError):
+        deterministic_run.run()
 
 
-class TestRunDeterministicNewDep(TestRunDeterministicBase):
-    def test(self):
-        self.deps = [self.FOO, self.BAR, self.CODE]
-        with self.assertRaises(StageFileAlreadyExistsError):
-            self._run()
+def test_run_deterministic_changed_deps_list(deterministic_run, repo_dir):
+    deterministic_run.deps = [repo_dir.BAR, repo_dir.CODE]
+    with pytest.raises(StageFileAlreadyExistsError):
+        deterministic_run.run()
 
 
-class TestRunDeterministicRemoveDep(TestRunDeterministicBase):
-    def test(self):
-        self.deps = [self.CODE]
-        with self.assertRaises(StageFileAlreadyExistsError):
-            self._run()
+def test_run_deterministic_new_dep(deterministic_run, repo_dir):
+    deterministic_run.deps = [repo_dir.FOO, repo_dir.BAR, repo_dir.CODE]
+    with pytest.raises(StageFileAlreadyExistsError):
+        deterministic_run.run()
 
 
-class TestRunDeterministicChangedOut(TestRunDeterministicBase):
-    def test(self):
-        os.unlink(self.out_file)
-        self.out_file_mtime = None
-        with self.assertRaises(StageFileAlreadyExistsError):
-            self._run()
+def test_run_deterministic_remove_dep(deterministic_run, repo_dir):
+    deterministic_run.deps = [repo_dir.CODE]
+    with pytest.raises(StageFileAlreadyExistsError):
+        deterministic_run.run()
 
 
-class TestRunDeterministicChangedCmd(TestRunDeterministicBase):
-    def test(self):
-        self.cmd += " arg"
-        with self.assertRaises(StageFileAlreadyExistsError):
-            self._run()
+def test_run_deterministic_changed_out(deterministic_run):
+    os.unlink(deterministic_run.out_file)
+    with pytest.raises(StageFileAlreadyExistsError):
+        deterministic_run.run()
+
+
+def test_run_deterministic_changed_cmd(deterministic_run):
+    deterministic_run.cmd += " arg"
+    with pytest.raises(StageFileAlreadyExistsError):
+        deterministic_run.run()
 
 
 class TestRunCommit(TestDvc):