Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run executable from task dir #124

Merged
merged 2 commits into from
Aug 7, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 1 addition & 13 deletions benchcab/repository.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
"""A module containing functions and data structures for manipulating CABLE repositories."""

import shlex
import contextlib
import os
import shutil
import stat
from pathlib import Path
Expand All @@ -11,17 +9,7 @@
from benchcab import internal
from benchcab.environment_modules import EnvironmentModulesInterface, EnvironmentModules
from benchcab.utils.subprocess import SubprocessWrapperInterface, SubprocessWrapper


@contextlib.contextmanager
def chdir(newdir: Path):
"""Context manager `cd`."""
prevdir = Path.cwd()
os.chdir(newdir.expanduser())
try:
yield
finally:
os.chdir(prevdir)
from benchcab.utils.os import chdir


class CableRepository:
Expand Down
12 changes: 7 additions & 5 deletions benchcab/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from benchcab.repository import CableRepository
from benchcab.comparison import ComparisonTask
from benchcab.utils.subprocess import SubprocessWrapperInterface, SubprocessWrapper
from benchcab.utils.os import chdir


# fmt: off
Expand Down Expand Up @@ -260,14 +261,15 @@ def run_cable(self, verbose=False):
"""
task_name = self.get_task_name()
task_dir = self.root_dir / internal.FLUXSITE_TASKS_DIR / task_name
exe_path = task_dir / internal.CABLE_EXE
nml_path = task_dir / internal.CABLE_NML
stdout_path = task_dir / internal.CABLE_STDOUT_FILENAME

try:
self.subprocess_handler.run_cmd(
f"{exe_path} {nml_path}", output_file=stdout_path, verbose=verbose
)
with chdir(task_dir):
self.subprocess_handler.run_cmd(
f"./{internal.CABLE_EXE} {internal.CABLE_NML}",
output_file=stdout_path,
verbose=verbose,
)
except CalledProcessError as exc:
print(f"Error: CABLE returned an error for task {task_name}")
raise CableError from exc
Expand Down
16 changes: 16 additions & 0 deletions benchcab/utils/os.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
"""Utility functions which wrap around the os module."""

import os
import contextlib
from pathlib import Path


@contextlib.contextmanager
def chdir(newdir: Path):
"""Context manager `cd`."""
prevdir = Path.cwd()
os.chdir(newdir.expanduser())
try:
yield
finally:
os.chdir(prevdir)
2 changes: 1 addition & 1 deletion tests/test_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ def test_run_cable():

# Success case: run CABLE executable in subprocess
task.run_cable()
assert f"{exe_path} {nml_path}" in mock_subprocess.commands
assert f"./{exe_path.name} {nml_path.name}" in mock_subprocess.commands
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm suddenly wondering if exe_path and nml_path really need to exist? Could this assert be: assert f"./{internal.CABLE_EXE} {internal.CABLE_nml} in mock_subprocess.commands and remove the definition of exe_path and nml_path.

That's a detail, I'm happy to put things in as is. Just mentioning it as I like things as simple as possible when we think of it.

assert stdout_file.exists()

# Success case: test non-verbose output
Expand Down