Skip to content

Commit 4186f8b

Browse files
test: make pre-GUI hook integ test interpreter-portable + prove the hook ran
Addresses a PR #480 review comment on the hook fixture: `command: python` is not a reliable interpreter name. deadline-cloud resolves a hook `command` as an absolute path, then relative to the hooks dir, then via `shutil.which` on PATH (deadline.client.job_bundle._hooks._executor) with no env-var expansion. A bare `python` doesn't exist on macOS (only `python3` since the system Python 2 removal) and otherwise depends on the PATH Cinema 4D inherited, so it could fail to resolve — and when it did, the hook silently never ran and the failure surfaced as the three value assertions failing rather than "the hook could not be launched". - hooks.yaml is no longer committed. The test generates it per-run (_materialize_pregui_hooks_dir) with `command` = sys.executable — an absolute path that always resolves; pregui_hook.py needs only the stdlib, so any interpreter works. args references the committed script by absolute path. - pregui_hook.py writes a marker file when DEADLINE_CLOUD_PREGUI_MARKER is set; the test asserts the marker up front, separating "hook never launched" from "hook ran but its output wasn't wired into the bundle". Value-assertion messages reworded to reflect that the hook is known to have run by then. - test/AGENTS.md updated to describe the generated manifest + marker. The generated hooks dir is a temp dir, always cleaned up; on failure actual_dir is what stays behind for inspection, matching the rest of the suite. Signed-off-by: Leon Li <2182521+leon-li-inspire@users.noreply.github.com>
1 parent 2667972 commit 4186f8b

4 files changed

Lines changed: 136 additions & 66 deletions

File tree

test/AGENTS.md

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -374,18 +374,25 @@ behavior in this repository.
374374

