matt and amanda review comments

smartsim/_core/entrypoints/file_operations.py

@@ -35,65 +35,50 @@
 from distutils import dir_util  # pylint: disable=deprecated-module
 
 
-def _check_path(file_path: str) -> str:
-    """Given a user provided path-like str, find the actual path to
-        the directory or file and create a full path.
-
-    :param file_path: path to a specific file or directory
-    :raises FileNotFoundError: if file or directory does not exist
-    :return: full path to file or directory
-    """
-    full_path = os.path.abspath(file_path)
-    if os.path.isfile(full_path):
-        return full_path
-    if os.path.isdir(full_path):
-        return full_path
-    raise FileNotFoundError(f"File or Directory {file_path} not found")
-
-
 def move(parsed_args: argparse.Namespace) -> None:
-    """Move a file
+    """Move a source file or directory to another location. If dest is an
+    existing directory or a symlink to a directory, then the srouce will
+    be moved inside that directory. The destination path in that directory
+    must not already exist. If dest is an existing file, it will be overwritten.
 
     Sample usage:
         python _core/entrypoints/file_operations.py move
-        /absolute/file/src/path /absolute/file/dest/path
+        /absolute/file/source/path /absolute/file/dest/path
 
-        source path: Path to a source file to be copied
-        dest path: Path to a file to copy the contents from the source file into
+        /absolute/file/source/path: File or directory to be moved
+        /absolute/file/dest/path: Path to a file or directory location
     """
-    _check_path(parsed_args.source)
-    _check_path(parsed_args.dest)
     shutil.move(parsed_args.source, parsed_args.dest)
 
 
 def remove(parsed_args: argparse.Namespace) -> None:
-    """Write a python script that removes a file when executed.
+    """Remove a file or directory.
 
     Sample usage:
         python _core/entrypoints/file_operations.py remove
         /absolute/file/path
 
-        file path: Path to the file to be deleted
+        /absolute/file/path: Path to the file or directory to be deleted
     """
-    _check_path(parsed_args.to_remove)
-    os.remove(parsed_args.to_remove)
+    if os.path.isdir(parsed_args.to_remove):
+        os.rmdir(parsed_args.to_remove)
+    else:
+        os.remove(parsed_args.to_remove)
 
 
 def copy(parsed_args: argparse.Namespace) -> None:
-    """
-    Write a python script to copy the entity files and directories attached
-    to this entity into an entity directory
+    """Copy the contents from the source file into the dest file.
+    If source is a directory, copy the entire directory tree source to dest.
 
     Sample usage:
         python _core/entrypoints/file_operations.py copy
-        /absolute/file/src/path /absolute/file/dest/path
+        /absolute/file/source/path /absolute/file/dest/path
 
-        source path: Path to directory, or path to file to copy into an entity directory
-        dest path: Path to destination directory or path to destination file to copy
+        /absolute/file/source/path: Path to directory, or path to file to
+            copy to a new location
+        /absolute/file/dest/path: Path to destination directory or path to
+            destination file
     """
-    _check_path(parsed_args.source)
-    _check_path(parsed_args.dest)
-
     if os.path.isdir(parsed_args.source):
         dir_util.copy_tree(parsed_args.source, parsed_args.dest)
     else:
@@ -103,46 +88,40 @@ def copy(parsed_args: argparse.Namespace) -> None:
 def symlink(parsed_args: argparse.Namespace) -> None:
     """
     Create a symbolic link pointing to the exisiting source file
-    named link
+    named link.
 
     Sample usage:
         python _core/entrypoints/file_operations.py symlink
-        /absolute/file/src/path /absolute/file/dest/path
+        /absolute/file/source/path /absolute/file/dest/path
 
-        source path: the exisiting source path
-        dest path: target name where the symlink will be created.
+        /absolute/file/source/path: the exisiting source path
+        /absolute/file/dest/path: target name where the symlink will be created.
     """
-    _check_path(parsed_args.source)
-
     os.symlink(parsed_args.source, parsed_args.dest)
 
 
 def configure(parsed_args: argparse.Namespace) -> None:
-    """Write a python script to set, search and replace the tagged parameters for the
-    configure operation within tagged files attached to an entity.
+    """Write a python script to set, search and replace the tagged parameters
+    for the configure operation within tagged files attached to an entity.
 
-    User-formatted files can be attached using the `configure` argument. These files
-    will be modified during ``Application`` generation to replace tagged sections in the
-    user-formatted files with values from the `params` initializer argument used during
-    ``Application`` creation:
+    User-formatted files can be attached using the `configure` argument.
+    These files will be modified during ``Application`` generation to replace
+    tagged sections in the user-formatted files with values from the `params`
+    initializer argument used during ``Application`` creation:
 
     Sample usage:
         python _core/entrypoints/file_operations.py configure
-        /absolute/file/src/path /absolute/file/dest/path tag_deliminator param_dict
+        /absolute/file/source/pat /absolute/file/dest/path tag_deliminator param_dict
 
-        source path: The tagged files the search and replace operations to be
-            performed upon
-        dest path: Optional destination for configured files to be written to
+        /absolute/file/source/path: The tagged files the search and replace operations
+        to be performed upon
+        /absolute/file/dest/path: Optional destination for configured files to be
+        written to.
         tag_delimiter: tag for the configure operation to search for, defaults to
             semi-colon e.g. ";"
         param_dict: A dict of parameter names and values set for the file
 
     """
