fix: checking for double nested strings in gres and gpu settings (#251)

users ought to get an informative error message, if they request
resources with `gres="'gpu:1'"` instead of `gres="gpu:1!` (or similarly
for the `gpu` resource).

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **Bug Fixes**
- Enhanced error feedback for GPU and GRES argument validations,
providing more specific messages to distinguish nested quotation issues
from general format errors.
  
- **Tests**
- Added tests to verify error handling for nested string formats in GRES
and GPU model arguments, ensuring accurate error messages are raised.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

Author: cmeesters

snakemake_executor_plugin_slurm/utils.py

@@ -58,6 +58,9 @@ def set_gres_string(job: JobExecutorInterface) -> str:
     gres_re = re.compile(r"^[a-zA-Z0-9_]+(:[a-zA-Z0-9_]+)?:\d+$")
     # gpu model arguments can be of type "string"
     gpu_model_re = re.compile(r"^[a-zA-Z0-9_]+$")
+    # any arguments should not start and end with ticks or
+    # quotation marks:
+    string_check = re.compile(r"^[^'\"].*[^'\"]$")
     # The Snakemake resources can be only be of type "int",
     # hence no further regex is needed.
 
@@ -81,20 +84,36 @@ def set_gres_string(job: JobExecutorInterface) -> str:
         # Validate GRES format (e.g., "gpu:1", "gpu:tesla:2")
         gres = job.resources.gres
         if not gres_re.match(gres):
-            raise WorkflowError(
-                f"Invalid GRES format: {gres}. Expected format: "
-                "'<name>:<number>' or '<name>:<type>:<number>' "
-                "(e.g., 'gpu:1' or 'gpu:tesla:2')"
-            )
+            if not string_check.match(gres):
+                raise WorkflowError(
+                    "GRES format should not be a nested string (start "
+                    "and end with ticks or quotation marks). "
+                    "Expected format: "
+                    "'<name>:<number>' or '<name>:<type>:<number>' "
+                    "(e.g., 'gpu:1' or 'gpu:tesla:2')"
+                )
+            else:
+                raise WorkflowError(
+                    f"Invalid GRES format: {gres}. Expected format: "
+                    "'<name>:<number>' or '<name>:<type>:<number>' "
+                    "(e.g., 'gpu:1' or 'gpu:tesla:2')"
+                )
         return f" --gres={job.resources.gres}"
 
     if gpu_model and gpu_string:
         # validate GPU model format
         if not gpu_model_re.match(gpu_model):
-            raise WorkflowError(
-                f"Invalid GPU model format: {gpu_model}."
-                " Expected format: '<name>' (e.g., 'tesla')"
-            )
+            if not string_check.match(gpu_model):
+                raise WorkflowError(
+                    "GPU model format should not be a nested string (start "
+                    "and end with ticks or quotation marks). "
+                    "Expected format: '<name>' (e.g., 'tesla')"
+                )
+            else:
+                raise WorkflowError(
+                    f"Invalid GPU model format: {gpu_model}."
+                    " Expected format: '<name>' (e.g., 'tesla')"
+                )
         return f" --gpus={gpu_model}:{gpu_string}"
     elif gpu_model and not gpu_string:
         raise WorkflowError("GPU model is set, but no GPU number is given")

tests/tests.py

@@ -215,6 +215,42 @@ def test_both_gres_and_gpu_set(self, mock_job):
             ):
                 set_gres_string(job)
 
+    def test_nested_string_raise(self, mock_job):
+        """Test error case when gres is a nested string."""
+        job = mock_job(gres="'gpu:1'")
+        # Patch subprocess.Popen to simulate job submission
+        with patch("subprocess.Popen") as mock_popen:
+            # Configure the mock to simulate successful submission
+            process_mock = MagicMock()
+            process_mock.communicate.return_value = ("123", "")
+            process_mock.returncode = 0
+            mock_popen.return_value = process_mock
+
+            # Ensure the error is raised when both GRES and GPU are set
+            with pytest.raises(
+                WorkflowError,
+                match="GRES format should not be a nested string",
+            ):
+                set_gres_string(job)
+
+    def test_gpu_model_nested_string_raise(self, mock_job):
+        """Test error case when gpu_model is a nested string."""
+        job = mock_job(gpu_model="'tesla'", gpu="2")
+        # Patch subprocess.Popen to simulate job submission
+        with patch("subprocess.Popen") as mock_popen:
+            # Configure the mock to simulate successful submission
+            process_mock = MagicMock()
+            process_mock.communicate.return_value = ("123", "")
+            process_mock.returncode = 0
+            mock_popen.return_value = process_mock
+
+            # Ensure the error is raised when both GRES and GPU are set
+            with pytest.raises(
+                WorkflowError,
+                match="GPU model format should not be a nested string",
+            ):
+                set_gres_string(job)
+
 
 class TestSLURMResources(TestWorkflows):
     """