From 848de1bf39083f3c8f7b8239460ea447f03903fd Mon Sep 17 00:00:00 2001 From: Andrew White Date: Fri, 24 Jul 2026 09:07:35 -0500 Subject: [PATCH] fix: avoid 'None' prefix in SlurmExecutor job names SlurmExecutor.job_name_prefix defaults to None, so f-strings in alloc() and srun() produced job names like 'Noneinteractive'. Use so the prefix is omitted when unset, matching the existing handling in other methods. Adds regression tests for both alloc() and srun(). Signed-off-by: Andrew White --- nemo_run/core/execution/slurm.py | 4 +- test/core/execution/test_slurm_job_name.py | 75 ++++++++++++++++++++++ 2 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 test/core/execution/test_slurm_job_name.py diff --git a/nemo_run/core/execution/slurm.py b/nemo_run/core/execution/slurm.py index 2a794e30..139755d9 100644 --- a/nemo_run/core/execution/slurm.py +++ b/nemo_run/core/execution/slurm.py @@ -434,7 +434,7 @@ def info(self) -> str: return f"{self.__class__.__qualname__} on {self.tunnel.key}" def alloc(self, job_name="interactive"): - self.job_name = f"{self.job_name_prefix}{job_name}" + self.job_name = f"{self.job_name_prefix or ''}{job_name}" args = [ f"--{arg}={getattr(self, arg.replace('-', '_'))}" for arg in self.ALLOC_ARGS @@ -457,7 +457,7 @@ def srun( arg_dict=None, **kwargs, ): - self.job_name = f"{self.job_name_prefix}{job_name}" + self.job_name = f"{self.job_name_prefix or ''}{job_name}" _arg_dict = { arg: getattr(self, arg.replace("-", "_")) for arg in self.SRUN_ARGS diff --git a/test/core/execution/test_slurm_job_name.py b/test/core/execution/test_slurm_job_name.py new file mode 100644 index 00000000..8ce44f46 --- /dev/null +++ b/test/core/execution/test_slurm_job_name.py @@ -0,0 +1,75 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from unittest.mock import MagicMock, PropertyMock, patch + +from nemo_run.core.execution.slurm import SlurmExecutor +from nemo_run.core.tunnel.client import LocalTunnel + + +class TestSlurmJobNamePrefix: + def test_alloc_without_prefix_uses_job_name_only(self): + """alloc() must not prepend 'None' when job_name_prefix is unset.""" + executor = SlurmExecutor(account="test", tunnel=LocalTunnel(job_dir="/tmp")) + assert executor.job_name_prefix is None + + mock_slurm = MagicMock() + with patch.object( + SlurmExecutor, "slurm", new_callable=PropertyMock, return_value=mock_slurm + ): + executor.alloc(job_name="interactive") + + assert executor.job_name == "interactive" + + def test_alloc_with_prefix_prefixes_job_name(self): + """alloc() must prepend the configured prefix when set.""" + executor = SlurmExecutor( + account="test", tunnel=LocalTunnel(job_dir="/tmp"), job_name_prefix="nemo-" + ) + mock_slurm = MagicMock() + with patch.object( + SlurmExecutor, "slurm", new_callable=PropertyMock, return_value=mock_slurm + ): + executor.alloc(job_name="interactive") + + assert executor.job_name == "nemo-interactive" + + def test_srun_without_prefix_uses_job_name_only(self): + """srun() must not prepend 'None' when job_name_prefix is unset.""" + executor = SlurmExecutor(account="test", tunnel=LocalTunnel(job_dir="/tmp")) + assert executor.job_name_prefix is None + + mock_slurm = MagicMock() + with patch.object( + SlurmExecutor, "slurm", new_callable=PropertyMock, return_value=mock_slurm + ): + with patch.object(executor, "SRUN_ARGS", []): + executor.srun("echo hi", job_name="interactive") + + assert executor.job_name == "interactive" + + def test_srun_with_prefix_prefixes_job_name(self): + """srun() must prepend the configured prefix when set.""" + executor = SlurmExecutor( + account="test", tunnel=LocalTunnel(job_dir="/tmp"), job_name_prefix="nemo-" + ) + mock_slurm = MagicMock() + with patch.object( + SlurmExecutor, "slurm", new_callable=PropertyMock, return_value=mock_slurm + ): + with patch.object(executor, "SRUN_ARGS", []): + executor.srun("echo hi", job_name="interactive") + + assert executor.job_name == "nemo-interactive"