-
-    _check_path(parsed_args.source)
-    if parsed_args.dest:
-        _check_path(parsed_args.dest)
-
     tag_delimiter = ";"
     if parsed_args.tag_delimiter:
         tag_delimiter = parsed_args.tag_delimiter

tests/test_file_operations.py

@@ -46,9 +46,9 @@ def test_symlink_files(test_dir):
     # Set source directory and file
     source = pathlib.Path(test_dir) / "sym_source"
     os.mkdir(source)
-    source_file = pathlib.Path(source) / "sym_source.txt"
+    source_file = source / "sym_source.txt"
     with open(source_file, "w+", encoding="utf-8") as dummy_file:
-        dummy_file.write("")
+        dummy_file.write("dummy")
 
     # Set path to be the destination directory
     entity_path = os.path.join(test_dir, "entity_name")
@@ -125,7 +125,11 @@ def test_copy_op_file(test_dir):
     # Execute copy
     file_operations.copy(ns)
 
-    # clean up
+    # Assert files were copied over
+    with open(dest_file, "r", encoding="utf-8") as dummy_file:
+        assert dummy_file.read() == "dummy"
+
+    # Clean up
     os.remove(pathlib.Path(to_copy) / "copy_file.txt")
     os.rmdir(pathlib.Path(test_dir) / "to_copy")
 
@@ -160,6 +164,15 @@ def test_copy_op_dirs(test_dir):
     # Execute copy
     file_operations.copy(ns)
 
+    # Assert dirs were copied over
+    entity_files_1 = pathlib.Path(entity_path) / "copy_file.txt"
+    with open(entity_files_1, "r", encoding="utf-8") as dummy_file:
+        assert dummy_file.read() == "dummy1"
+
+    entity_files_2 = pathlib.Path(entity_path) / "copy_file_2.txt"
+    with open(entity_files_2, "r", encoding="utf-8") as dummy_file:
+        assert dummy_file.read() == "dummy2"
+
     # Clean up
     os.remove(pathlib.Path(to_copy) / "copy_file.txt")
     os.remove(pathlib.Path(to_copy) / "copy_file_2.txt")
@@ -187,9 +200,9 @@ def test_copy_op_bad_source_file(test_dir):
 
     with pytest.raises(FileNotFoundError) as ex:
         file_operations.copy(ns)
-    assert f"File or Directory {bad_path} not found" in ex.value.args[0]
+    assert f"No such file or directory" in ex.value.args
 
-    # clean up
+    # Clean up
     os.rmdir(pathlib.Path(test_dir) / "to_copy")
     os.rmdir(pathlib.Path(test_dir) / "entity_name")
 
@@ -215,7 +228,7 @@ def test_copy_op_bad_dest_path(test_dir):
 
     with pytest.raises(FileNotFoundError) as ex:
         file_operations.copy(ns)
-    assert f"File or Directory {bad_path} not found" in ex.value.args[0]
+    assert f"No such file or directory" in ex.value.args
 
     # clean up
     os.remove(pathlib.Path(to_copy) / "copy_file.txt")
@@ -263,7 +276,7 @@ def test_move_op(test_dir):
     os.rmdir(dest_dir)
 
 
-def test_remove_op(test_dir):
+def test_remove_op_file(test_dir):
     """Test the operation to delete a file"""
 
     # Make a test file with dummy text
@@ -286,6 +299,24 @@ def test_remove_op(test_dir):
     assert not osp.exists(to_del)
 
 
+def test_remove_op_dir(test_dir):
+    """Test the operation to delete a directory"""
+
+    # Make a test file with dummy text
+    to_del = pathlib.Path(test_dir) / "dir_del"
+    os.mkdir(to_del)
+
+    parser = get_parser()
+    cmd = f"remove {to_del}"
+    args = cmd.split()
+    ns = parser.parse_args(args)
+
+    file_operations.remove(ns)
+
+    # Assert directory has been deleted
+    assert not osp.exists(to_del)
+
+
 def test_remove_op_bad_path(test_dir):
     """Test that FileNotFoundError is raised when a bad path is given to the
     soperation to delete a file"""
@@ -299,7 +330,7 @@ def test_remove_op_bad_path(test_dir):
 
     with pytest.raises(FileNotFoundError) as ex:
         file_operations.remove(ns)
-    assert f"File or Directory {to_del} not found" in ex.value.args[0]
+    assert f"No such file or directory" in ex.value.args
 
 
 @pytest.mark.parametrize(