375375
**Pre-GUI hook case (`test_pre_gui_hook`):** this one does *not* use the `_CASES`
376376
/ golden-bundle model — it's a standalone test asserting only the fields a
377-
pre-GUI hook owns. A hook fixture lives in `fixtures/pregui_hooks/` (a
378-
`hooks.yaml`, version `"1.0"`, plus `pregui_hook.py`, which reads the job
379-
metadata on stdin and emits `name` / `description` / `parameters` as JSON on
380-
stdout). The test enables `settings.allow_environment_hooks` (so the submitter
381-
sources `DEADLINE_HOOKS_DIR`) and `settings.auto_accept` (so hooks run without
382-
the Qt confirmation prompt) in the config the `deadline_farm` fixture wrote,
383-
points `DEADLINE_HOOKS_DIR` at the fixture via the launch env, Exports, and
384-
asserts the emitted `name`/`description` reached `template.yaml` and
385-
`deadline:priority` reached `parameter_values.yaml`. To change what the hook
386-
injects, edit `pregui_hook.py` and the `_HOOK_*` constants in `test_cinema4d.py`
387-
together (they're the paired source of truth). It has no `expected/job_bundle/`
388-
and never renders, so it needs no golden capture and runs on macOS too.
377+
pre-GUI hook owns. The hook script `fixtures/pregui_hooks/pregui_hook.py` reads
378+
the job metadata on stdin and emits `name` / `description` / `parameters` as JSON
379+
on stdout. Its `hooks.yaml` (version `"1.0"`) is *not* committed: the test
380+
generates it per-run (`_materialize_pregui_hooks_dir`) with `command` set to
381+
`sys.executable`, because deadline-cloud resolves a hook `command` via absolute
382+
path / hooks-dir / `shutil.which` with no env-var expansion, so a static `python`
383+
isn't a portable interpreter (absent on macOS; PATH-dependent everywhere) and the
384+
hook would silently not run. The test enables `settings.allow_environment_hooks`
385+
(so the submitter sources `DEADLINE_HOOKS_DIR`) and `settings.auto_accept` (so
386+
hooks run without the Qt confirmation prompt) in the config the `deadline_farm`
387+
fixture wrote, points `DEADLINE_HOOKS_DIR` at that generated dir via the launch
388+
env, Exports, then asserts a marker file proves the hook actually ran before
389+
asserting the emitted `name`/`description` reached `template.yaml` and
390+
`deadline:priority` reached `parameter_values.yaml` (the marker separates "hook
391+
never launched" from "hook ran but output wasn't wired in"). To change what the
392+
hook injects, edit `pregui_hook.py` and the `_HOOK_*` constants in
393+
`test_cinema4d.py` together (they're the paired source of truth). It has no
394+
`expected/job_bundle/` and never renders, so it needs no golden capture and runs
395+
on macOS too.
389396

390397
## Installer Tests
391398

test/integ/fixtures/pregui_hooks/hooks.yaml

Lines changed: 0 additions & 10 deletions
This file was deleted.

test/integ/fixtures/pregui_hooks/pregui_hook.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,13 @@
1111
wires ``run_pre_gui_hooks`` + ``apply_pre_gui_output`` correctly (PR #480). This case has no
1212
``expected/job_bundle/`` golden dir; the assertions compare against the ``_HOOK_*`` constants in
1313
``test/integ/test_cinema4d.py`` instead, so keep the emitted values in sync with those constants.
14+
15+
The test also sets ``DEADLINE_CLOUD_PREGUI_MARKER``; when present this script writes that file, so
16+
the test can assert the hook actually ran (separately from asserting its output was applied).
1417
"""
1518

1619
import json
20+
import os
1721
import sys
1822

1923
# Consume the metadata C4D passes on stdin (jobName, parameters, submitterName, ...). We do not
@@ -27,6 +31,18 @@
2731
# No usable metadata on stdin; the fixture's output does not depend on it, so continue.
2832
pass
2933

34+
# Signal that this hook actually executed. The integ test points DEADLINE_CLOUD_PREGUI_MARKER at a
35+
# path it checks after export, so a run that yields the wrong bundle can be told apart as "the hook
36+
# never launched" (discovery / interpreter resolution failure) rather than "the hook ran but its
37+
# output wasn't wired in". Best-effort: marker I/O must never break the hook's stdout contract.
38+
_marker = os.environ.get("DEADLINE_CLOUD_PREGUI_MARKER")
39+
if _marker:
40+
try:
41+
with open(_marker, "w", encoding="utf-8") as _fh:
42+
_fh.write("ran")
43+
except OSError:
44+
pass
45+
3046
output = {
3147
"name": "PREGUI RAN",
3248
"description": "populated by pre-GUI hook",

test/integ/test_cinema4d.py

Lines changed: 101 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
import pytest
1313
import xa11y
14-
from yaml import safe_load
14+
from yaml import safe_dump, safe_load
1515
from deadline_test_fixtures.job_bundle import (
1616
JobBundleCase,
1717
assert_valid_job_bundle,
@@ -767,17 +767,50 @@ def test_job_specific_take_selection(
767767
)
768768

769769

770-
# The directory holding the pre-GUI hook fixture (hooks.yaml + pregui_hook.py). Pointed at by
771-
# DEADLINE_HOOKS_DIR when launching C4D for the hook case; gated by settings.allow_environment_hooks.
772-
_PREGUI_HOOKS_DIR = Path(__file__).parent / "fixtures" / "pregui_hooks"
770+
# The committed pre-GUI hook script. Its hooks.yaml is NOT committed: it is generated per-run by
771+
# _materialize_pregui_hooks_dir so the hook's `command` can be this interpreter's absolute path
772+
# (see that helper for why a static `command` is not portable). DEADLINE_HOOKS_DIR points at the
773+
# generated dir; the run is gated by settings.allow_environment_hooks.
774+
_PREGUI_HOOK_SCRIPT = Path(__file__).parent / "fixtures" / "pregui_hooks" / "pregui_hook.py"
773775

774-
# The values fixtures/pregui_hooks/pregui_hook.py emits. Kept next to the test as the single source
775-
# of truth for the assertions; must match that script.
776+
# The values pregui_hook.py emits. Kept next to the test as the single source of truth for the
777+
# assertions; must match that script.
776778
_HOOK_JOB_NAME = "PREGUI RAN"
777779
_HOOK_DESCRIPTION = "populated by pre-GUI hook"
778780
_HOOK_PRIORITY = 88
779781

780782

783+
def _materialize_pregui_hooks_dir(dest: Path) -> None:
784+
"""Write a ``hooks.yaml`` under ``dest`` that runs the committed ``pregui_hook.py`` with the
785+
interpreter currently running the tests (``sys.executable``).
786+
787+
The manifest cannot be a static committed file with a portable ``command``. deadline-cloud
788+
resolves a hook ``command`` as an absolute path, then relative to the hooks dir, then via
789+
``shutil.which`` on PATH (``deadline.client.job_bundle._hooks._executor``) — it does not expand
790+
environment variables. A bare ``python`` is not a reliable interpreter name: on macOS only
791+
``python3`` exists (the system ``python`` went away with Python 2), and on every platform
792+
whether it resolves depends on the PATH Cinema 4D happened to inherit, not on this fixture. When
793+
it fails to resolve, the hook silently never runs and the failure surfaces as the value
794+
assertions below failing rather than a clear "hook could not be launched".
795+
796+
Writing ``sys.executable`` (an absolute path that exists on this machine — and ``pregui_hook.py``
797+
needs only the stdlib, so any interpreter works) makes the hook resolve on every platform.
798+
``args`` references the committed script by absolute path, so only ``hooks.yaml`` lives here.
799+
"""
800+
dest.mkdir(parents=True, exist_ok=True)
801+
manifest = {
802+
"version": "1.0",
803+
"preGUI": [
804+
{
805+
"command": sys.executable,
806+
"args": [str(_PREGUI_HOOK_SCRIPT)],
807+
"timeout": 60,
808+
}
809+
],
810+
}
811+
(dest / "hooks.yaml").write_text(safe_dump(manifest, sort_keys=False), encoding="utf-8")
812+
813+
781814
def _bundle_parameter_values(actual_dir: Path) -> dict:
782815
"""Read the exported bundle's parameter_values.yaml into a {name: value} dict."""
783816
params_file = actual_dir / "parameter_values.yaml"
@@ -795,9 +828,10 @@ def test_pre_gui_hook(
795828
This is the GUI counterpart to the headless ``test_pre_gui_hooks`` unit tests: those check the
796829
``apply_pre_gui_output`` mapping in isolation, whereas this drives the *real* submitter end to
797830
end and proves the wiring holds through an actual export. It launches Cinema 4D with
798-
``DEADLINE_HOOKS_DIR`` pointing at ``fixtures/pregui_hooks`` and ``allow_environment_hooks`` /
799-
``auto_accept`` enabled, so the shipped submitter runs the hook (``run_pre_gui_hooks``) before
800-
building the dialog and applies its output (``apply_pre_gui_output``). The hook emits a fixed
831+
``DEADLINE_HOOKS_DIR`` pointing at a per-run hooks dir (see ``_materialize_pregui_hooks_dir``)
832+
and ``allow_environment_hooks`` / ``auto_accept`` enabled, so the shipped submitter runs the
833+
hook (``run_pre_gui_hooks``) before building the dialog and applies its output
834+
(``apply_pre_gui_output``). A marker file proves the hook actually ran. The hook emits a fixed
801835
name, description, and ``deadline:priority``; the same Export path the render cases use then
802836
writes a job bundle, and we assert those three values survived into it:
803837
@@ -815,44 +849,67 @@ def test_pre_gui_hook(
815849

816850
scene_path = _build_cinema4d_scene(cinema4d_location, case_folder, actual_dir, case)
817851

818-
# Enable the env-hook path in the config the fixture wrote, and point C4D at the hooks dir.
852+
# Enable the env-hook path in the config the fixture wrote.
819853
_enable_environment_hooks(deadline_farm["env_overlay"])
820854

821-
_export_job_bundle_via_submitter(
822-
cinema4d_location=cinema4d_location,
823-
scene_path=scene_path,
824-
job_bundle_generated=actual_dir,
825-
deadline_farm=deadline_farm,
826-
configure=None,
827-
extra_env={"DEADLINE_HOOKS_DIR": str(_PREGUI_HOOKS_DIR)},
828-
)
855+
# Generate the hooks dir (so `command` is sys.executable, resolvable on every platform) and
856+
# give the hook a marker path so we can prove it actually ran. The dir is temporary and always
857+
# cleaned up; on failure `actual_dir` (below) is what stays behind for inspection.
858+
hooks_dir = Path(tempfile.mkdtemp(prefix="c4d-pregui-hooks-"))
859+
marker_path = hooks_dir / "hook_ran.marker"
860+
try:
861+
_materialize_pregui_hooks_dir(hooks_dir)
862+
863+
_export_job_bundle_via_submitter(
864+
cinema4d_location=cinema4d_location,
865+
scene_path=scene_path,
866+
job_bundle_generated=actual_dir,
867+
deadline_farm=deadline_farm,
868+
configure=None,
869+
extra_env={
870+
"DEADLINE_HOOKS_DIR": str(hooks_dir),
871+
"DEADLINE_CLOUD_PREGUI_MARKER": str(marker_path),
872+
},
873+
)
829874

830-
# The submitter reached the mock, not real AWS. The hook itself is a local subprocess that runs
831-
# before any AWS call, so no new mock routes are needed; still confirm nothing hit an unmocked
832-
# route (a hook misfire that changed the submit path would surface here).
833-
backend = deadline_farm["backend"]
834-
log(f"mock backend call_counts: {dict(backend.call_counts)}")
835-
assert (
836-
backend.unmatched_requests == []
837-
), f"submitter hit routes the mock doesn't implement: {backend.unmatched_requests}"
875+
# The submitter reached the mock, not real AWS. The hook itself is a local subprocess that
876+
# runs before any AWS call, so no new mock routes are needed; still confirm nothing hit an
877+
# unmocked route (a hook misfire that changed the submit path would surface here).
878+
backend = deadline_farm["backend"]
879+
log(f"mock backend call_counts: {dict(backend.call_counts)}")
880+
assert (
881+
backend.unmatched_requests == []
882+
), f"submitter hit routes the mock doesn't implement: {backend.unmatched_requests}"
883+
884+
# Prove the hook subprocess ran at all before trusting the exported values. This separates
885+
# "the hook never launched" (discovery / interpreter resolution failure) from "the hook ran
886+
# but its output wasn't wired into the bundle" — the value assertions below can't tell those
887+
# two apart on their own.
888+
assert marker_path.is_file(), (
889+
f"pre-GUI hook never ran: no marker at {marker_path} (DEADLINE_HOOKS_DIR={hooks_dir}). "
890+
"The submitter didn't execute the hook subprocess — look at hook discovery / "
891+
"interpreter resolution, not the output-mapping wiring."
892+
)
838893

839-
# The exported bundle must carry the hook's output.
840-
assert_valid_job_bundle(actual_dir / "template.yaml")
841-
template = safe_load((actual_dir / "template.yaml").read_text(encoding="utf-8"))
842-
params = _bundle_parameter_values(actual_dir)
894+
# The exported bundle must carry the hook's output.
895+
assert_valid_job_bundle(actual_dir / "template.yaml")
896+
template = safe_load((actual_dir / "template.yaml").read_text(encoding="utf-8"))
897+
params = _bundle_parameter_values(actual_dir)
843898

844-
assert template.get("name") == _HOOK_JOB_NAME, (
845-
f"pre-GUI hook name did not reach the bundle: template name is "
846-
f"{template.get('name')!r}, expected {_HOOK_JOB_NAME!r}"
847-
)
848-
assert template.get("description") == _HOOK_DESCRIPTION, (
849-
f"pre-GUI hook description did not reach the bundle: template description is "
850-
f"{template.get('description')!r}, expected {_HOOK_DESCRIPTION!r}"
851-
)
852-
assert params.get("deadline:priority") == _HOOK_PRIORITY, (
853-
f"pre-GUI hook priority did not reach the bundle: parameter_values has "
854-
f"{params.get('deadline:priority')!r}, expected {_HOOK_PRIORITY}"
855-
)
899+
assert template.get("name") == _HOOK_JOB_NAME, (
900+
f"pre-GUI hook ran but its name did not reach the bundle: template name is "
901+
f"{template.get('name')!r}, expected {_HOOK_JOB_NAME!r}"
902+
)
903+
assert template.get("description") == _HOOK_DESCRIPTION, (
904+
f"pre-GUI hook ran but its description did not reach the bundle: template description is "
905+
f"{template.get('description')!r}, expected {_HOOK_DESCRIPTION!r}"
906+
)
907+
assert params.get("deadline:priority") == _HOOK_PRIORITY, (
908+
f"pre-GUI hook ran but its priority did not reach the bundle: parameter_values has "
909+
f"{params.get('deadline:priority')!r}, expected {_HOOK_PRIORITY}"
910+
)
856911

857-
# Clean up if the test was successful
858-
rmtree(actual_dir, ignore_errors=True)
912+
# Clean up if the test was successful
913+
rmtree(actual_dir, ignore_errors=True)
914+
finally:
915+
rmtree(hooks_dir, ignore_errors=True)

0 commit comments

Comments
 (0)