From bf68fbffec0954c9b39af993c43518970bad89ab Mon Sep 17 00:00:00 2001 From: pared Date: Tue, 26 Mar 2019 15:52:43 +0100 Subject: [PATCH] run: make run respect persist flag --- dvc/stage.py | 7 ++--- tests/test_run.py | 79 +++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 72 insertions(+), 14 deletions(-) diff --git a/dvc/stage.py b/dvc/stage.py index 0a6f9ccc8d..cec811136d 100644 --- a/dvc/stage.py +++ b/dvc/stage.py @@ -286,10 +286,6 @@ def reproduce( if not self.changed() and not force: return None - if (self.cmd or self.is_import) and not self.locked and not dry: - # Removing outputs only if we actually have command to reproduce - self.remove_outs(ignore_remove=False) - msg = ( "Going to reproduce '{stage}'. " "Are you sure you want to continue?".format(stage=self.relpath) @@ -734,6 +730,9 @@ def _run(self): raise StageCmdFailedError(self) def run(self, dry=False, resume=False, no_commit=False, force=False): + if self.cmd and not self.locked and not dry: + self.remove_outs(ignore_remove=False, force=False) + if self.locked: logger.info( "Verifying outputs in locked stage '{stage}'".format( diff --git a/tests/test_run.py b/tests/test_run.py index 5b0694abc0..448e3803a2 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -379,7 +379,7 @@ def test(self): ) self.assertEqual(ret, 0) self.assertFalse(os.access(self.FOO, os.W_OK)) - self.assertEqual(open(self.FOO, "r").read(), "foofoo") + self.assertEqual(open(self.FOO, "r").read(), "foo") ret = main( [ @@ -397,7 +397,7 @@ def test(self): ) self.assertEqual(ret, 0) self.assertFalse(os.access(self.FOO, os.W_OK)) - self.assertEqual(open(self.FOO, "r").read(), "foofoofoo") + self.assertEqual(open(self.FOO, "r").read(), "foo") class TestRunUnprotectOutsSymlink(TestDvc): @@ -405,7 +405,6 @@ def test(self): with open(self.CODE, "w+") as fobj: fobj.write("import sys\n") fobj.write("import os\n") - fobj.write("assert os.path.exists(sys.argv[1])\n") fobj.write("with open(sys.argv[1], 'a+') as fobj:\n") fobj.write(" fobj.write('foo')\n") @@ -431,7 +430,7 @@ def test(self): self.assertEqual(ret, 0) self.assertFalse(os.access(self.FOO, os.W_OK)) self.assertTrue(System.is_symlink(self.FOO)) - self.assertEqual(open(self.FOO, "r").read(), "foofoo") + self.assertEqual(open(self.FOO, "r").read(), "foo") ret = main( [ @@ -450,7 +449,7 @@ def test(self): self.assertEqual(ret, 0) self.assertFalse(os.access(self.FOO, os.W_OK)) self.assertTrue(System.is_symlink(self.FOO)) - self.assertEqual(open(self.FOO, "r").read(), "foofoofoo") + self.assertEqual(open(self.FOO, "r").read(), "foo") class TestRunUnprotectOutsHardlink(TestDvc): @@ -458,7 +457,6 @@ def test(self): with open(self.CODE, "w+") as fobj: fobj.write("import sys\n") fobj.write("import os\n") - fobj.write("assert os.path.exists(sys.argv[1])\n") fobj.write("with open(sys.argv[1], 'a+') as fobj:\n") fobj.write(" fobj.write('foo')\n") @@ -484,7 +482,7 @@ def test(self): self.assertEqual(ret, 0) self.assertFalse(os.access(self.FOO, os.W_OK)) self.assertTrue(System.is_hardlink(self.FOO)) - self.assertEqual(open(self.FOO, "r").read(), "foofoo") + self.assertEqual(open(self.FOO, "r").read(), "foo") ret = main( [ @@ -503,7 +501,7 @@ def test(self): self.assertEqual(ret, 0) self.assertFalse(os.access(self.FOO, os.W_OK)) self.assertTrue(System.is_hardlink(self.FOO)) - self.assertEqual(open(self.FOO, "r").read(), "foofoofoo") + self.assertEqual(open(self.FOO, "r").read(), "foo") class TestCmdRunOverwrite(TestDvc): @@ -755,13 +753,13 @@ class TestRunCommit(TestDvc): def test(self): fname = "test" ret = main( - ["run", "-o", self.FOO, "--no-commit", "echo", "test", ">", fname] + ["run", "-o", fname, "--no-commit", "echo", "test", ">", fname] ) self.assertEqual(ret, 0) self.assertTrue(os.path.isfile(fname)) self.assertEqual(len(os.listdir(self.dvc.cache.local.cache_dir)), 0) - ret = main(["commit", self.FOO + ".dvc"]) + ret = main(["commit", fname + ".dvc"]) self.assertEqual(ret, 0) self.assertTrue(os.path.isfile(fname)) self.assertEqual(len(os.listdir(self.dvc.cache.local.cache_dir)), 1) @@ -861,3 +859,64 @@ def test(self): "command with non overlapping outs paths.", error_output, ) + + +class TestRerunWithSameOutputs(TestDvc): + def _read_content_only(self, path): + with open(path, "r") as fobj: + return [line.rstrip() for line in fobj.readlines()] + + @property + def _outs_command(self): + raise NotImplementedError + + def _run_twice_with_same_outputs(self): + ret = main( + [ + "run", + "--outs", + self.FOO, + "echo {} > {}".format(self.FOO_CONTENTS, self.FOO), + ] + ) + self.assertEqual(0, ret) + + output_file_content = self._read_content_only(self.FOO) + self.assertEqual([self.FOO_CONTENTS], output_file_content) + + ret = main( + [ + "run", + self._outs_command, + self.FOO, + "--overwrite-dvcfile", + "echo {} >> {}".format(self.BAR_CONTENTS, self.FOO), + ] + ) + self.assertEqual(0, ret) + + +class TestNewRunShouldRemoveOutsOnNoPersist(TestRerunWithSameOutputs): + def test(self): + self._run_twice_with_same_outputs() + + output_file_content = self._read_content_only(self.FOO) + self.assertEqual([self.BAR_CONTENTS], output_file_content) + + @property + def _outs_command(self): + return "--outs" + + +class TestNewRunShouldNotRemoveOutsOnPersist(TestRerunWithSameOutputs): + def test(self): + self._run_twice_with_same_outputs() + + output_file_content = self._read_content_only(self.FOO) + self.assertEqual( + [self.FOO_CONTENTS, self.BAR_CONTENTS], output_file_content + ) + + @property + def _outs_command(self): + return "--outs-persist"