From c0b50ecdbd5c7affc4596948044e1bfaa4595156 Mon Sep 17 00:00:00 2001 From: Anna Giroti Date: Wed, 15 Jul 2026 09:37:58 -0700 Subject: [PATCH 01/11] Log resolved run paths; clarify missing-log warning --- src/toil/common.py | 21 +++++++++++++++++++++ src/toil/leader.py | 9 +++++++-- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/toil/common.py b/src/toil/common.py index 0ba9a87542..abe3e13782 100644 --- a/src/toil/common.py +++ b/src/toil/common.py @@ -1154,11 +1154,32 @@ def __enter__(self) -> "Toil": self._start_time = time.time() self._inContextManager = True + self._log_resolved_paths(config) + # This will make sure `self.__exit__()` is called when we get a SIGTERM signal. signal.signal(signal.SIGTERM, lambda *_: sys.exit(1)) return self + def _log_resolved_paths(self, config: "Config") -> None: + """ + Log the resolved job store, work dir, and coordination dir paths + at INFO, once, so users can find them without tracing flag + precedence themselves. Skips the batch logs dir; the batch system + isn't built yet here, and grid systems log their own dir on startup. + """ + assert config.workflowID is not None + work_dir = self.getLocalWorkflowDir(config.workflowID, config.workDir) + coordination_dir = self.get_local_workflow_coordination_dir( + config.workflowID, config.workDir, config.coordination_dir + ) + logger.info( + "Resolved Toil run paths: job store: %s, work dir: %s, coordination dir: %s", + self.canonical_locator(config.jobStore), + work_dir, + coordination_dir, + ) + def __exit__( self, exc_type: type[BaseException] | None, diff --git a/src/toil/leader.py b/src/toil/leader.py index 52698db776..9712ff645e 100644 --- a/src/toil/leader.py +++ b/src/toil/leader.py @@ -1595,8 +1595,13 @@ def process_finished_job_description( # reduce the try count here. if replacement_job.logJobStoreFileID is None: logger.warning( - "No log file is present, despite job failing: %s", - replacement_job, + "No log file is present, despite job failing: %s. " + "Toil does not retain worker logs by default; rerun with " + "--writeLogs=PATH or --writeLogsGzip=PATH to save failed " + "jobs' logs to disk, or check the batch system's own " + "logs (see --batchLogsDir) if you are running on a grid " + "engine.", + replacement_job, ) if batch_system_id is not None: From 422b251a4c9142948fa5c6558dbf00751e44bc03 Mon Sep 17 00:00:00 2001 From: Anna Giroti Date: Wed, 15 Jul 2026 11:35:38 -0700 Subject: [PATCH 02/11] Add --runDir option to derive job store/work/coordination paths --- src/toil/common.py | 22 ++++++++++++++++++++++ src/toil/options/common.py | 10 ++++++++++ 2 files changed, 32 insertions(+) diff --git a/src/toil/common.py b/src/toil/common.py index abe3e13782..09343ca51b 100644 --- a/src/toil/common.py +++ b/src/toil/common.py @@ -173,6 +173,7 @@ class Config: colored_logs: bool workDir: str | None coordination_dir: str | None + runDir: str | None noStdOutErr: bool stats: bool @@ -356,6 +357,27 @@ def set_option(option_name: str, old_names: list[str] | None = None) -> None: # TODO: LOG LEVEL STRING set_option("workDir") set_option("coordination_dir") + set_option("runDir") + + if self.runDir is not None: + # A single --runDir was given. Derive defaults for anything the + # user didn't set explicitly; explicit flags always win. + self.runDir = os.path.abspath(self.runDir) + if self.workDir is None: + self.workDir = os.path.join(self.runDir, "work") + os.makedirs(self.workDir, exist_ok=True) + if self.coordination_dir is None: + self.coordination_dir = os.path.join(self.runDir, "coordination") + os.makedirs(self.coordination_dir, exist_ok=True) + if self.jobStore is None: + # Only reachable when --jobStore is optional (the CWL/WDL + # runners); the plain `toil` entry point requires jobStore + # as a positional argument, so it can never be None here. + from toil.options.common import parse_jobstore + + self.jobStore = parse_jobstore( + os.path.join(self.runDir, "jobstore") + ) set_option("noStdOutErr") set_option("stats") diff --git a/src/toil/options/common.py b/src/toil/options/common.py index ebd2b42130..5c28a6fc1d 100644 --- a/src/toil/options/common.py +++ b/src/toil/options/common.py @@ -357,6 +357,16 @@ def is_within(x: int | float) -> bool: "When sharing a cache between containers on a host, this directory must be " "shared between the containers.", ) + core_options.add_argument( + "--runDir", + dest="runDir", + default=None, + env_var="TOIL_RUN_DIR", + metavar="PATH", + help="Directory under which to place the job store, work dir, coordination " + "dir, and CWL/WDL image caches for this run, unless overridden individually. " + "Toil creates the directory and its subdirectories.", + ) core_options.add_argument( "--noStdOutErr", dest="noStdOutErr", From 0b37e2f5d27d81c6a9bdfb2182b230f37c5a56c7 Mon Sep 17 00:00:00 2001 From: Anna Giroti Date: Mon, 20 Jul 2026 13:21:48 -0700 Subject: [PATCH 03/11] Wire --runDir into CWL/WDL job store and cache dirs --- src/toil/cwl/cwltoil.py | 27 ++++++++++++++++++++++++++- src/toil/wdl/wdltoil.py | 40 +++++++++++++++++++++++++++++++++++----- 2 files changed, 61 insertions(+), 6 deletions(-) diff --git a/src/toil/cwl/cwltoil.py b/src/toil/cwl/cwltoil.py index 0781af5837..78b4928fbf 100644 --- a/src/toil/cwl/cwltoil.py +++ b/src/toil/cwl/cwltoil.py @@ -286,7 +286,6 @@ def ensure_no_collisions( ) seen_names.add(wanted_name) - def try_prepull( cwl_tool_uri: str, runtime_context: cwltool.context.RuntimeContext, batchsystem: str ) -> None: @@ -4748,6 +4747,23 @@ def main(args: list[str] | None = None, stdout: TextIO = sys.stdout) -> int: tmp_outdir_prefix = options.tmp_outdir_prefix or tmpdir_prefix # tmpdir_prefix and tmp_outdir_prefix must not be checked for existence as they may exist on a worker only path # See https://github.com/DataBiosphere/toil/issues/5310 + + if options.runDir is not None: + # A single --runDir was given. Derive defaults for the job store, + # work dir, and cachedir from it, for anything not set explicitly. + # This has to happen here, before the fallbacks below run, because + # by the time Toil's own Config.setOptions sees these options, + # options.jobStore is never None (the fallback below always fills + # it in first). + options.runDir = os.path.abspath(options.runDir) + if options.jobStore is None: + options.jobStore = "file:" + os.path.join(options.runDir, "jobstore") + if options.workDir is None: + options.workDir = os.path.join(options.runDir, "work") + os.makedirs(options.workDir, exist_ok=True) + if options.cachedir is None: + options.cachedir = os.path.join(options.runDir, "image-cache") + workdir = options.workDir or tmp_outdir_prefix if options.jobStore is None: @@ -4856,6 +4872,15 @@ def main(args: list[str] | None = None, stdout: TextIO = sys.stdout) -> int: expected_config = Config() expected_config.setOptions(options) + if expected_config.runDir is not None and "CWL_SINGULARITY_CACHE" not in os.environ: + # try_prepull() and cwltool's own Singularity execution code + # read this straight from the environment, so default it + # here rather than on an options/config attribute. + os.environ["CWL_SINGULARITY_CACHE"] = os.path.join( + expected_config.runDir, "image-cache", "singularity" + ) + os.makedirs(os.environ["CWL_SINGULARITY_CACHE"], exist_ok=True) + # Before showing the options to any cwltool stuff that wants to # load the workflow, transform options.cwltool, where our # argument for what to run is, to handle Dockstore workflows. diff --git a/src/toil/wdl/wdltoil.py b/src/toil/wdl/wdltoil.py index 883cc524af..88fcf68c32 100755 --- a/src/toil/wdl/wdltoil.py +++ b/src/toil/wdl/wdltoil.py @@ -257,6 +257,10 @@ class WDLContext(TypedDict): """Namespace of the WDL that the current job is in""" all_call_outputs: bool """Whether a job should include all calls outputs""" + run_dir: NotRequired[str] + """Value of Toil's --runDir option, if set. Used to default container + image cache locations when the corresponding environment variables + aren't already set.""" class InsufficientMountDiskSpace(Exception): @@ -4049,14 +4053,22 @@ def run(self, file_store: AbstractFileStore) -> Promised[WDLBindings]: # Prepare to use Singularity. We will need plenty of space to # download images. # Default the Singularity and MiniWDL cache directories. This sets the cache to the same place as - # Singularity/MiniWDL's default cache directory + # Singularity/MiniWDL's default cache directory, unless --runDir was used, in which case we put + # them under /image-cache to match Toil's other --runDir-derived paths. # With launch-cluster, the singularity and miniwdl cache is set to /var/lib/toil in abstractProvisioner.py # A current limitation with the singularity/miniwdl cache is it cannot check for image updates if the # filename is the same - singularity_cache = os.path.join(os.path.expanduser("~"), ".singularity") - miniwdl_singularity_cache = os.path.join( - os.path.expanduser("~"), ".cache/miniwdl" - ) + run_dir = self._wdl_options.get("run_dir") + if run_dir is not None: + singularity_cache = os.path.join(run_dir, "image-cache", "singularity") + miniwdl_singularity_cache = os.path.join( + run_dir, "image-cache", "miniwdl" + ) + else: + singularity_cache = os.path.join(os.path.expanduser("~"), ".singularity") + miniwdl_singularity_cache = os.path.join( + os.path.expanduser("~"), ".cache/miniwdl" + ) # Cache Singularity's layers somewhere known to have space os.environ["SINGULARITY_CACHEDIR"] = os.environ.get( @@ -5973,6 +5985,20 @@ def main() -> None: # TODO: the Toil context manager will do this again. set_logging_from_options(options) + + if options.runDir is not None: + # A single --runDir was given. Derive defaults for the job store and + # work dir from it, for anything not set explicitly. This has to + # happen here, before the fallback below runs, because by the time + # Toil's own Config.setOptions sees these options, options.jobStore + # is never None (the fallback below always fills it in first). + options.runDir = os.path.abspath(options.runDir) + if options.jobStore is None: + options.jobStore = "file:" + os.path.join(options.runDir, "jobstore") + if options.workDir is None: + options.workDir = os.path.join(options.runDir, "work") + os.makedirs(options.workDir, exist_ok=True) + # Make sure we have a jobStore if options.jobStore is None: jobstore = mkdtemp(prefix="toil-wdl-", dir=os.getcwd()) @@ -6110,6 +6136,10 @@ def main() -> None: "namespace": target.name, "all_call_outputs": options.all_call_outputs, } + + if toil.config.runDir is not None: + wdl_options["run_dir"] = toil.config.runDir + assert wdl_options.get("container") is not None if options.restart: From 32558548e7dd3fbecf5774fe56bab7e68cfc1723 Mon Sep 17 00:00:00 2001 From: Anna Giroti Date: Thu, 30 Jul 2026 10:59:45 -0700 Subject: [PATCH 04/11] Make --workDir/--coordinationDir create-if-missing; reorder failure log warning --workDir and --coordinationDir get created automatically instead of raising when missing, matching --batchLogsDir's existing behavior. ensure_dir_exists moves to toil.lib.io and backstops this in Leader.run(). The "no log file" warning on job failure now checks the batch system's own logs first, so it isn't shown next to log content Toil already found. Adds tests for dir auto-creation and --runDir derivation/override precedence. --- src/toil/common.py | 16 +++--- src/toil/cwl/cwltoil.py | 3 +- src/toil/leader.py | 61 ++++++++++++++++++---- src/toil/lib/io.py | 28 ++++++++++ src/toil/options/common.py | 8 --- src/toil/test/options/options.py | 87 ++++++++++++++++++++++++++++++++ src/toil/wdl/wdltoil.py | 1 - 7 files changed, 178 insertions(+), 26 deletions(-) diff --git a/src/toil/common.py b/src/toil/common.py index 09343ca51b..564247ac5c 100644 --- a/src/toil/common.py +++ b/src/toil/common.py @@ -354,7 +354,6 @@ def set_option(option_name: str, old_names: list[str] | None = None) -> None: # Core options set_option("jobStore") - # TODO: LOG LEVEL STRING set_option("workDir") set_option("coordination_dir") set_option("runDir") @@ -1760,8 +1759,9 @@ def getToilWorkDir(configWorkDir: str | None = None) -> str: Return a path to a writable directory under which per-workflow directories exist. This directory is always required to exist on a machine, even if the Toil - worker has not run yet. If your workers and leader have different temp - directories, you may need to set TOIL_WORKDIR. + worker has not run yet, and is created if it does not already exist. If + your workers and leader have different temp directories, you may need to + set TOIL_WORKDIR. :param configWorkDir: Value passed to the program using the --workDir flag :return: Path to the Toil work directory, constant across all machines @@ -1773,9 +1773,13 @@ def getToilWorkDir(configWorkDir: str | None = None) -> str: or tempfile.gettempdir() ) if not os.path.exists(workDir): - raise RuntimeError( - f"The directory specified by --workDir or TOIL_WORKDIR ({workDir}) does not exist." - ) + try: + os.makedirs(workDir, exist_ok=True) + except OSError as e: + raise RuntimeError( + f"The directory specified by --workDir or TOIL_WORKDIR " + f"({workDir}) does not exist and could not be created: {e}" + ) return workDir @classmethod diff --git a/src/toil/cwl/cwltoil.py b/src/toil/cwl/cwltoil.py index 78b4928fbf..c33fb40fb3 100644 --- a/src/toil/cwl/cwltoil.py +++ b/src/toil/cwl/cwltoil.py @@ -286,6 +286,7 @@ def ensure_no_collisions( ) seen_names.add(wanted_name) + def try_prepull( cwl_tool_uri: str, runtime_context: cwltool.context.RuntimeContext, batchsystem: str ) -> None: @@ -4763,7 +4764,7 @@ def main(args: list[str] | None = None, stdout: TextIO = sys.stdout) -> int: os.makedirs(options.workDir, exist_ok=True) if options.cachedir is None: options.cachedir = os.path.join(options.runDir, "image-cache") - + workdir = options.workDir or tmp_outdir_prefix if options.jobStore is None: diff --git a/src/toil/leader.py b/src/toil/leader.py index 9712ff645e..1c6181384a 100644 --- a/src/toil/leader.py +++ b/src/toil/leader.py @@ -34,6 +34,9 @@ BatchJobExitReason, UpdatedBatchJobInfo, ) +from toil.batchSystems.abstractGridEngineBatchSystem import ( + AbstractGridEngineBatchSystem, +) from toil.bus import ( JobCompletedMessage, JobFailedMessage, @@ -53,6 +56,7 @@ TemporaryID, ) from toil.jobStores.abstractJobStore import AbstractJobStore, NoSuchJobException, TOIL_WORKER_NO_JOB_STORE_EXIT_CODE +from toil.lib.io import ensure_dir_exists from toil.lib.throttle import LocalThrottle from toil.provisioners.abstractProvisioner import AbstractProvisioner from toil.provisioners.clusterScaler import ScalerThread, NonScalableBatchSystemError @@ -262,6 +266,22 @@ def run(self) -> Any: :return: The return value of the root job's run function. """ + + # Toil.getToilWorkDir and get_local_workflow_coordination_dir already + # create these directories during Toil.__enter__, before the Leader + # exists. These calls are a defensive backstop for any code path that + # reaches Leader.run() without having gone through Toil.__enter__ first. + ensure_dir_exists(self.config.workDir, "--workDir") + ensure_dir_exists(self.config.coordination_dir, "--coordinationDir") + if isinstance(self.batchSystem, AbstractGridEngineBatchSystem): + # The batch system isn't available yet when Toil logs the other + # resolved run paths (see Toil._log_resolved_paths), so log this + # one here instead, now that it exists. Only grid batch systems + # actually write their own logs to this directory. + logger.info( + "Resolved batch logs dir: %s", self.batchSystem.get_batch_logs_dir() + ) + self.jobStore.write_kill_flag(kill=False) with enlighten.get_manager( @@ -1593,17 +1613,11 @@ def process_finished_job_description( # If the batch system returned a non-zero exit code then the worker # is assumed not to have captured the failure of the job, so we # reduce the try count here. - if replacement_job.logJobStoreFileID is None: - logger.warning( - "No log file is present, despite job failing: %s. " - "Toil does not retain worker logs by default; rerun with " - "--writeLogs=PATH or --writeLogsGzip=PATH to save failed " - "jobs' logs to disk, or check the batch system's own " - "logs (see --batchLogsDir) if you are running on a grid " - "engine.", - replacement_job, - ) + # Search for the batch system's own logs first, so the + # "no log file" warning below is only shown when Toil + # genuinely found nothing, and can say so specifically. + found_batch_system_log = False if batch_system_id is not None: # Look for any standard output/error files created by the batch system. # They will only appear if the batch system actually supports @@ -1625,6 +1639,7 @@ def process_finished_job_description( else: with log_stream: if os.path.getsize(log_file) > 0: + found_batch_system_log = True StatsAndLogging.logWithFormatting( f'Log from job "{job_store_id}"', log_stream, @@ -1660,6 +1675,32 @@ def process_finished_job_description( % log_file ) + if ( + replacement_job.logJobStoreFileID is None + and not found_batch_system_log + ): + if batch_system_id is None: + logger.warning( + "No log file is present, despite job failing: %s. " + "Toil does not retain worker logs by default; rerun with " + "--writeLogs=PATH or --writeLogsGzip=PATH to save failed " + "jobs' logs to disk. Toil was not able to look for logs " + "from the batch system for this job; check the batch " + "system's own tools or logs directly.", + replacement_job, + ) + else: + logger.warning( + "No log file is present, despite job failing: %s. " + "Toil does not retain worker logs by default; rerun with " + "--writeLogs=PATH or --writeLogsGzip=PATH to save failed " + "jobs' logs to disk. Toil looked for the batch system's " + "own logs (see --batchLogsDir) but found none; check the " + "batch system's own tools or logs directly if you are " + "running on a grid engine.", + replacement_job, + ) + # Tell the job to reset itself after a failure. # It needs to know the failure reason if available; some are handled specially. replacement_job.setupJobAfterFailure( diff --git a/src/toil/lib/io.py b/src/toil/lib/io.py index 7f06ce0d88..063381304c 100644 --- a/src/toil/lib/io.py +++ b/src/toil/lib/io.py @@ -365,6 +365,34 @@ def try_path(path: str, min_size: int = 100 * 1024 * 1024) -> str | None: return path + +def ensure_dir_exists(path: str | None, arg_name: str) -> None: + """ + Ensure that the given directory exists, creating it if necessary. + + Logs a critical error and exits if the directory does not exist and + cannot be created. + + :param arg_name: Name of the flag or option that provided the path, used + to make the error message actionable. + """ + + if path is None: + return + if not os.path.exists(path): + try: + os.makedirs(path, exist_ok=True) + except OSError as e: + logger.critical( + "The path provided to %s (%s) does not exist and could not " + "be created: %s", + arg_name, + path, + e, + ) + sys.exit(1) + + def path_union(first_path: str, second_path: str | None) -> str: """ Union two os.pathsep-separated PATH environment variables. diff --git a/src/toil/options/common.py b/src/toil/options/common.py index 5c28a6fc1d..8631a60f3f 100644 --- a/src/toil/options/common.py +++ b/src/toil/options/common.py @@ -257,10 +257,6 @@ def __call__( workDir = values if workDir is not None: workDir = os.path.abspath(workDir) - if not os.path.exists(workDir): - raise RuntimeError( - f"The path provided to --workDir ({workDir}) does not exist." - ) if len(workDir) > 80: logger.warning( @@ -281,10 +277,6 @@ def __call__( coordination_dir = values if coordination_dir is not None: coordination_dir = os.path.abspath(coordination_dir) - if not os.path.exists(coordination_dir): - raise RuntimeError( - f"The path provided to --coordinationDir ({coordination_dir}) does not exist." - ) setattr(namespace, self.dest, coordination_dir) def make_closed_interval_action( diff --git a/src/toil/test/options/options.py b/src/toil/test/options/options.py index 0bdc014115..ac160c058d 100644 --- a/src/toil/test/options/options.py +++ b/src/toil/test/options/options.py @@ -1,3 +1,5 @@ +import os + from configargparse import ArgParser from toil.common import Toil, addOptions @@ -48,3 +50,88 @@ def test_caching_option_priority(self): with Toil(options) as toil: caching_value = toil.config.caching self.assertEqual(caching_value, True) + + def test_workdir_created_if_missing(self): + """ + --workDir should be created automatically if it doesn't exist (issue #5516). + """ + parser = ArgParser() + addOptions(parser, jobstore_as_flag=True, wdl=False, cwl=False) + work_dir = os.path.join(self._createTempDir(), "missing-workdir") + test_args = [ + f"--jobstore=file:{self._getTestJobStorePath()}", + f"--workDir={work_dir}", + ] + options = parser.parse_args(test_args) + self.assertFalse(os.path.exists(work_dir)) + with Toil(options): + pass + self.assertTrue(os.path.isdir(work_dir)) + + def test_coordination_dir_created_if_missing(self): + """ + --coordinationDir should be created automatically if it doesn't exist (issue #5516). + """ + parser = ArgParser() + addOptions(parser, jobstore_as_flag=True, wdl=False, cwl=False) + coordination_dir = os.path.join(self._createTempDir(), "missing-coordination") + test_args = [ + f"--jobstore=file:{self._getTestJobStorePath()}", + f"--coordinationDir={coordination_dir}", + ] + options = parser.parse_args(test_args) + self.assertFalse(os.path.exists(coordination_dir)) + with Toil(options): + pass + self.assertTrue(os.path.isdir(coordination_dir)) + + def test_rundir_derives_workdir_and_coordination_dir(self): + """ + --runDir should derive workDir/coordinationDir when they aren't explicitly set. + """ + parser = ArgParser() + addOptions(parser, jobstore_as_flag=True, wdl=False, cwl=False) + run_dir = self._createTempDir() + test_args = [ + f"--jobstore=file:{self._getTestJobStorePath()}", + f"--runDir={run_dir}", + ] + options = parser.parse_args(test_args) + with Toil(options) as toil: + config = toil.config + self.assertEqual(config.workDir, os.path.join(run_dir, "work")) + self.assertEqual( + config.coordination_dir, os.path.join(run_dir, "coordination") + ) + + def test_rundir_derives_jobstore_when_omitted(self): + """ + --runDir should derive the job store location when --jobstore is not given. + Only reachable when --jobstore is an optional flag, as with the CWL/WDL runners. + """ + parser = ArgParser() + addOptions(parser, jobstore_as_flag=True, wdl=False, cwl=False) + run_dir = self._createTempDir() + options = parser.parse_args([f"--runDir={run_dir}"]) + with Toil(options) as toil: + config = toil.config + self.assertEqual(config.jobStore, f"file:{os.path.join(run_dir, 'jobstore')}") + + def test_explicit_workdir_overrides_rundir(self): + """ + An explicit --workDir should win over the --runDir-derived default. + """ + parser = ArgParser() + addOptions(parser, jobstore_as_flag=True, wdl=False, cwl=False) + run_dir = self._createTempDir() + explicit_work_dir = self._createTempDir() + test_args = [ + f"--jobstore=file:{self._getTestJobStorePath()}", + f"--runDir={run_dir}", + f"--workDir={explicit_work_dir}", + ] + options = parser.parse_args(test_args) + with Toil(options) as toil: + config = toil.config + self.assertEqual(config.workDir, explicit_work_dir) + self.assertFalse(os.path.exists(os.path.join(run_dir, "work"))) diff --git a/src/toil/wdl/wdltoil.py b/src/toil/wdl/wdltoil.py index 88fcf68c32..f45a257219 100755 --- a/src/toil/wdl/wdltoil.py +++ b/src/toil/wdl/wdltoil.py @@ -5985,7 +5985,6 @@ def main() -> None: # TODO: the Toil context manager will do this again. set_logging_from_options(options) - if options.runDir is not None: # A single --runDir was given. Derive defaults for the job store and # work dir from it, for anything not set explicitly. This has to From 6e76eba176987d0c1cfc9a2bccc8ca8c2780b443 Mon Sep 17 00:00:00 2001 From: Anna Giroti Date: Thu, 30 Jul 2026 12:42:18 -0700 Subject: [PATCH 05/11] Fix misleading comment on --runDir jobStore derivation branch --- src/toil/common.py | 9 ++++++--- src/toil/test/options/options.py | 5 ++++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/toil/common.py b/src/toil/common.py index 564247ac5c..ad8ca19ad7 100644 --- a/src/toil/common.py +++ b/src/toil/common.py @@ -369,9 +369,12 @@ def set_option(option_name: str, old_names: list[str] | None = None) -> None: self.coordination_dir = os.path.join(self.runDir, "coordination") os.makedirs(self.coordination_dir, exist_ok=True) if self.jobStore is None: - # Only reachable when --jobStore is optional (the CWL/WDL - # runners); the plain `toil` entry point requires jobStore - # as a positional argument, so it can never be None here. + # Only reachable for direct callers of the flag-based parser + # (jobstore_as_flag=True) that leave --jobStore unset. The + # CWL/WDL runners fill in their own jobStore default before + # calling setOptions, and the plain `toil` entry point + # requires jobStore as a positional argument, so neither + # ever reaches this branch in practice. from toil.options.common import parse_jobstore self.jobStore = parse_jobstore( diff --git a/src/toil/test/options/options.py b/src/toil/test/options/options.py index ac160c058d..9f68b26952 100644 --- a/src/toil/test/options/options.py +++ b/src/toil/test/options/options.py @@ -107,7 +107,10 @@ def test_rundir_derives_workdir_and_coordination_dir(self): def test_rundir_derives_jobstore_when_omitted(self): """ --runDir should derive the job store location when --jobstore is not given. - Only reachable when --jobstore is an optional flag, as with the CWL/WDL runners. + Only reachable for direct callers of the flag-based parser + (jobstore_as_flag=True); the CWL/WDL runners fill in their own jobStore + default before Config.setOptions ever runs, so they never hit this + branch in practice. """ parser = ArgParser() addOptions(parser, jobstore_as_flag=True, wdl=False, cwl=False) From a62076f1adfed20b4ae65db638a40a8746b0d9ee Mon Sep 17 00:00:00 2001 From: Anna Giroti Date: Mon, 3 Aug 2026 12:25:04 -0700 Subject: [PATCH 06/11] Add tests for --runDir end-to-end wiring and ensure_dur exists --- src/toil/test/cwl/cwlTest.py | 26 ++++++++++++++++++++++ src/toil/test/lib/test_misc.py | 36 +++++++++++++++++++++++++++++++ src/toil/test/options/options.py | 20 +++++++++++++++-- src/toil/test/wdl/wdltoil_test.py | 23 ++++++++++++++++++++ 4 files changed, 103 insertions(+), 2 deletions(-) diff --git a/src/toil/test/cwl/cwlTest.py b/src/toil/test/cwl/cwlTest.py index 7d79ec5278..fc686fe170 100644 --- a/src/toil/test/cwl/cwlTest.py +++ b/src/toil/test/cwl/cwlTest.py @@ -250,6 +250,32 @@ def test_cwl_cmdline_input(self) -> None: # If the workflow runs, it must have had options cwltoil.main(args, stdout=st) + def test_cwl_run_dir(self, tmp_path: Path) -> None: + """ + Test that --runDir derives the job store, work dir, and image cache + locations. + """ + from toil.cwl import cwltoil + + run_dir = tmp_path / "rundir" + with get_data("test/cwl/conditional_wf.cwl") as cwlfile: + args = [ + f"--runDir={run_dir}", + "--clean=never", + "--outdir", + str(tmp_path / "out"), + str(cwlfile), + "--message", + "str", + "--sleep", + "2", + ] + st = StringIO() + cwltoil.main(args, stdout=st) + assert (run_dir / "jobstore").is_dir() + assert (run_dir / "work").is_dir() + assert (run_dir / "image-cache").is_dir() + def _tester( self, cwlfile: Path, diff --git a/src/toil/test/lib/test_misc.py b/src/toil/test/lib/test_misc.py index 87db174282..30c90b1869 100644 --- a/src/toil/test/lib/test_misc.py +++ b/src/toil/test/lib/test_misc.py @@ -13,7 +13,10 @@ # limitations under the License. import getpass import logging +import os +from unittest.mock import patch +from toil.lib.io import ensure_dir_exists from toil.lib.misc import get_user_name from toil.test import ToilTest @@ -61,6 +64,39 @@ def test_get_user_name(self): self.assertNotEqual(apparent_user_name, "") +class EnsureDirExistsTest(ToilTest): + """ + Tests for ensure_dir_exists. + """ + + def test_none_path_is_a_noop(self): + ensure_dir_exists(None, "--workDir") + + def test_creates_missing_directory(self): + target = os.path.join(self._createTempDir(), "missing", "nested") + self.assertFalse(os.path.exists(target)) + ensure_dir_exists(target, "--workDir") + self.assertTrue(os.path.isdir(target)) + + def test_existing_directory_is_left_alone(self): + target = self._createTempDir() + marker = os.path.join(target, "keep-me") + with open(marker, "w") as f: + f.write("data") + ensure_dir_exists(target, "--coordinationDir") + self.assertTrue(os.path.exists(marker)) + + def test_exits_when_directory_cannot_be_created(self): + target = os.path.join(self._createTempDir(), "unwritable") + with patch("os.makedirs", side_effect=OSError("Permission denied")): + with self.assertLogs("toil.lib.io", level="CRITICAL") as cm: + with self.assertRaises(SystemExit) as exc_info: + ensure_dir_exists(target, "--workDir") + self.assertEqual(exc_info.exception.code, 1) + self.assertIn("--workDir", cm.output[0]) + self.assertIn(target, cm.output[0]) + + class UserNameVeryBrokenTest(ToilTest): """ Make sure we can get something for a user name when user name fetching is diff --git a/src/toil/test/options/options.py b/src/toil/test/options/options.py index 9f68b26952..1faa4a1e22 100644 --- a/src/toil/test/options/options.py +++ b/src/toil/test/options/options.py @@ -53,7 +53,7 @@ def test_caching_option_priority(self): def test_workdir_created_if_missing(self): """ - --workDir should be created automatically if it doesn't exist (issue #5516). + --workDir should be created automatically if it doesn't exist. """ parser = ArgParser() addOptions(parser, jobstore_as_flag=True, wdl=False, cwl=False) @@ -70,7 +70,7 @@ def test_workdir_created_if_missing(self): def test_coordination_dir_created_if_missing(self): """ - --coordinationDir should be created automatically if it doesn't exist (issue #5516). + --coordinationDir should be created automatically if it doesn't exist. """ parser = ArgParser() addOptions(parser, jobstore_as_flag=True, wdl=False, cwl=False) @@ -138,3 +138,19 @@ def test_explicit_workdir_overrides_rundir(self): config = toil.config self.assertEqual(config.workDir, explicit_work_dir) self.assertFalse(os.path.exists(os.path.join(run_dir, "work"))) + + def test_resolved_paths_are_logged(self): + """ + Entering Toil(options) should log the resolved job store, work dir, + and coordination dir at INFO. + """ + parser = ArgParser() + addOptions(parser, jobstore_as_flag=True, wdl=False, cwl=False) + job_store = self._getTestJobStorePath() + options = parser.parse_args([f"--jobstore=file:{job_store}"]) + with self.assertLogs("toil.common", level="INFO") as cm: + with Toil(options): + pass + self.assertTrue( + any("Resolved Toil run paths" in message for message in cm.output) + ) diff --git a/src/toil/test/wdl/wdltoil_test.py b/src/toil/test/wdl/wdltoil_test.py index f6ddf01023..a4ac2b9d1a 100644 --- a/src/toil/test/wdl/wdltoil_test.py +++ b/src/toil/test/wdl/wdltoil_test.py @@ -1050,6 +1050,29 @@ def test_missing_output_directory(self, tmp_path: Path) -> None: ] ) + @needs_singularity_or_docker + def test_run_dir(self, tmp_path: Path) -> None: + """ + Test that --runDir derives the job store and work dir locations. + """ + run_dir = tmp_path / "rundir" + with get_data("test/wdl/md5sum/md5sum.1.0.wdl") as wdl: + with get_data("test/wdl/md5sum/md5sum.json") as json_file: + subprocess.check_call( + self.base_command + + [ + str(wdl), + str(json_file), + "-o", + str(tmp_path / "out"), + f"--runDir={run_dir}", + "--clean=never", + "--retryCount=0", + ] + ) + assert (run_dir / "jobstore").is_dir() + assert (run_dir / "work").is_dir() + @needs_singularity_or_docker def test_miniwdl_self_test( self, tmp_path: Path, extra_args: list[str] | None = None From b13080eeac54b0c360a4a9309211bbc3715501c5 Mon Sep 17 00:00:00 2001 From: Anna Giroti Date: Mon, 3 Aug 2026 12:25:49 -0700 Subject: [PATCH 07/11] Flag --runDir job store collision risk with concurrent workflows --- src/toil/common.py | 5 +++++ src/toil/cwl/cwltoil.py | 5 +++++ src/toil/wdl/wdltoil.py | 5 +++++ 3 files changed, 15 insertions(+) diff --git a/src/toil/common.py b/src/toil/common.py index ad8ca19ad7..b9757ed3ad 100644 --- a/src/toil/common.py +++ b/src/toil/common.py @@ -375,6 +375,11 @@ def set_option(option_name: str, old_names: list[str] | None = None) -> None: # calling setOptions, and the plain `toil` entry point # requires jobStore as a positional argument, so neither # ever reaches this branch in practice. + # TODO: This path is fixed (/jobstore), so multiple + # concurrent workflows sharing one --runDir will collide + # trying to create the same job store. Needs a unique + # suffix per invocation, like the create_tmp_dir/mkdtemp + # fallback used when --runDir isn't set. from toil.options.common import parse_jobstore self.jobStore = parse_jobstore( diff --git a/src/toil/cwl/cwltoil.py b/src/toil/cwl/cwltoil.py index c33fb40fb3..6259f6c3fe 100644 --- a/src/toil/cwl/cwltoil.py +++ b/src/toil/cwl/cwltoil.py @@ -4758,6 +4758,11 @@ def main(args: list[str] | None = None, stdout: TextIO = sys.stdout) -> int: # it in first). options.runDir = os.path.abspath(options.runDir) if options.jobStore is None: + # TODO: This path is fixed (/jobstore), so multiple + # concurrent workflows sharing one --runDir will collide + # trying to create the same job store. Needs a unique suffix + # per invocation, like the create_tmp_dir fallback below used + # when --runDir isn't set. options.jobStore = "file:" + os.path.join(options.runDir, "jobstore") if options.workDir is None: options.workDir = os.path.join(options.runDir, "work") diff --git a/src/toil/wdl/wdltoil.py b/src/toil/wdl/wdltoil.py index f45a257219..3d761cd4cd 100755 --- a/src/toil/wdl/wdltoil.py +++ b/src/toil/wdl/wdltoil.py @@ -5993,6 +5993,11 @@ def main() -> None: # is never None (the fallback below always fills it in first). options.runDir = os.path.abspath(options.runDir) if options.jobStore is None: + # TODO: This path is fixed (/jobstore), so multiple + # concurrent workflows sharing one --runDir will collide + # trying to create the same job store. Needs a unique suffix + # per invocation, like the mkdtemp fallback below used when + # --runDir isn't set. options.jobStore = "file:" + os.path.join(options.runDir, "jobstore") if options.workDir is None: options.workDir = os.path.join(options.runDir, "work") From 6e8f230b906401465d8d4bc75849d22a358aea1e Mon Sep 17 00:00:00 2001 From: Anna Giroti Date: Mon, 3 Aug 2026 14:29:22 -0700 Subject: [PATCH 08/11] Call ensure_dir_exists at worker start-up --- src/toil/worker.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/toil/worker.py b/src/toil/worker.py index 9e866d5b01..0e75c9a327 100644 --- a/src/toil/worker.py +++ b/src/toil/worker.py @@ -50,7 +50,7 @@ JobDescription, ) from toil.jobStores.abstractJobStore import AbstractJobStore, NoSuchJobStoreException, TOIL_WORKER_NO_JOB_STORE_EXIT_CODE -from toil.lib.io import make_public_dir, path_union +from toil.lib.io import make_public_dir, path_union, ensure_dir_exists from toil.lib.resources import ResourceMonitor from toil.statsAndLogging import StatsDict, configure_root_logger, install_log_color, set_log_level @@ -334,6 +334,10 @@ def workerScript( unstick_thread = threading.Thread(target=unstick_worker, args=()) unstick_thread.daemon = True unstick_thread.start() + + # Make sure the directories we need exist. + ensure_dir_exists(config.workDir, "--workDir") + ensure_dir_exists(config.coordination_dir, "--coordinationDir") ########################################## # Load the environment for the job From 53c262cd7f65ea8b6065135fc5ce474a6a6783d9 Mon Sep 17 00:00:00 2001 From: Anna Giroti Date: Tue, 4 Aug 2026 12:58:21 -0700 Subject: [PATCH 09/11] Deduplicate --runDir derivation into a shared function --- src/toil/common.py | 72 +++++++++++++++++++++++++++-------------- src/toil/cwl/cwltoil.py | 37 +++++++++++---------- src/toil/wdl/wdltoil.py | 37 +++++++++++---------- 3 files changed, 87 insertions(+), 59 deletions(-) diff --git a/src/toil/common.py b/src/toil/common.py index b9757ed3ad..2b8d791e8e 100644 --- a/src/toil/common.py +++ b/src/toil/common.py @@ -83,7 +83,7 @@ from toil.lib.retry import retry from toil.lib.threading import ensure_filesystem_lockable from toil.lib.url import URLAccess -from toil.options.common import JOBSTORE_HELP, add_base_toil_options +from toil.options.common import JOBSTORE_HELP, add_base_toil_options, parse_jobstore from toil.options.cwl import add_cwl_options from toil.options.runner import add_runner_options from toil.options.wdl import add_wdl_options @@ -123,6 +123,41 @@ def get_default_config_path() -> str: return os.path.join(get_toil_home(), "default.yaml") +def derive_run_dir_defaults( + run_dir: str, + job_store: str | None, + work_dir: str | None, + coordination_dir: str | None, +) -> tuple[str, str, str, str]: + """ + Given --runDir and the current values of --jobStore, --workDir, and + --coordinationDir, fill in defaults for any of them left unset, + derived from --runDir. Explicit values always win. + + Creates the derived work dir and coordination dir; the job store + creates itself. + + :return: (run_dir, job_store, work_dir, coordination_dir), with + run_dir made absolute and the other three either the + original explicit value or the runDir-derived default. + """ + run_dir = os.path.abspath(run_dir) + if job_store is None: + # TODO: This path is fixed (/jobstore), so multiple + # concurrent workflows sharing one --runDir will collide trying + # to create the same job store. Needs a unique suffix per + # invocation, like the create_tmp_dir/mkdtemp fallback used when + # --runDir isn't set. + job_store = parse_jobstore(os.path.join(run_dir, "jobstore")) + if work_dir is None: + work_dir = os.path.join(run_dir, "work") + os.makedirs(work_dir, exist_ok=True) + if coordination_dir is None: + coordination_dir = os.path.join(run_dir, "coordination") + os.makedirs(coordination_dir, exist_ok=True) + return run_dir, job_store, work_dir, coordination_dir + + class Config: """Class to represent configuration operations for a toil workflow run.""" @@ -360,31 +395,18 @@ def set_option(option_name: str, old_names: list[str] | None = None) -> None: if self.runDir is not None: # A single --runDir was given. Derive defaults for anything the - # user didn't set explicitly; explicit flags always win. - self.runDir = os.path.abspath(self.runDir) - if self.workDir is None: - self.workDir = os.path.join(self.runDir, "work") - os.makedirs(self.workDir, exist_ok=True) - if self.coordination_dir is None: - self.coordination_dir = os.path.join(self.runDir, "coordination") - os.makedirs(self.coordination_dir, exist_ok=True) - if self.jobStore is None: - # Only reachable for direct callers of the flag-based parser - # (jobstore_as_flag=True) that leave --jobStore unset. The - # CWL/WDL runners fill in their own jobStore default before - # calling setOptions, and the plain `toil` entry point - # requires jobStore as a positional argument, so neither - # ever reaches this branch in practice. - # TODO: This path is fixed (/jobstore), so multiple - # concurrent workflows sharing one --runDir will collide - # trying to create the same job store. Needs a unique - # suffix per invocation, like the create_tmp_dir/mkdtemp - # fallback used when --runDir isn't set. - from toil.options.common import parse_jobstore - - self.jobStore = parse_jobstore( - os.path.join(self.runDir, "jobstore") + # user didn't set explicitly; explicit flags always win. The + # jobStore-derivation branch inside this is only reachable for + # direct callers of the flag-based parser (jobstore_as_flag=True) + # that leave --jobStore unset. The CWL/WDL runners fill in their + # own jobStore default before calling setOptions, and the plain + # `toil` entry point requires jobStore as a positional argument, + # so neither ever reaches it in practice. + self.runDir, self.jobStore, self.workDir, self.coordination_dir = ( + derive_run_dir_defaults( + self.runDir, self.jobStore, self.workDir, self.coordination_dir ) + ) set_option("noStdOutErr") set_option("stats") diff --git a/src/toil/cwl/cwltoil.py b/src/toil/cwl/cwltoil.py index 6259f6c3fe..0d089427ed 100644 --- a/src/toil/cwl/cwltoil.py +++ b/src/toil/cwl/cwltoil.py @@ -111,7 +111,13 @@ from toil.batchSystems.abstractBatchSystem import InsufficientSystemResources from toil.batchSystems.registry import DEFAULT_BATCH_SYSTEM -from toil.common import Config, Toil, addOptions, InconsistentConfigurationError +from toil.common import ( + Config, + Toil, + addOptions, + derive_run_dir_defaults, + InconsistentConfigurationError, +) from toil.cwl import check_cwltool_version from toil.lib.directory import DirectoryContents, decode_directory, encode_directory from toil.lib.interpreter import ( @@ -4751,22 +4757,19 @@ def main(args: list[str] | None = None, stdout: TextIO = sys.stdout) -> int: if options.runDir is not None: # A single --runDir was given. Derive defaults for the job store, - # work dir, and cachedir from it, for anything not set explicitly. - # This has to happen here, before the fallbacks below run, because - # by the time Toil's own Config.setOptions sees these options, - # options.jobStore is never None (the fallback below always fills - # it in first). - options.runDir = os.path.abspath(options.runDir) - if options.jobStore is None: - # TODO: This path is fixed (/jobstore), so multiple - # concurrent workflows sharing one --runDir will collide - # trying to create the same job store. Needs a unique suffix - # per invocation, like the create_tmp_dir fallback below used - # when --runDir isn't set. - options.jobStore = "file:" + os.path.join(options.runDir, "jobstore") - if options.workDir is None: - options.workDir = os.path.join(options.runDir, "work") - os.makedirs(options.workDir, exist_ok=True) + # work dir, coordination dir, and cachedir from it, for anything + # not set explicitly. This has to happen here, before the + # fallbacks below run, because by the time Toil's own + # Config.setOptions sees these options, options.jobStore is + # never None (the fallback below always fills it in first). + options.runDir, options.jobStore, options.workDir, options.coordination_dir = ( + derive_run_dir_defaults( + options.runDir, + options.jobStore, + options.workDir, + options.coordination_dir, + ) + ) if options.cachedir is None: options.cachedir = os.path.join(options.runDir, "image-cache") diff --git a/src/toil/wdl/wdltoil.py b/src/toil/wdl/wdltoil.py index 3d761cd4cd..8d55eed420 100755 --- a/src/toil/wdl/wdltoil.py +++ b/src/toil/wdl/wdltoil.py @@ -76,7 +76,12 @@ from WDL.Tree import ReadSourceResult from toil.batchSystems.abstractBatchSystem import InsufficientSystemResources -from toil.common import Toil, addOptions, InconsistentConfigurationError +from toil.common import ( + Toil, + addOptions, + derive_run_dir_defaults, + InconsistentConfigurationError, +) from toil.exceptions import FailedJobsException from toil.fileStores import FileID from toil.fileStores.abstractFileStore import AbstractFileStore @@ -5986,22 +5991,20 @@ def main() -> None: set_logging_from_options(options) if options.runDir is not None: - # A single --runDir was given. Derive defaults for the job store and - # work dir from it, for anything not set explicitly. This has to - # happen here, before the fallback below runs, because by the time - # Toil's own Config.setOptions sees these options, options.jobStore - # is never None (the fallback below always fills it in first). - options.runDir = os.path.abspath(options.runDir) - if options.jobStore is None: - # TODO: This path is fixed (/jobstore), so multiple - # concurrent workflows sharing one --runDir will collide - # trying to create the same job store. Needs a unique suffix - # per invocation, like the mkdtemp fallback below used when - # --runDir isn't set. - options.jobStore = "file:" + os.path.join(options.runDir, "jobstore") - if options.workDir is None: - options.workDir = os.path.join(options.runDir, "work") - os.makedirs(options.workDir, exist_ok=True) + # A single --runDir was given. Derive defaults for the job store, + # work dir, and coordination dir from it, for anything not set + # explicitly. This has to happen here, before the fallback below + # runs, because by the time Toil's own Config.setOptions sees + # these options, options.jobStore is never None (the fallback + # below always fills it in first). + options.runDir, options.jobStore, options.workDir, options.coordination_dir = ( + derive_run_dir_defaults( + options.runDir, + options.jobStore, + options.workDir, + options.coordination_dir, + ) + ) # Make sure we have a jobStore if options.jobStore is None: From c5ae340ddedd3a3fe8fc7bbcf20d07f99a4ec405 Mon Sep 17 00:00:00 2001 From: Anna Giroti Date: Tue, 4 Aug 2026 21:19:06 -0700 Subject: [PATCH 10/11] Use a UUID for --runDir-derived job store paths --- src/toil/common.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/toil/common.py b/src/toil/common.py index 2b8d791e8e..839c88df20 100644 --- a/src/toil/common.py +++ b/src/toil/common.py @@ -143,12 +143,13 @@ def derive_run_dir_defaults( """ run_dir = os.path.abspath(run_dir) if job_store is None: - # TODO: This path is fixed (/jobstore), so multiple - # concurrent workflows sharing one --runDir will collide trying - # to create the same job store. Needs a unique suffix per - # invocation, like the create_tmp_dir/mkdtemp fallback used when - # --runDir isn't set. - job_store = parse_jobstore(os.path.join(run_dir, "jobstore")) + # Give each invocation its own job store directory under runDir, + # so concurrent workflows sharing one --runDir don't collide + # trying to create the same job store. The exact path is logged + # at startup (see Toil._log_resolved_paths) for --restart. + job_store = parse_jobstore( + os.path.join(run_dir, f"jobstore-{uuid.uuid4().hex}") + ) if work_dir is None: work_dir = os.path.join(run_dir, "work") os.makedirs(work_dir, exist_ok=True) From de58bc2574e315721b0b8202f5c614d99b7bdeba Mon Sep 17 00:00:00 2001 From: Anna Giroti Date: Thu, 6 Aug 2026 12:34:34 -0700 Subject: [PATCH 11/11] Test derive_run_dir_defaults directly instead of via each entry point --- src/toil/test/cwl/cwlTest.py | 9 ++-- src/toil/test/options/options.py | 54 ----------------------- src/toil/test/src/commonTests.py | 73 ++++++++++++++++++++++++++++++- src/toil/test/wdl/wdltoil_test.py | 7 ++- 4 files changed, 79 insertions(+), 64 deletions(-) diff --git a/src/toil/test/cwl/cwlTest.py b/src/toil/test/cwl/cwlTest.py index fc686fe170..d4d5c4348b 100644 --- a/src/toil/test/cwl/cwlTest.py +++ b/src/toil/test/cwl/cwlTest.py @@ -252,8 +252,10 @@ def test_cwl_cmdline_input(self) -> None: def test_cwl_run_dir(self, tmp_path: Path) -> None: """ - Test that --runDir derives the job store, work dir, and image cache - locations. + Test that --runDir derives the CWL image cache location. The job + store/work dir derivation is covered by + commonTests.TestDeriveRunDirDefaults; cachedir is CWL-specific and + isn't. """ from toil.cwl import cwltoil @@ -261,7 +263,6 @@ def test_cwl_run_dir(self, tmp_path: Path) -> None: with get_data("test/cwl/conditional_wf.cwl") as cwlfile: args = [ f"--runDir={run_dir}", - "--clean=never", "--outdir", str(tmp_path / "out"), str(cwlfile), @@ -272,8 +273,6 @@ def test_cwl_run_dir(self, tmp_path: Path) -> None: ] st = StringIO() cwltoil.main(args, stdout=st) - assert (run_dir / "jobstore").is_dir() - assert (run_dir / "work").is_dir() assert (run_dir / "image-cache").is_dir() def _tester( diff --git a/src/toil/test/options/options.py b/src/toil/test/options/options.py index 1faa4a1e22..53d6aa11cd 100644 --- a/src/toil/test/options/options.py +++ b/src/toil/test/options/options.py @@ -85,60 +85,6 @@ def test_coordination_dir_created_if_missing(self): pass self.assertTrue(os.path.isdir(coordination_dir)) - def test_rundir_derives_workdir_and_coordination_dir(self): - """ - --runDir should derive workDir/coordinationDir when they aren't explicitly set. - """ - parser = ArgParser() - addOptions(parser, jobstore_as_flag=True, wdl=False, cwl=False) - run_dir = self._createTempDir() - test_args = [ - f"--jobstore=file:{self._getTestJobStorePath()}", - f"--runDir={run_dir}", - ] - options = parser.parse_args(test_args) - with Toil(options) as toil: - config = toil.config - self.assertEqual(config.workDir, os.path.join(run_dir, "work")) - self.assertEqual( - config.coordination_dir, os.path.join(run_dir, "coordination") - ) - - def test_rundir_derives_jobstore_when_omitted(self): - """ - --runDir should derive the job store location when --jobstore is not given. - Only reachable for direct callers of the flag-based parser - (jobstore_as_flag=True); the CWL/WDL runners fill in their own jobStore - default before Config.setOptions ever runs, so they never hit this - branch in practice. - """ - parser = ArgParser() - addOptions(parser, jobstore_as_flag=True, wdl=False, cwl=False) - run_dir = self._createTempDir() - options = parser.parse_args([f"--runDir={run_dir}"]) - with Toil(options) as toil: - config = toil.config - self.assertEqual(config.jobStore, f"file:{os.path.join(run_dir, 'jobstore')}") - - def test_explicit_workdir_overrides_rundir(self): - """ - An explicit --workDir should win over the --runDir-derived default. - """ - parser = ArgParser() - addOptions(parser, jobstore_as_flag=True, wdl=False, cwl=False) - run_dir = self._createTempDir() - explicit_work_dir = self._createTempDir() - test_args = [ - f"--jobstore=file:{self._getTestJobStorePath()}", - f"--runDir={run_dir}", - f"--workDir={explicit_work_dir}", - ] - options = parser.parse_args(test_args) - with Toil(options) as toil: - config = toil.config - self.assertEqual(config.workDir, explicit_work_dir) - self.assertFalse(os.path.exists(os.path.join(run_dir, "work"))) - def test_resolved_paths_are_logged(self): """ Entering Toil(options) should log the resolved job store, work dir, diff --git a/src/toil/test/src/commonTests.py b/src/toil/test/src/commonTests.py index 5a133d6bde..c6672b5c2f 100644 --- a/src/toil/test/src/commonTests.py +++ b/src/toil/test/src/commonTests.py @@ -14,10 +14,11 @@ import logging import os import sys +from pathlib import Path import pytest -from toil.common import Config, InconsistentConfigurationError +from toil.common import Config, InconsistentConfigurationError, derive_run_dir_defaults logger = logging.getLogger(__name__) logging.basicConfig() @@ -44,4 +45,74 @@ def test_check_configuration_consistency_disallows_bad_scaling_setup(self) -> No assert "aws" in str(info.value) +class TestDeriveRunDirDefaults: + """ + Tests for derive_run_dir_defaults, which backs --runDir. + """ + + def test_all_explicit_values_are_unchanged(self, tmp_path: Path) -> None: + run_dir = tmp_path / "rundir" + job_store = "file:/some/explicit/jobstore" + work_dir = str(tmp_path / "explicit-work") + coordination_dir = str(tmp_path / "explicit-coordination") + + result = derive_run_dir_defaults( + str(run_dir), job_store, work_dir, coordination_dir + ) + + assert result == (str(run_dir), job_store, work_dir, coordination_dir) + # Explicit paths are the caller's responsibility; this function + # should not have touched the filesystem for them. + assert not os.path.exists(work_dir) + assert not os.path.exists(coordination_dir) + + def test_derives_all_three_under_run_dir(self, tmp_path: Path) -> None: + run_dir = tmp_path / "rundir" + + result_run_dir, job_store, work_dir, coordination_dir = ( + derive_run_dir_defaults(str(run_dir), None, None, None) + ) + + assert result_run_dir == str(run_dir) + assert job_store.startswith(f"file:{run_dir / 'jobstore-'}") + assert work_dir == str(run_dir / "work") + assert coordination_dir == str(run_dir / "coordination") + + # work_dir and coordination_dir are created directly; the job + # store is only a path here and creates itself later. + assert os.path.isdir(work_dir) + assert os.path.isdir(coordination_dir) + assert not os.path.exists(job_store.removeprefix("file:")) + + def test_job_store_paths_are_unique_across_calls(self, tmp_path: Path) -> None: + run_dir = tmp_path / "rundir" + + _, job_store_1, _, _ = derive_run_dir_defaults(str(run_dir), None, None, None) + _, job_store_2, _, _ = derive_run_dir_defaults(str(run_dir), None, None, None) + + assert job_store_1 != job_store_2 + + def test_mixed_explicit_and_derived(self, tmp_path: Path) -> None: + run_dir = tmp_path / "rundir" + explicit_work_dir = str(tmp_path / "explicit-work") + + _, job_store, work_dir, coordination_dir = derive_run_dir_defaults( + str(run_dir), None, explicit_work_dir, None + ) + + assert work_dir == explicit_work_dir + assert not os.path.exists(explicit_work_dir) + assert job_store.startswith(f"file:{run_dir / 'jobstore-'}") + assert coordination_dir == str(run_dir / "coordination") + assert os.path.isdir(coordination_dir) + + def test_relative_run_dir_is_absolutized( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.chdir(tmp_path) + result_run_dir, _, _, _ = derive_run_dir_defaults( + "relative-rundir", None, None, None + ) + assert result_run_dir == str(tmp_path / "relative-rundir") + diff --git a/src/toil/test/wdl/wdltoil_test.py b/src/toil/test/wdl/wdltoil_test.py index a4ac2b9d1a..858fb7624d 100644 --- a/src/toil/test/wdl/wdltoil_test.py +++ b/src/toil/test/wdl/wdltoil_test.py @@ -1053,7 +1053,9 @@ def test_missing_output_directory(self, tmp_path: Path) -> None: @needs_singularity_or_docker def test_run_dir(self, tmp_path: Path) -> None: """ - Test that --runDir derives the job store and work dir locations. + Test that a WDL run with --runDir set succeeds end-to-end. The job + store/work dir derivation itself is covered by + commonTests.TestDeriveRunDirDefaults. """ run_dir = tmp_path / "rundir" with get_data("test/wdl/md5sum/md5sum.1.0.wdl") as wdl: @@ -1066,12 +1068,9 @@ def test_run_dir(self, tmp_path: Path) -> None: "-o", str(tmp_path / "out"), f"--runDir={run_dir}", - "--clean=never", "--retryCount=0", ] ) - assert (run_dir / "jobstore").is_dir() - assert (run_dir / "work").is_dir() @needs_singularity_or_docker def test_miniwdl_self_test(