From bb04488ebdab2e3f05927e3b0b990969c8bd0bfb Mon Sep 17 00:00:00 2001 From: Julia Putko Date: Thu, 17 Oct 2024 14:23:51 -0700 Subject: [PATCH] more value checking, move tests to launcher specific tests --- smartsim/settings/arguments/batch/lsf.py | 3 ++ smartsim/settings/arguments/batch/pbs.py | 7 +++- smartsim/settings/arguments/batch/slurm.py | 11 +++++- .../test_settings/test_batchSettings.py | 38 ------------------- .../test_settings/test_lsfScheduler.py | 30 +++++++++++++++ .../test_settings/test_pbsScheduler.py | 18 +++++++++ .../test_settings/test_slurmScheduler.py | 24 ++++++++++++ 7 files changed, 90 insertions(+), 41 deletions(-) diff --git a/smartsim/settings/arguments/batch/lsf.py b/smartsim/settings/arguments/batch/lsf.py index 6daed05d0b..7289a5dd33 100644 --- a/smartsim/settings/arguments/batch/lsf.py +++ b/smartsim/settings/arguments/batch/lsf.py @@ -57,8 +57,11 @@ def set_walltime(self, walltime: str) -> None: :param walltime: Time in hh:mm format, e.g. "10:00" for 10 hours, if time is supplied in hh:mm:ss format, seconds will be ignored and walltime will be set as ``hh:mm`` + :raises TypeError: if not type str """ # For compatibility with other launchers, as explained in docstring + if not isinstance(walltime, str): + raise TypeError("walltime argument was not of type str") if walltime: if len(walltime.split(":")) > 2: walltime = ":".join(walltime.split(":")[:2]) diff --git a/smartsim/settings/arguments/batch/pbs.py b/smartsim/settings/arguments/batch/pbs.py index de9194ecb3..ba248d1fb1 100644 --- a/smartsim/settings/arguments/batch/pbs.py +++ b/smartsim/settings/arguments/batch/pbs.py @@ -95,7 +95,10 @@ def set_walltime(self, walltime: str) -> None: :param walltime: wall time :raises ValueError: if walltime format is invalid + :raises TypeError: if not type str """ + if not isinstance(walltime, str): + raise TypeError("walltime argument was not of type str") pattern = r"^\d{2}:\d{2}:\d{2}$" if walltime and re.match(pattern, walltime): self.set("walltime", walltime) @@ -110,7 +113,7 @@ def set_queue(self, queue: str) -> None: """ if not isinstance(queue, str): raise TypeError("queue argument was not of type str") - self.set("q", str(queue)) + self.set("q", queue) def set_ncpus(self, num_cpus: int) -> None: """Set the number of cpus obtained in each node. @@ -134,7 +137,7 @@ def set_account(self, account: str) -> None: """ if not isinstance(account, str): raise TypeError("account argument was not of type str") - self.set("A", str(account)) + self.set("A", account) def format_batch_args(self) -> t.List[str]: """Get the formatted batch arguments for a preview diff --git a/smartsim/settings/arguments/batch/slurm.py b/smartsim/settings/arguments/batch/slurm.py index bb71b161ac..51a99f7bb9 100644 --- a/smartsim/settings/arguments/batch/slurm.py +++ b/smartsim/settings/arguments/batch/slurm.py @@ -57,7 +57,10 @@ def set_walltime(self, walltime: str) -> None: :param walltime: wall time :raises ValueError: if walltime format is invalid + :raises TypeError: if not str """ + if not isinstance(walltime, str): + raise TypeError("walltime argument was not of type str") pattern = r"^\d{2}:\d{2}:\d{2}$" if walltime and re.match(pattern, walltime): self.set("time", str(walltime)) @@ -94,8 +97,11 @@ def set_partition(self, partition: str) -> None: This sets ``--partition``. :param partition: partition name + :raises TypeError: if not a str """ - self.set("partition", str(partition)) + if not isinstance(partition, str): + raise TypeError("partition argument was not of type str") + self.set("partition", partition) def set_queue(self, queue: str) -> None: """alias for set_partition @@ -115,7 +121,10 @@ def set_cpus_per_task(self, cpus_per_task: int) -> None: This sets ``--cpus-per-task`` :param num_cpus: number of cpus to use per task + :raises TypeError: if not int """ + if not isinstance(cpus_per_task, int): + raise TypeError("cpus_per_task argument was not of type int") self.set("cpus-per-task", str(cpus_per_task)) def set_hostlist(self, host_list: t.Union[str, t.List[str]]) -> None: diff --git a/tests/temp_tests/test_settings/test_batchSettings.py b/tests/temp_tests/test_settings/test_batchSettings.py index 5db3c16dde..01e61214bf 100644 --- a/tests/temp_tests/test_settings/test_batchSettings.py +++ b/tests/temp_tests/test_settings/test_batchSettings.py @@ -159,41 +159,3 @@ def test_batch_arguments_type_set_queue(scheduler): bs = BatchSettings(batch_scheduler=scheduler, env_vars={"ENV": "VAR"}) with pytest.raises(TypeError, match="queue argument was not of type str"): bs.batch_args.set_queue(27) - - -@pytest.mark.parametrize( - "scheduler", - [ - pytest.param("slurm", id="slurm scheduler"), - pytest.param("lsf", id="bsub scheduler"), - pytest.param("pbs", id="qsub scheduler"), - ], -) -def test_batch_arguments_type_set_hostlist(scheduler): - bs = BatchSettings(batch_scheduler=scheduler, env_vars={"ENV": "VAR"}) - with pytest.raises(TypeError, match="host_list argument must be a list of strings"): - bs.batch_args.set_hostlist([25, 37]) - - -def test_batch_arguments_type_set_ncpus(): - bs = BatchSettings(batch_scheduler="pbs", env_vars={"ENV": "VAR"}) - with pytest.raises(TypeError, match="num_cpus argument was not of type int"): - bs.batch_args.set_ncpus("invalid") - - -def test_batch_arguments_type_set_smts(): - bs = BatchSettings(batch_scheduler="lsf", env_vars={"ENV": "VAR"}) - with pytest.raises(TypeError, match="smts argument was not of type int"): - bs.batch_args.set_smts("invalid") - - -def test_batch_arguments_type_set_project(): - bs = BatchSettings(batch_scheduler="lsf", env_vars={"ENV": "VAR"}) - with pytest.raises(TypeError, match="project argument was not of type str"): - bs.batch_args.set_project(27) - - -def test_batch_arguments_type_set_tasks(): - bs = BatchSettings(batch_scheduler="lsf", env_vars={"ENV": "VAR"}) - with pytest.raises(TypeError, match="tasks argument was not of type int"): - bs.batch_args.set_tasks("invalid") diff --git a/tests/temp_tests/test_settings/test_lsfScheduler.py b/tests/temp_tests/test_settings/test_lsfScheduler.py index 5e6b7fd0c4..81ab25c8ad 100644 --- a/tests/temp_tests/test_settings/test_lsfScheduler.py +++ b/tests/temp_tests/test_settings/test_lsfScheduler.py @@ -75,3 +75,33 @@ def test_create_bsub(): lsfScheduler.batch_args.set_queue("default") args = lsfScheduler.format_batch_args() assert args == ["-core_isolation", "-nnodes", "1", "-W", "10:10", "-q", "default"] + + +def test_batch_arguments_type_set_hostlist(scheduler): + bs = BatchSettings(batch_scheduler="lsf", env_vars={"ENV": "VAR"}) + with pytest.raises(TypeError, match="host_list argument must be a list of strings"): + bs.batch_args.set_hostlist([25, 37]) + + +def test_batch_arguments_type_set_smts(): + bs = BatchSettings(batch_scheduler="lsf", env_vars={"ENV": "VAR"}) + with pytest.raises(TypeError, match="smts argument was not of type int"): + bs.batch_args.set_smts("invalid") + + +def test_batch_arguments_type_set_project(): + bs = BatchSettings(batch_scheduler="lsf", env_vars={"ENV": "VAR"}) + with pytest.raises(TypeError, match="project argument was not of type str"): + bs.batch_args.set_project(27) + + +def test_batch_arguments_type_set_tasks(): + bs = BatchSettings(batch_scheduler="lsf", env_vars={"ENV": "VAR"}) + with pytest.raises(TypeError, match="tasks argument was not of type int"): + bs.batch_args.set_tasks("invalid") + + +def test_batch_arguments_type_set_walltime(): + bs = BatchSettings(batch_scheduler="lsf", env_vars={"ENV": "VAR"}) + with pytest.raises(TypeError, match="walltime argument was not of type str"): + bs.batch_args.set_walltime(27) diff --git a/tests/temp_tests/test_settings/test_pbsScheduler.py b/tests/temp_tests/test_settings/test_pbsScheduler.py index 36fde6776d..718311a322 100644 --- a/tests/temp_tests/test_settings/test_pbsScheduler.py +++ b/tests/temp_tests/test_settings/test_pbsScheduler.py @@ -86,3 +86,21 @@ def test_format_pbs_batch_args(): "-A", "myproject", ] + + +def test_batch_arguments_type_set_hostlist(scheduler): + bs = BatchSettings(batch_scheduler="pbs", env_vars={"ENV": "VAR"}) + with pytest.raises(TypeError, match="host_list argument must be a list of strings"): + bs.batch_args.set_hostlist([25, 37]) + + +def test_batch_arguments_type_set_ncpus(): + bs = BatchSettings(batch_scheduler="pbs", env_vars={"ENV": "VAR"}) + with pytest.raises(TypeError, match="num_cpus argument was not of type int"): + bs.batch_args.set_ncpus("invalid") + + +def test_batch_arguments_type_set_walltime(): + bs = BatchSettings(batch_scheduler="pbs", env_vars={"ENV": "VAR"}) + with pytest.raises(TypeError, match="walltime argument was not of type str"): + bs.batch_args.set_walltime(27) diff --git a/tests/temp_tests/test_settings/test_slurmScheduler.py b/tests/temp_tests/test_settings/test_slurmScheduler.py index 8ab489cc8b..b0f32b7a28 100644 --- a/tests/temp_tests/test_settings/test_slurmScheduler.py +++ b/tests/temp_tests/test_settings/test_slurmScheduler.py @@ -134,3 +134,27 @@ def test_sbatch_manual(): formatted = slurmScheduler.format_batch_args() result = ["--nodes=5", "--account=A3531", "--time=10:00:00"] assert formatted == result + + +def test_batch_arguments_type_set_walltime(): + bs = BatchSettings(batch_scheduler="slurm", env_vars={"ENV": "VAR"}) + with pytest.raises(TypeError, match="walltime argument was not of type str"): + bs.batch_args.set_walltime(27) + + +def test_batch_arguments_type_set_cpus_per_task(): + bs = BatchSettings(batch_scheduler="slurm", env_vars={"ENV": "VAR"}) + with pytest.raises(TypeError, match="cpus_per_task argument was not of type int"): + bs.batch_args.set_cpus_per_task("invalid") + + +def test_batch_arguments_type_set_partition(): + bs = BatchSettings(batch_scheduler="slurm", env_vars={"ENV": "VAR"}) + with pytest.raises(TypeError, match="partition argument was not of type str"): + bs.batch_args.set_partition(27) + + +def test_batch_arguments_type_set_hostlist(scheduler): + bs = BatchSettings(batch_scheduler="slurm", env_vars={"ENV": "VAR"}) + with pytest.raises(TypeError, match="host_list argument must be a list of strings"): + bs.batch_args.set_hostlist([25, 37])