Skip to content

Commit a39246e

Browse files
Application.files refactor (#732)
This PR refactors how files are added to an Application. [ reviewed by @MattToast @mellis13 ] [ committed by @amandarichardsonn ]
1 parent 2cbd3be commit a39246e

File tree

21 files changed

+1102
-564
lines changed

21 files changed

+1102
-564
lines changed

conftest.py

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@
4040
import typing as t
4141
import uuid
4242
import warnings
43+
from glob import glob
44+
from os import path as osp
4345
from collections import defaultdict
4446
from dataclasses import dataclass
4547
from subprocess import run
@@ -53,6 +55,8 @@
5355
from smartsim._core.config.config import Config
5456
from smartsim._core.launcher.dragon.dragon_connector import DragonConnector
5557
from smartsim._core.launcher.dragon.dragon_launcher import DragonLauncher
58+
from smartsim._core.generation.operations.operations import ConfigureOperation, CopyOperation, SymlinkOperation
59+
from smartsim._core.generation.generator import Generator
5660
from smartsim._core.utils.telemetry.telemetry import JobEntity
5761
from smartsim.database import FeatureStore
5862
from smartsim.entity import Application
@@ -469,6 +473,58 @@ def check_output_dir() -> None:
469473
def fsutils() -> t.Type[FSUtils]:
470474
return FSUtils
471475

476+
@pytest.fixture
477+
def files(fileutils):
478+
path_to_files = fileutils.get_test_conf_path(
479+
osp.join("generator_files", "easy", "correct/")
480+
)
481+
list_of_files_strs = glob(path_to_files + "/*")
482+
yield [pathlib.Path(str_path) for str_path in list_of_files_strs]
483+
484+
485+
@pytest.fixture
486+
def directory(fileutils):
487+
directory = fileutils.get_test_conf_path(
488+
osp.join("generator_files", "easy", "correct/")
489+
)
490+
yield [pathlib.Path(directory)]
491+
492+
493+
@pytest.fixture(params=["files", "directory"])
494+
def source(request):
495+
yield request.getfixturevalue(request.param)
496+
497+
498+
@pytest.fixture
499+
def mock_src(test_dir: str):
500+
"""Fixture to create a mock source path."""
501+
return pathlib.Path(test_dir) / pathlib.Path("mock_src")
502+
503+
504+
@pytest.fixture
505+
def mock_dest():
506+
"""Fixture to create a mock destination path."""
507+
return pathlib.Path("mock_dest")
508+
509+
510+
@pytest.fixture
511+
def copy_operation(mock_src: pathlib.Path, mock_dest: pathlib.Path):
512+
"""Fixture to create a CopyOperation object."""
513+
return CopyOperation(src=mock_src, dest=mock_dest)
514+
515+
516+
@pytest.fixture
517+
def symlink_operation(mock_src: pathlib.Path, mock_dest: pathlib.Path):
518+
"""Fixture to create a CopyOperation object."""
519+
return SymlinkOperation(src=mock_src, dest=mock_dest)
520+
521+
522+
@pytest.fixture
523+
def configure_operation(mock_src: pathlib.Path, mock_dest: pathlib.Path):
524+
"""Fixture to create a Configure object."""
525+
return ConfigureOperation(
526+
src=mock_src, dest=mock_dest, file_parameters={"FOO": "BAR"}
527+
)
472528

473529
class FSUtils:
474530
@staticmethod

smartsim/_core/commands/command.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,10 @@ class Command(MutableSequence[str]):
3535
"""Basic container for command information"""
3636

3737
def __init__(self, command: t.List[str]) -> None:
38+
if not command:
39+
raise TypeError("Command list cannot be empty")
40+
if not all(isinstance(item, str) for item in command):
41+
raise TypeError("All items in the command list must be strings")
3842
"""Command constructor"""
3943
self._command = command
4044

@@ -66,17 +70,15 @@ def __setitem__(
6670
"""Set the command at the specified index."""
6771
if isinstance(idx, int):
6872
if not isinstance(value, str):
69-
raise ValueError(
73+
raise TypeError(
7074
"Value must be of type `str` when assigning to an index"
7175
)
7276
self._command[idx] = deepcopy(value)
7377
return
7478
if not isinstance(value, list) or not all(
7579
isinstance(item, str) for item in value
7680
):
77-
raise ValueError(
78-
"Value must be a list of strings when assigning to a slice"
79-
)
81+
raise TypeError("Value must be a list of strings when assigning to a slice")
8082
self._command[idx] = (deepcopy(val) for val in value)
8183

8284
def __delitem__(self, idx: t.Union[int, slice]) -> None:

smartsim/_core/commands/command_list.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,20 +69,20 @@ def __setitem__(
6969
"""Set the Commands at the specified index."""
7070
if isinstance(idx, int):
7171
if not isinstance(value, Command):
72-
raise ValueError(
72+
raise TypeError(
7373
"Value must be of type `Command` when assigning to an index"
7474
)
7575
self._commands[idx] = deepcopy(value)
7676
return
7777
if not isinstance(value, list):
78-
raise ValueError(
78+
raise TypeError(
7979
"Value must be a list of Commands when assigning to a slice"
8080
)
8181
for sublist in value:
8282
if not isinstance(sublist.command, list) or not all(
8383
isinstance(item, str) for item in sublist.command
8484
):
85-
raise ValueError(
85+
raise TypeError(
8686
"Value sublists must be a list of Commands when assigning to a slice"
8787
)
8888
self._commands[idx] = (deepcopy(val) for val in value)

smartsim/_core/entrypoints/file_operations.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -154,9 +154,9 @@ def copy(parsed_args: argparse.Namespace) -> None:
154154
/absolute/file/dest/path: Path to destination directory or path to
155155
destination file
156156
--dirs_exist_ok: if the flag is included, the copying operation will
157-
continue if the destination directory and files alrady exist,
157+
continue if the destination directory and files already exist,
158158
and will be overwritten by corresponding files. If the flag is
159-
not includedm and the destination file already exists, a
159+
not included and the destination file already exists, a
160160
FileExistsError will be raised
161161
"""
162162
if os.path.isdir(parsed_args.source):
@@ -226,7 +226,6 @@ def configure(parsed_args: argparse.Namespace) -> None:
226226
for file_name in filenames:
227227
src_file = os.path.join(dirpath, file_name)
228228
dst_file = os.path.join(new_dir_dest, file_name)
229-
print(type(substitutions))
230229
_process_file(substitutions, src_file, dst_file)
231230
else:
232231
dst_file = parsed_args.dest / os.path.basename(parsed_args.source)

0 commit comments

Comments
 (0)