From e26db30f6039a82f00a54475a0dc9a34b7861937 Mon Sep 17 00:00:00 2001 From: Ivan Shcheklein Date: Thu, 28 Feb 2019 21:43:14 -0800 Subject: [PATCH] add more tests for wdir/fname new logic --- dvc/command/run.py | 4 +-- dvc/scm/git.py | 3 +- dvc/stage.py | 14 +++++++-- tests/basic_env.py | 1 + tests/test_repro.py | 40 +++++++++++++++++++++++- tests/test_run.py | 65 +++++++++++++++++++++++++++++++++++++-- tests/test_stage.py | 23 ++++++++++++++ tests/unit/command/run.py | 63 +++++++++++++++++++++++++++++++++++++ 8 files changed, 204 insertions(+), 9 deletions(-) create mode 100644 tests/unit/command/run.py diff --git a/dvc/command/run.py b/dvc/command/run.py index d16c9f7a2..1dd99bbe5 100644 --- a/dvc/command/run.py +++ b/dvc/command/run.py @@ -147,9 +147,7 @@ def add_parser(subparsers, parent_parser): "--yes", action="store_true", default=False, - help="(OBSOLETED, use --overwrite-dvcfile instead) Automatic 'yes' " - "answer to all prompts. E.g. when '.dvc' file exists and dvc " - "asks if you want to overwrite it.", + help="Deprecated, use --overwrite-dvcfile instead", ) run_parser.add_argument( "--overwrite-dvcfile", diff --git a/dvc/scm/git.py b/dvc/scm/git.py index 0c9ea835c..3e93490cb 100644 --- a/dvc/scm/git.py +++ b/dvc/scm/git.py @@ -75,7 +75,8 @@ class Git(Base): ignore_list = [] if os.path.exists(gitignore): - ignore_list = open(gitignore, "r").readlines() + with open(gitignore, "r") as f: + ignore_list = f.readlines() filtered = list( filter(lambda x: x.strip() == entry.strip(), ignore_list) ) diff --git a/dvc/stage.py b/dvc/stage.py index e8328b2bf..bbb2364c3 100644 --- a/dvc/stage.py +++ b/dvc/stage.py @@ -74,6 +74,12 @@ class StagePathNotFoundError(DvcException): super(StagePathNotFoundError, self).__init__(msg.format(path)) +class StagePathNotDirectoryError(DvcException): + def __init__(self, path): + msg = "stage working or file path '{}' is not directory" + super(StagePathNotDirectoryError, self).__init__(msg.format(path)) + + class StageCommitError(DvcException): pass @@ -331,11 +337,15 @@ class Stage(object): def _check_stage_path(repo, path): assert repo is not None - if not os.path.exists(os.path.realpath(path)): + real_path = os.path.realpath(path) + if not os.path.exists(real_path): raise StagePathNotFoundError(path) + if not os.path.isdir(real_path): + raise StagePathNotDirectoryError(path) + proj_dir = os.path.realpath(repo.root_dir) + os.path.sep - if not (os.path.realpath(path) + os.path.sep).startswith(proj_dir): + if not (real_path + os.path.sep).startswith(proj_dir): raise StagePathOutsideError(path) @property diff --git a/tests/basic_env.py b/tests/basic_env.py index ab48e2c73..c38e9bb74 100644 --- a/tests/basic_env.py +++ b/tests/basic_env.py @@ -1,6 +1,7 @@ import os import uuid import tempfile + from git import Repo from git.exc import GitCommandNotFound from unittest import TestCase diff --git a/tests/test_repro.py b/tests/test_repro.py index b3b38ac7b..89a40696d 100644 --- a/tests/test_repro.py +++ b/tests/test_repro.py @@ -732,7 +732,7 @@ class TestCmdRepro(TestReproChangedData): self.assertNotEqual(ret, 0) -class TestCmdReproChdir(TestDvc): +class TestCmdReproChdirCwdBackwardCompatible(TestDvc): def test(self): dname = "dir" os.mkdir(dname) @@ -770,6 +770,44 @@ class TestCmdReproChdir(TestDvc): self.assertTrue(filecmp.cmp(foo, bar, shallow=False)) +class TestCmdReproChdir(TestDvc): + def test(self): + dname = "dir" + os.mkdir(dname) + foo = os.path.join(dname, self.FOO) + bar = os.path.join(dname, self.BAR) + code = os.path.join(dname, self.CODE) + shutil.copyfile(self.FOO, foo) + shutil.copyfile(self.CODE, code) + + ret = main( + [ + "run", + "-f", + "{}/Dvcfile".format(dname), + "-w", + "{}".format(dname), + "-d", + self.FOO, + "-o", + self.BAR, + "python {} {} {}".format(self.CODE, self.FOO, self.BAR), + ] + ) + self.assertEqual(ret, 0) + self.assertTrue(os.path.isfile(foo)) + self.assertTrue(os.path.isfile(bar)) + self.assertTrue(filecmp.cmp(foo, bar, shallow=False)) + + os.unlink(bar) + + ret = main(["repro", "-c", dname]) + self.assertEqual(ret, 0) + self.assertTrue(os.path.isfile(foo)) + self.assertTrue(os.path.isfile(bar)) + self.assertTrue(filecmp.cmp(foo, bar, shallow=False)) + + class TestReproExternalBase(TestDvc): def should_test(self): return False diff --git a/tests/test_run.py b/tests/test_run.py index ab1b820a7..1cd56d28a 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -5,11 +5,12 @@ import mock import shutil import filecmp import subprocess +import yaml from dvc.main import main from dvc.utils import file_md5 from dvc.system import System -from dvc.stage import Stage, StagePathNotFoundError +from dvc.stage import Stage, StagePathNotFoundError, StagePathNotDirectoryError from dvc.stage import StageFileBadNameError, MissingDep from dvc.stage import StagePathOutsideError, StageFileAlreadyExistsError from dvc.exceptions import ( @@ -242,6 +243,14 @@ class TestRunBadWdir(TestDvc): path = os.path.join(self._root_dir, str(uuid.uuid4())) self.dvc.run(cmd="", wdir=path) + def test_not_dir(self): + with self.assertRaises(StagePathNotDirectoryError): + path = os.path.join(self._root_dir, str(uuid.uuid4())) + os.mkdir(path) + path = os.path.join(path, str(uuid.uuid4())) + open(path, "a").close() + self.dvc.run(cmd="", wdir=path) + class TestRunBadName(TestDvc): def test(self): @@ -316,7 +325,7 @@ class TestCmdRun(TestDvc): self.assertEqual(stage.cmd, 'echo "foo bar"') @mock.patch.object(subprocess, "Popen", side_effect=KeyboardInterrupt) - def test_keyboard_interrupt(self, mock_popen): + def test_keyboard_interrupt(self, _): ret = main(["run", "mycmd"]) self.assertEqual(ret, 252) @@ -605,6 +614,58 @@ class TestCmdRunCliMetrics(TestDvc): self.assertEqual(open("metrics.txt", "r").read().rstrip(), "test") +class TestCmdRunWorkingDirectory(TestDvc): + def test_default_wdir_is_written(self): + stage = self.dvc.run( + cmd="echo test > {}".format(self.FOO), outs=[self.FOO], wdir="." + ) + with open(stage.relpath, "r") as fobj: + d = yaml.safe_load(fobj) + self.assertEqual(d[Stage.PARAM_WDIR], ".") + + stage = self.dvc.run( + cmd="echo test > {}".format(self.BAR), outs=[self.BAR] + ) + with open(stage.relpath, "r") as fobj: + d = yaml.safe_load(fobj) + self.assertEqual(d[Stage.PARAM_WDIR], ".") + + def test_fname_changes_path_and_wdir(self): + dname = "dir" + os.mkdir(os.path.join(self._root_dir, dname)) + foo = os.path.join(dname, self.FOO) + fname = os.path.join(dname, "stage" + Stage.STAGE_FILE_SUFFIX) + stage = self.dvc.run( + cmd="echo test > {}".format(foo), outs=[foo], fname=fname + ) + self.assertEqual(stage.wdir, os.path.realpath(self._root_dir)) + self.assertEqual( + stage.path, os.path.join(os.path.realpath(self._root_dir), fname) + ) + + # Check that it is dumped properly (relative to fname) + with open(stage.relpath, "r") as fobj: + d = yaml.safe_load(fobj) + self.assertEqual(d[Stage.PARAM_WDIR], "..") + + def test_cwd_is_ignored(self): + dname = "dir" + os.mkdir(os.path.join(self._root_dir, dname)) + foo = os.path.join(dname, self.FOO) + fname = os.path.join("stage" + Stage.STAGE_FILE_SUFFIX) + stage = self.dvc.run( + cmd="echo test > {}".format(foo), + outs=[foo], + cwd=dname, + wdir=".", + fname=fname, + ) + self.assertEqual(stage.wdir, os.path.realpath(self._root_dir)) + self.assertEqual( + stage.path, os.path.join(os.path.realpath(self._root_dir), fname) + ) + + class TestRunDeterministicBase(TestDvc): def setUp(self): super(TestRunDeterministicBase, self).setUp() diff --git a/tests/test_stage.py b/tests/test_stage.py index 9f79268d1..b1ef539ee 100644 --- a/tests/test_stage.py +++ b/tests/test_stage.py @@ -1,3 +1,5 @@ +import yaml + from dvc.output.local import OutputLOCAL from dvc.remote.local import RemoteLOCAL from dvc.stage import Stage, StageFileFormatError @@ -91,3 +93,24 @@ class TestReload(TestDvc): d = load_stage_file(stage.relpath) self.assertEqual(d[stage.PARAM_MD5], md5) + + +class TestDefaultWorkingDirectory(TestDvc): + def test_ignored_in_checksum(self): + stage = self.dvc.run( + cmd="echo test > {}".format(self.FOO), + deps=[self.BAR], + outs=[self.FOO], + ) + + with open(stage.relpath, "r") as fobj: + d = yaml.safe_load(fobj) + self.assertEqual(d[stage.PARAM_WDIR], ".") + + del d[stage.PARAM_WDIR] + with open(stage.relpath, "w") as fobj: + yaml.safe_dump(d, fobj, default_flow_style=False) + + with self.dvc.state: + stage = Stage.load(self.dvc, stage.relpath) + self.assertFalse(stage.changed()) diff --git a/tests/unit/command/run.py b/tests/unit/command/run.py new file mode 100644 index 000000000..0dee369bd --- /dev/null +++ b/tests/unit/command/run.py @@ -0,0 +1,63 @@ +import mock + +from unittest import TestCase + +from dvc.main import main + + +class TestCli(TestCase): + @mock.patch("dvc.main.Analytics", autospec=True) + @mock.patch("dvc.cli.run.CmdRun", autospec=True) + def test_file_name(self, cmd_mock, _): + cmd_mock.return_value.run_cmd.return_value = 0 + ret = main(["run", "-f", "test/out.dvc", "echo"]) + self.assertEqual(ret, 0) + cmd_mock.assert_called_once() + args, kwargs = cmd_mock.call_args_list[0] + self.assertEqual(len(args), 1) + self.assertEqual(len(kwargs), 0) + self.assertEqual(args[0].file, "test/out.dvc") + self.assertSequenceEqual(args[0].command, ["echo"]) + cmd_mock.return_value.run_cmd.assert_called_once() + + @mock.patch("dvc.main.Analytics", autospec=True) + @mock.patch("dvc.cli.run.CmdRun", autospec=True) + def test_file_name_default(self, cmd_mock, _): + cmd_mock.return_value.run_cmd.return_value = 0 + ret = main(["run", "-d", "foo", "echo"]) + self.assertEqual(ret, 0) + cmd_mock.assert_called_once() + args, kwargs = cmd_mock.call_args_list[0] + self.assertEqual(len(args), 1) + self.assertEqual(len(kwargs), 0) + self.assertIsNone(args[0].file) + self.assertSequenceEqual(args[0].command, ["echo"]) + cmd_mock.return_value.run_cmd.assert_called_once() + + @mock.patch("dvc.main.Analytics", autospec=True) + @mock.patch("dvc.cli.run.CmdRun", autospec=True) + def test_wdir(self, cmd_mock, _): + cmd_mock.return_value.run_cmd.return_value = 0 + ret = main(["run", "--wdir", "test/", "echo"]) + self.assertEqual(ret, 0) + cmd_mock.assert_called_once() + args, kwargs = cmd_mock.call_args_list[0] + self.assertEqual(len(args), 1) + self.assertEqual(len(kwargs), 0) + self.assertEqual(args[0].wdir, "test/") + self.assertSequenceEqual(args[0].command, ["echo"]) + cmd_mock.return_value.run_cmd.assert_called_once() + + @mock.patch("dvc.main.Analytics", autospec=True) + @mock.patch("dvc.cli.run.CmdRun", autospec=True) + def test_wdir_default(self, cmd_mock, _): + cmd_mock.return_value.run_cmd.return_value = 0 + ret = main(["run", "echo"]) + self.assertEqual(ret, 0) + cmd_mock.assert_called_once() + args, kwargs = cmd_mock.call_args_list[0] + self.assertEqual(len(args), 1) + self.assertEqual(len(kwargs), 0) + self.assertEqual(args[0].wdir, None) + self.assertSequenceEqual(args[0].command, ["echo"]) + cmd_mock.return_value.run_cmd.assert_called_once()