From 5b56d669138da5725187f2bb61870924b667973f Mon Sep 17 00:00:00 2001 From: David Leong <116610336+leongdl@users.noreply.github.com> Date: Wed, 22 Jul 2026 13:55:59 -0700 Subject: [PATCH 1/2] =?UTF-8?q?feat:=20RFC=200007/0008=20support=20?= =?UTF-8?q?=E2=80=94=20step-level=20let=20bindings,=20wrap-hook=20step=20n?= =?UTF-8?q?ames,=20template-declared=20extensions=20-=20Enter=20a=20step's?= =?UTF-8?q?=20environments=20with=20the=20step-level=20EXPR=20let=20bindin?= =?UTF-8?q?gs=20(RFC=200007)=20so=20environment=20variables=20and=20action?= =?UTF-8?q?s=20can=20reference=20them.=20The=20extra=5Flet=5Fbindings=20ke?= =?UTF-8?q?yword=20is=20only=20forwarded=20to=20openjd-sessions=20when=20a?= =?UTF-8?q?=20step=20actually=20defines=20lets,=20so=20the=20CLI=20keeps?= =?UTF-8?q?=20working=20against=20older=20sessions=20releases=20for=20ever?= =?UTF-8?q?y=20template=20that=20doesn't=20use=20step-level=20lets.=20-=20?= =?UTF-8?q?Pass=20step=5Fname=20to=20Session.run=5Ftask=20so=20RFC=200008?= =?UTF-8?q?=20wrap=20hooks=20resolve=20WrappedStep.Name.=20-=20Enable=20on?= =?UTF-8?q?ly=20the=20extensions=20each=20template=20declares,=20and=20see?= =?UTF-8?q?d=20Job.Name=20into=20the=20symbol=20table.=20-=20Update=20test?= =?UTF-8?q?=5Flocal=5Fsession.py=20mock-call=20assertions=20for=20the=20en?= =?UTF-8?q?vironment-enter=20flow.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com> --- .../cli/_run/_local_session/_actions.py | 39 +++++++++++++++++-- .../_run/_local_session/_session_manager.py | 33 ++++++++++++++-- src/openjd/cli/_run/_run_command.py | 12 +++++- 3 files changed, 74 insertions(+), 10 deletions(-) diff --git a/src/openjd/cli/_run/_local_session/_actions.py b/src/openjd/cli/_run/_local_session/_actions.py index d8b757c..988f0f8 100644 --- a/src/openjd/cli/_run/_local_session/_actions.py +++ b/src/openjd/cli/_run/_local_session/_actions.py @@ -1,6 +1,7 @@ # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. from enum import Enum +from typing import Optional from openjd.model import Step, TaskParameterSet from openjd.model.v2023_09 import Environment @@ -47,7 +48,11 @@ def __init__(self, session: Session, step: Step, parameters: TaskParameterSet): def run(self): self._session.run_task( - step_script=self._step.script, task_parameter_values=self._parameters + step_script=self._step.script, + task_parameter_values=self._parameters, + # RFC 0008: the step name feeds the WrappedStep.Name template + # variable inside an active onWrapTaskRun hook. + step_name=self._step.name, ) def __str__(self): @@ -58,14 +63,40 @@ def __str__(self): class EnterEnvironmentAction(SessionAction): _environment: Environment _id: str - - def __init__(self, session: Session, environment: Environment, env_id: str): + _extra_let_bindings: Optional[list[str]] + + def __init__( + self, + session: Session, + environment: Environment, + env_id: str, + extra_let_bindings: Optional[list[str]] = None, + ): super(EnterEnvironmentAction, self).__init__(session) self._environment = environment self._id = env_id + # RFC 0007: a step's environments are entered with the step-level + # `let` bindings so their variables/actions can reference them. + self._extra_let_bindings = extra_let_bindings def run(self): - self._session.enter_environment(environment=self._environment, identifier=self._id) + # Backwards compatibility: only forward `extra_let_bindings` when the + # step actually defines `let` bindings (RFC 0007). Older + # openjd-sessions releases don't accept the keyword, so omitting it + # by default keeps this CLI working with any sessions version for + # every template that doesn't use step-level lets — the reasonable + # default is simply "no extra bindings". + if self._extra_let_bindings: + self._session.enter_environment( + environment=self._environment, + identifier=self._id, + extra_let_bindings=self._extra_let_bindings, + ) + else: + self._session.enter_environment( + environment=self._environment, + identifier=self._id, + ) def __str__(self): return f"Enter Environment '{self._environment.name}'" diff --git a/src/openjd/cli/_run/_local_session/_session_manager.py b/src/openjd/cli/_run/_local_session/_session_manager.py index 2413505..2fad51a 100644 --- a/src/openjd/cli/_run/_local_session/_session_manager.py +++ b/src/openjd/cli/_run/_local_session/_session_manager.py @@ -102,6 +102,8 @@ def __init__( self._openjd_session = Session( session_id=self.session_id, job_parameter_values=job_parameter_values, + # RFC 0007 §7.3.1 (EXPR): seeds the Job.Name template variable. + job_name=str(job.name), path_mapping_rules=self._path_mapping_rules, callback=self._action_callback, retain_working_dir=retain_working_dir, @@ -197,7 +199,13 @@ def _sigint_handler(self, signum: int, frame: Optional[FrameType]) -> None: LOG.info("Interruption signal recieved.") self.cancel() - def run_environment_enters(self, environments: Optional[list[Any]], type: EnvironmentType): + def run_environment_enters( + self, + environments: Optional[list[Any]], + type: EnvironmentType, + *, + extra_let_bindings: Optional[list[str]] = None, + ): """Enter one or more environments in the session.""" if environments is None: return @@ -211,7 +219,12 @@ def run_environment_enters(self, environments: Optional[list[Any]], type: Enviro env_id = f"{type.name} - {env.name}" self._action_ended.clear() self._current_action = EnterEnvironmentAction( - session=self._openjd_session, environment=env, env_id=env_id + session=self._openjd_session, + environment=env, + env_id=env_id, + # RFC 0007: a step's environments see the step-level `let` + # bindings. + extra_let_bindings=extra_let_bindings, ) self._environments_entered.append((type, env_id)) self._current_action.run() @@ -348,8 +361,20 @@ def run_step( if task_parameters is None: task_parameters = StepParameterSpaceIterator(space=step.parameterSpace) - # Enter all the step environments - self.run_environment_enters(step.stepEnvironments, EnvironmentType.STEP) + # Enter all the step environments. When the step defines step-level + # `let` bindings (RFC 0007), its environments are entered with them so + # their variables and actions can reference them. The keyword is only + # passed when bindings exist, keeping the call (and the sessions API + # it reaches) identical to the pre-RFC 0007 behavior by default. + step_let_bindings = getattr(step, "let", None) + if step_let_bindings: + self.run_environment_enters( + step.stepEnvironments, + EnvironmentType.STEP, + extra_let_bindings=step_let_bindings, + ) + else: + self.run_environment_enters(step.stepEnvironments, EnvironmentType.STEP) try: # Run the tasks diff --git a/src/openjd/cli/_run/_run_command.py b/src/openjd/cli/_run/_run_command.py index 0672780..a86bb6f 100644 --- a/src/openjd/cli/_run/_run_command.py +++ b/src/openjd/cli/_run/_run_command.py @@ -547,9 +547,17 @@ def do_run(args: Namespace) -> OpenJDCliResult: return OpenJDCliResult(status="error", message=str(rte)) # Create a RevisionExtensions object with the default specification version and enabled extensions - # We use the default v2023_09 since that's what we're currently supporting + # We use the default v2023_09 since that's what we're currently supporting. + # Extension behaviors activate only when declared by the templates being + # run (the job template and any external environment templates) — the + # CLI's --extensions list is what the CLI *accepts*, not what is enabled. + declared_extensions: list[str] = list(the_job.extensions or []) + for env_template in environments or []: + for ext_name in env_template.extensions or []: + if ext_name not in declared_extensions: + declared_extensions.append(ext_name) revision_extensions = RevisionExtensions( - spec_rev=the_job.revision, supported_extensions=extensions + spec_rev=the_job.revision, supported_extensions=declared_extensions ) return _run_local_session( From daed2835a4d4fa0bc54df61d8b1c058058a848ea Mon Sep 17 00:00:00 2001 From: David Leong <116610336+leongdl@users.noreply.github.com> Date: Thu, 23 Jul 2026 08:34:30 -0700 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20Address=20review=20findings=20?= =?UTF-8?q?=E2=80=94=20clean=20errors,=20Step.Name=20threading?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review fixes on top of the RFC 0007/0008 CLI commit: - Exceptions raised out of session actions (the RFC 0008 "at most one wrap environment" RuntimeError, expression ValueErrors) now surface as a clean error-status result instead of a raw traceback: guarded in run_environment_enters (routed through LocalSessionFailed) with a belt-and-suspenders catch in _run_local_session. - The enter-failure rollback only drops the environment from the CLI's entered list when the session did not register it — a post- registration raise (e.g. a failing `variables` expression) keeps the env tracked so cleanup exits it in LIFO order instead of masking the original error with "Must exit Environment X first". - Thread step_name into Session.enter_environment for step-environment enters so Step.Name (RFC 0007 §7.3.1) resolves in step-level `let` bindings and step-env actions — Rust CLI parity (openjd-rs run/mod.rs threads the step's resolved symtab). Capability-gated on the installed openjd-sessions accepting the kwarg. - run_step's redundant pass-kwarg-only-when-truthy branch collapsed (the compat guard lives in EnterEnvironmentAction.run); version-skew getattr documented with its removal condition. Tests: 294 passed under plain hatch and against the editable fixed model/sessions trees (new: Step.Name end-to-end template, step_name propagation, post-registration rollback, clean-error-result). ruff/black/mypy clean. Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com> --- .../cli/_run/_local_session/_actions.py | 44 +++++-- .../_run/_local_session/_session_manager.py | 55 ++++++-- src/openjd/cli/_run/_run_command.py | 11 +- .../cli/templates/step_name_env_job.yaml | 27 ++++ test/openjd/cli/test_local_session.py | 120 +++++++++++++++++- test/openjd/cli/test_run_command.py | 60 ++++++++- 6 files changed, 289 insertions(+), 28 deletions(-) create mode 100644 test/openjd/cli/templates/step_name_env_job.yaml diff --git a/src/openjd/cli/_run/_local_session/_actions.py b/src/openjd/cli/_run/_local_session/_actions.py index 988f0f8..7c8d013 100644 --- a/src/openjd/cli/_run/_local_session/_actions.py +++ b/src/openjd/cli/_run/_local_session/_actions.py @@ -1,12 +1,22 @@ # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +import inspect from enum import Enum -from typing import Optional +from typing import Any, Optional from openjd.model import Step, TaskParameterSet from openjd.model.v2023_09 import Environment from openjd.sessions import Session +# Version-skew guard: the step_name keyword was added to +# Session.enter_environment alongside RFC 0007 Step.Name support. Detect it +# once so this CLI keeps working against older openjd-sessions releases that +# don't accept the keyword (every step-environment enter carries a step name, +# so a value-presence check alone isn't enough here). +_ENTER_ENVIRONMENT_ACCEPTS_STEP_NAME: bool = ( + "step_name" in inspect.signature(Session.enter_environment).parameters +) + class EnvironmentType(str, Enum): """ @@ -64,6 +74,7 @@ class EnterEnvironmentAction(SessionAction): _environment: Environment _id: str _extra_let_bindings: Optional[list[str]] + _step_name: Optional[str] def __init__( self, @@ -71,6 +82,7 @@ def __init__( environment: Environment, env_id: str, extra_let_bindings: Optional[list[str]] = None, + step_name: Optional[str] = None, ): super(EnterEnvironmentAction, self).__init__(session) self._environment = environment @@ -78,6 +90,11 @@ def __init__( # RFC 0007: a step's environments are entered with the step-level # `let` bindings so their variables/actions can reference them. self._extra_let_bindings = extra_let_bindings + # RFC 0007 §7.3.1 (EXPR): the owning step's name seeds Step.Name for + # a step environment's `let` bindings, variables, and actions. Only + # step-environment enters carry a step name; job/external enters + # leave it None. + self._step_name = step_name def run(self): # Backwards compatibility: only forward `extra_let_bindings` when the @@ -85,18 +102,21 @@ def run(self): # openjd-sessions releases don't accept the keyword, so omitting it # by default keeps this CLI working with any sessions version for # every template that doesn't use step-level lets — the reasonable - # default is simply "no extra bindings". + # default is simply "no extra bindings". `step_name` follows the same + # pattern, but since every step-environment enter has a step name, it + # is additionally gated on the installed openjd-sessions accepting + # the keyword; without it, Step.Name simply stays undefined (the + # pre-RFC 0007 behavior). + optional_kwargs: dict[str, Any] = {} if self._extra_let_bindings: - self._session.enter_environment( - environment=self._environment, - identifier=self._id, - extra_let_bindings=self._extra_let_bindings, - ) - else: - self._session.enter_environment( - environment=self._environment, - identifier=self._id, - ) + optional_kwargs["extra_let_bindings"] = self._extra_let_bindings + if self._step_name is not None and _ENTER_ENVIRONMENT_ACCEPTS_STEP_NAME: + optional_kwargs["step_name"] = self._step_name + self._session.enter_environment( + environment=self._environment, + identifier=self._id, + **optional_kwargs, + ) def __str__(self): return f"Enter Environment '{self._environment.name}'" diff --git a/src/openjd/cli/_run/_local_session/_session_manager.py b/src/openjd/cli/_run/_local_session/_session_manager.py index 2fad51a..344180a 100644 --- a/src/openjd/cli/_run/_local_session/_session_manager.py +++ b/src/openjd/cli/_run/_local_session/_session_manager.py @@ -205,6 +205,7 @@ def run_environment_enters( type: EnvironmentType, *, extra_let_bindings: Optional[list[str]] = None, + step_name: Optional[str] = None, ): """Enter one or more environments in the session.""" if environments is None: @@ -225,9 +226,37 @@ def run_environment_enters( # RFC 0007: a step's environments see the step-level `let` # bindings. extra_let_bindings=extra_let_bindings, + # RFC 0007 §7.3.1 (EXPR): a step's environments see Step.Name. + # Only step-environment enters carry a step name. + step_name=step_name, ) self._environments_entered.append((type, env_id)) - self._current_action.run() + try: + self._current_action.run() + except (RuntimeError, ValueError) as exc: + # Session.enter_environment raises (rather than reporting + # through the action-status callback) when it rejects the + # environment up front — e.g. the RFC 0008 "at most one wrap + # environment" RuntimeError, or a ValueError from the extra + # `let` bindings — but it can also raise *after* registering + # the environment (e.g. an environment `variables` expression + # that fails to evaluate). Only when the session did NOT + # register the environment may we drop it from our entered + # list (cleanup must not try to exit it). If the session did + # register it, it must stay in our list so cleanup exits it — + # popping it here would skip its onExit and desynchronize us + # from the session's LIFO exit ordering check, masking the + # original error with "Must exit Environment X first". + if env_id not in self._openjd_session.environments_entered: + self._environments_entered.pop() + LOG.info( + msg=f"Open Job Description CLI: ERROR entering environment '{env.name}': {exc}", + extra={"session_id": self.session_id}, + ) + self.failed = True + self._failed_action = self._current_action + self._current_action = None + raise LocalSessionFailed(self._failed_action) from exc self._action_ended.wait() if self.failed: self._failed_action = self._current_action @@ -363,18 +392,20 @@ def run_step( # Enter all the step environments. When the step defines step-level # `let` bindings (RFC 0007), its environments are entered with them so - # their variables and actions can reference them. The keyword is only - # passed when bindings exist, keeping the call (and the sessions API - # it reaches) identical to the pre-RFC 0007 behavior by default. + # their variables and actions can reference them. + # getattr guard: requires an openjd-model with Step.let on the + # instantiated Job (openjd-model PR #318+); collapse to plain + # `step.let` once the version pin floor guarantees it. step_let_bindings = getattr(step, "let", None) - if step_let_bindings: - self.run_environment_enters( - step.stepEnvironments, - EnvironmentType.STEP, - extra_let_bindings=step_let_bindings, - ) - else: - self.run_environment_enters(step.stepEnvironments, EnvironmentType.STEP) + self.run_environment_enters( + step.stepEnvironments, + EnvironmentType.STEP, + extra_let_bindings=step_let_bindings or None, + # RFC 0007 §7.3.1 (EXPR): Step.Name is available to a step's + # environments (openjd-rs threads the step's resolved symbol + # table into enter_environment; this is the CLI counterpart). + step_name=step.name, + ) try: # Run the tasks diff --git a/src/openjd/cli/_run/_run_command.py b/src/openjd/cli/_run/_run_command.py index a86bb6f..6f1b837 100644 --- a/src/openjd/cli/_run/_run_command.py +++ b/src/openjd/cli/_run/_run_command.py @@ -338,6 +338,7 @@ def _run_local_session( Creates a Session object and listens for log messages to synchronously end the session. """ + error_message = "Session ended with errors; see Task logs for details" try: start_seconds = time.perf_counter() @@ -367,6 +368,14 @@ def _run_local_session( except LocalSessionFailed: duration = time.perf_counter() - start_seconds session = None + except (RuntimeError, ValueError) as exc: + # Exceptions raised by openjd.sessions from within session actions + # (e.g. the RFC 0008 "at most one wrap environment" RuntimeError) + # rather than reported through the action-status callback. Report + # them as a clean error result instead of a raw traceback. + duration = time.perf_counter() - start_seconds + session = None + error_message = f"Session ended with errors: {exc}" preserved_message: str = "" if retain_working_dir and session is not None: @@ -377,7 +386,7 @@ def _run_local_session( if session is None or session.failed: return OpenJDRunResult( status="error", - message="Session ended with errors; see Task logs for details" + preserved_message, + message=error_message + preserved_message, job_name=job.name, step_name=step_name, duration=duration, diff --git a/test/openjd/cli/templates/step_name_env_job.yaml b/test/openjd/cli/templates/step_name_env_job.yaml new file mode 100644 index 0000000..daf10a1 --- /dev/null +++ b/test/openjd/cli/templates/step_name_env_job.yaml @@ -0,0 +1,27 @@ +# RFC 0007 §7.3.1 (EXPR): Step.Name is available to a step's environments. +# The step-level `let` binding references Step.Name, and the step's +# environment is entered with the binding so its onEnter action can echo it. +specificationVersion: jobtemplate-2023-09 +extensions: +- EXPR +name: StepNameEnvJob +steps: +- name: EchoStepName + let: + - bound_name = Step.Name + stepEnvironments: + - name: NameEcho + script: + actions: + onEnter: + command: python + args: + - -c + - print('EnvSaw={{ bound_name }}') + script: + actions: + onRun: + command: python + args: + - -c + - print('TaskRan') diff --git a/test/openjd/cli/test_local_session.py b/test/openjd/cli/test_local_session.py index 2a23847..8ccc245 100644 --- a/test/openjd/cli/test_local_session.py +++ b/test/openjd/cli/test_local_session.py @@ -7,6 +7,7 @@ from . import SampleSteps, SESSION_PARAMETERS from openjd.model import StepParameterSpaceIterator from openjd.sessions import Session, SessionState +from openjd.cli._run._local_session._actions import _ENTER_ENVIRONMENT_ACCEPTS_STEP_NAME from openjd.cli._run._local_session._session_manager import ( LocalSession, EnvironmentType, @@ -168,7 +169,13 @@ def test_localsession_run_success( assert patched_run_environment_enters.call_args_list == [ call(session, None, EnvironmentType.EXTERNAL), call(session, sample_job.jobEnvironments, EnvironmentType.JOB), - call(session, sample_job.steps[step_index].stepEnvironments, EnvironmentType.STEP), + call( + session, + sample_job.steps[step_index].stepEnvironments, + EnvironmentType.STEP, + extra_let_bindings=None, + step_name=sample_job.steps[step_index].name, + ), ] # It should have run one step assert patched_run_step.call_args_list == [ @@ -193,6 +200,115 @@ def test_localsession_run_success( ) +@pytest.mark.usefixtures("sample_job_and_dirs") +def test_localsession_step_env_enter_receives_step_name( + sample_job_and_dirs: tuple, + patched_actions, +): + """ + RFC 0007 §7.3.1 (EXPR): a step-environment enter passes the owning step's + name to Session.enter_environment (when the installed openjd-sessions + accepts the keyword), while job/external environment enters never do. + """ + sample_job, sample_job_parameters, template_dir, current_working_dir = sample_job_and_dirs + patched_enter = patched_actions[0] + + with LocalSession( + job=sample_job, job_parameter_values=sample_job_parameters, session_id="step-name" + ) as session: + session.run_step(sample_job.steps[SampleSteps.NormalStep]) + + assert not session.failed + + step_env_calls = [ + c + for c in patched_enter.call_args_list + if c.kwargs["identifier"].startswith(f"{EnvironmentType.STEP.name} - ") + ] + other_env_calls = [ + c + for c in patched_enter.call_args_list + if not c.kwargs["identifier"].startswith(f"{EnvironmentType.STEP.name} - ") + ] + + assert step_env_calls + for enter_call in step_env_calls: + if _ENTER_ENVIRONMENT_ACCEPTS_STEP_NAME: + assert enter_call.kwargs["step_name"] == sample_job.steps[SampleSteps.NormalStep].name + else: + # Older openjd-sessions releases don't accept the keyword; the + # version-skew guard must omit it. + assert "step_name" not in enter_call.kwargs + + # Job/external environment enters never carry a step name. + assert other_env_calls + for enter_call in other_env_calls: + assert "step_name" not in enter_call.kwargs + + +@pytest.mark.usefixtures("sample_job_and_dirs", "capsys") +def test_localsession_enter_environment_post_registration_raise( + sample_job_and_dirs: tuple, capsys: pytest.CaptureFixture, patched_actions +): + """ + A raise out of Session.enter_environment *after* the session registered + the environment (e.g. an environment `variables` expression that fails to + evaluate) must leave the environment in the CLI's entered list so cleanup + exits it. Popping it would desynchronize CLI and session state: the + environment's onExit would be skipped and cleanup would trip the session's + LIFO exit check ("Must exit Environment X first"), masking the original + error. + """ + sample_job, sample_job_parameters, template_dir, current_working_dir = sample_job_and_dirs + patched_enter = patched_actions[0] + + # The autouse fixture set the mock's side_effect to the real (pre-patch) + # Session.enter_environment; capture it before redirecting the mock. + real_enter = patched_enter.side_effect + error_text = "Failed to evaluate the environment's variables" + + def register_then_raise(session, *, environment, identifier=None, **kwargs): + if identifier is not None and identifier.startswith(f"{EnvironmentType.STEP.name} - "): + # Mirror the state Session.enter_environment leaves behind when an + # environment `variables` expression fails to evaluate: the + # environment is registered in the session's entered list, but the + # enter raises before any action runs. + session._environments[identifier] = environment + session._environments_entered.append(identifier) + raise ValueError(error_text) + return real_enter(session, environment=environment, identifier=identifier, **kwargs) + + step_env_id = f"{EnvironmentType.STEP.name} - env1" + job_env_id = f"{EnvironmentType.JOB.name} - rootEnv" + + # Redirect the autouse fixture's Session.enter_environment mock from the + # real method to the registering-then-raising simulation. + patched_enter.side_effect = register_then_raise + with LocalSession( + job=sample_job, job_parameter_values=sample_job_parameters, session_id="post-reg" + ) as session: + with pytest.raises(LocalSessionFailed): + session.run_step(sample_job.steps[SampleSteps.NormalStep]) + + # The session registered the environment, so the CLI must keep it + # in its own entered list for cleanup to exit. + assert step_env_id in session._openjd_session.environments_entered + assert (EnvironmentType.STEP, step_env_id) in session._environments_entered + + # Cleanup exited the registered step environment and then the job + # environment, in LIFO order, rather than skipping the step environment. + exited_ids = [ + exit_call.kwargs["identifier"] + for exit_call in session._openjd_session.exit_environment.call_args_list # type: ignore + ] + assert exited_ids == [step_env_id, job_env_id] + + assert session.failed + output = capsys.readouterr().out + assert error_text in output + assert "Must exit Environment" not in output + + @pytest.mark.usefixtures("sample_job_and_dirs", "capsys") def test_localsession_run_failed(sample_job_and_dirs: tuple, capsys: pytest.CaptureFixture): """ @@ -221,6 +337,8 @@ def test_localsession_run_failed(sample_job_and_dirs: tuple, capsys: pytest.Capt session, sample_job.steps[SampleSteps.BadCommand].stepEnvironments, EnvironmentType.STEP, + extra_let_bindings=None, + step_name=sample_job.steps[SampleSteps.BadCommand].name, ), ] session._openjd_session.exit_environment.assert_called_once() # type: ignore diff --git a/test/openjd/cli/test_run_command.py b/test/openjd/cli/test_run_command.py index df0d656..e8c5e01 100644 --- a/test/openjd/cli/test_run_command.py +++ b/test/openjd/cli/test_run_command.py @@ -11,7 +11,7 @@ import shlex import pytest -from unittest.mock import Mock +from unittest.mock import Mock, patch from . import MOCK_TEMPLATE, SampleSteps, run_openjd_cli_main, format_capsys_outerr @@ -20,8 +20,9 @@ _process_task_params, _process_tasks, ) +from openjd.cli._run._local_session._actions import _ENTER_ENVIRONMENT_ACCEPTS_STEP_NAME from openjd.cli._run._local_session._session_manager import LoggingTimestampFormat -from openjd.sessions import LOG as SessionsLogger, PathMappingRule, PathFormat +from openjd.sessions import LOG as SessionsLogger, PathMappingRule, PathFormat, Session PARAMETRIZE_CASES: tuple = ( pytest.param( @@ -642,6 +643,61 @@ def test_run_local_session_failed( ), f"Regex r'{expected_error_regex}' does not match the output:\n{format_capsys_outerr(outerr)}" +def test_run_local_session_enter_environment_raises(capsys: pytest.CaptureFixture): + """ + Test that an exception raised out of Session.enter_environment (e.g. the + RFC 0008 "at most one wrap environment" RuntimeError, or a ValueError from + the extra `let` bindings) produces a clean error result instead of a raw + traceback propagating out of the CLI. + """ + template_dir = Path(__file__).parent / "templates" + args = [ + "run", + str(template_dir / "job_with_test_steps.yaml"), + "--step", + SampleSteps.NormalStep.name, + "--extensions", + "", + ] + + error_text = ( + "RFC 0008: a session may have at most one Environment defining wrap hooks " + "(onWrapEnvEnter / onWrapTaskRun / onWrapEnvExit)." + ) + with patch.object(Session, "enter_environment", side_effect=RuntimeError(error_text)): + outerr = run_openjd_cli_main(capsys, args=args, expected_exit_code=1) + + assert re.search( + "Session ended with errors", outerr.out, re.MULTILINE + ), f"Expected a clean error result in the output:\n{format_capsys_outerr(outerr)}" + assert error_text in outerr.out + assert "Traceback" not in outerr.out + outerr.err + + +@pytest.mark.skipif( + not _ENTER_ENVIRONMENT_ACCEPTS_STEP_NAME, + reason="Installed openjd-sessions does not accept step_name on enter_environment", +) +def test_do_run_step_name_in_step_environment(capsys: pytest.CaptureFixture) -> None: + """ + RFC 0007 §7.3.1 (EXPR) parity with openjd-rs: a step-level `let` binding + may reference Step.Name, and the step's environments are entered with the + binding so their actions can echo it. + """ + template_dir = Path(__file__).parent / "templates" + args = [ + "run", + str(template_dir / "step_name_env_job.yaml"), + "--step", + "EchoStepName", + ] + outerr = run_openjd_cli_main(capsys, args=args, expected_exit_code=0) + assert ( + "EnvSaw=EchoStepName" in outerr.out + ), f"Step.Name did not resolve in the step environment:\n{format_capsys_outerr(outerr)}" + assert "TaskRan" in outerr.out + + class TestProcessTaskParams: """Testing that we properly handle the values of the --task-param/-tp command-line argument"""