Correct ExecArgs Handling During RunSetting (#517)

Added `isinstance` check to RunSettings exe_args setter. Added
additional tests.

[ reviewed by @mellis13 ]
[ committed by @amandarichardsonn ]

doc/changelog.rst

@@ -18,6 +18,7 @@ To be released at some future point in time
 
 Description
 
+- ExecArgs handling correction
 - ReadTheDocs config file added and enabled on PRs
 - Enforce changelog updates
 - Remove deprecated SmartSim modules
@@ -28,6 +29,8 @@ Description
 
 Detailed Notes
 
+- Add checks and tests to ensure SmartSim users cannot initialize run settings
+  with a list of lists as the exe_args argument. (SmartSim-PR517_)
 - Add readthedocs configuration file and enable readthedocs builds
   on pull requests. Additionally added robots.txt file generation
   when readthedocs environment detected. (SmartSim-PR512_)
@@ -46,6 +49,7 @@ Detailed Notes
   (SmartSim-PR504_)
 - Update the generic `t.Any` typehints in Experiment API. (SmartSim-PR501_)
 
+.. _SmartSim-PR517: https://github.com/CrayLabs/SmartSim/pull/517
 .. _SmartSim-PR512: https://github.com/CrayLabs/SmartSim/pull/512
 .. _SmartSim-PR518: https://github.com/CrayLabs/SmartSim/pull/518
 .. _SmartSim-PR514: https://github.com/CrayLabs/SmartSim/pull/514

smartsim/settings/base.py

@@ -446,15 +446,8 @@ def add_exe_args(self, args: t.Union[str, t.List[str]]) -> None:
 
         :param args: executable arguments
         :type args: str | list[str]
-        :raises TypeError: if exe args are not strings
         """
-        if isinstance(args, str):
-            args = args.split()
-
-        for arg in args:
-            if not isinstance(arg, str):
-                raise TypeError("Executable arguments should be a list of str")
-
+        args = self._build_exe_args(args)
         self._exe_args.extend(args)
 
     def set(
@@ -533,26 +526,26 @@ def set(
 
     @staticmethod
     def _build_exe_args(exe_args: t.Optional[t.Union[str, t.List[str]]]) -> t.List[str]:
-        """Convert exe_args input to a desired collection format"""
-        if exe_args:
-            if isinstance(exe_args, str):
-                return exe_args.split()
-            if isinstance(exe_args, list):
-                exe_args = copy.deepcopy(exe_args)
-                plain_type = all(isinstance(arg, (str)) for arg in exe_args)
-                if not plain_type:
-                    nested_type = all(
-                        all(isinstance(arg, (str)) for arg in exe_args_list)
-                        for exe_args_list in exe_args
-                    )
-                    if not nested_type:
-                        raise TypeError(
-                            "Executable arguments were not list of str or str"
-                        )
-                    return exe_args
-                return exe_args
-            raise TypeError("Executable arguments were not list of str or str")
-        return []
+        """Check and convert exe_args input to a desired collection format"""
+        if not exe_args:
+            return []
+
+        if isinstance(exe_args, list):
+            exe_args = copy.deepcopy(exe_args)
+
+        if not (
+            isinstance(exe_args, str)
+            or (
+                isinstance(exe_args, list)
+                and all(isinstance(arg, str) for arg in exe_args)
+            )
+        ):
+            raise TypeError("Executable arguments were not a list of str or a str.")
+
+        if isinstance(exe_args, str):
+            return exe_args.split()
+
+        return exe_args
 
     def format_run_args(self) -> t.List[str]:
         """Return formatted run arguments

tests/test_run_settings.py

@@ -185,55 +185,34 @@ def test_add_exe_args_list_of_mixed():
         settings.add_exe_args(["1", "2", 3])
 
 
-def test_add_exe_args_space_delimited_string():
+def test_add_exe_args_list_of_lists():
     """Ensure that any non-string exe arg fails validation for all"""
     settings = RunSettings("python")
-    expected = ["1", "2", "3"]
-    settings.add_exe_args("1 2 3")
-
-    assert settings.exe_args == expected
-
-
-def test_add_exe_args_list_of_mixed_lists():
-    """Ensure that any non-string exe arg fails validation for all"""
-    settings = RunSettings("python")
-    with pytest.raises(TypeError) as type_error:
-        settings.add_exe_args([["1", "2", 3], ["4", "5", 6]])
-
-    assert "Executable arguments should be a list of str" in type_error.value.args
-
-
-def test_add_exe_args_list_of_mixed_lists_init():
-    """Ensure that any non-string exe arg fails validation for all"""
-    exe_args = [["1", "2", 3], ["4", "5", 6]]
-
-    with pytest.raises(TypeError) as type_error:
-        settings = RunSettings("python", exe_args=exe_args)
-
-    assert "Executable arguments were not list of str or str" in type_error.value.args
+    with pytest.raises(TypeError):
+        settings.add_exe_args(["1", "2", "3"], ["1", "2", "3"])
 
 
-def test_add_exe_args_list_of_str_lists_init():
-    """Ensure that list[list[str]] pass validation"""
+def test_init_exe_args_list_of_lists():
+    """Ensure that a list of lists exe arg fails validation"""
     exe_args = [["1", "2", "3"], ["4", "5", "6"]]
+    with pytest.raises(TypeError):
+        _ = RunSettings("python", exe_args=exe_args)
 
-    settings = RunSettings("python", exe_args=exe_args)
 
-    assert settings.exe_args == exe_args
+def test_init_exe_args_list_of_lists_mixed():
+    """Ensure that a list of lists exe arg fails validation"""
+    exe_args = [["1", "2", 3], ["4", "5", 6]]
+    with pytest.raises(TypeError):
+        _ = RunSettings("python", exe_args=exe_args)
 
 
-def test_add_exe_args_list_of_str_lists():
-    """Ensure that list[list[str]] fail validation when added via method"""
-    exe_args = [["1", "2", "3"], ["4", "5", "6"]]
-
+def test_add_exe_args_space_delimited_string():
+    """Ensure that any non-string exe arg fails validation for all"""
     settings = RunSettings("python")
+    expected = ["1", "2", "3"]
+    settings.add_exe_args("1 2 3")
 
-    with pytest.raises(TypeError) as type_error:
-        settings.add_exe_args(exe_args)
-
-    # NOTE that this behavior differs from sending constructor args like
-    # tested in test_add_exe_args_list_of_str_lists_init where it's allowed
-    assert "Executable arguments should be a list of str" in type_error.value.args
+    assert settings.exe_args == expected
 
 
 def test_format_run_args():