From e7f7513b922da159adc7e69b8dc39e7620095cba Mon Sep 17 00:00:00 2001 From: Leon Li <2182521+leon-li-inspire@users.noreply.github.com> Date: Wed, 8 Jul 2026 04:22:18 +0000 Subject: [PATCH 1/3] feat: Run pre-GUI hooks in the VRED render submitter Call deadline-cloud's run_pre_gui_hooks before building SubmitJobToDeadlineDialog so studios can pre-populate dialog fields. VRED has no on-disk job bundle at pre-GUI time, so hooks are sourced from DEADLINE_HOOKS_DIR only (bundle_dir=None); the confirmation prompt is skipped when settings.auto_accept is set. The merged hook output is applied with deadline-cloud's generic apply_pre_gui_output. RenderSubmitterUISettings has no .parameters list (unlike the standalone submitter's JobBundleSettings), so that helper writes name/description onto the settings and routes every hook parameter into the dialog's shared parameter values. The hooks run in _create_submitter_dialog, so they fire each time the submitter opens. Bumps the deadline floor to >= 0.60.1 (the release that ships run_pre_gui_hooks / PreGuiHookContext / apply_pre_gui_output via aws-deadline/deadline-cloud#1255; they are absent from 0.60.0) and adds headless unit tests that exercise the real core helper against VRED's settings dataclass. Signed-off-by: Leon Li <2182521+leon-li-inspire@users.noreply.github.com> --- pyproject.toml | 5 +- src/deadline/vred_submitter/vred_submitter.py | 30 ++++++ test/unit/test_pre_gui_hooks.py | 92 +++++++++++++++++++ 3 files changed, 126 insertions(+), 1 deletion(-) create mode 100644 test/unit/test_pre_gui_hooks.py diff --git a/pyproject.toml b/pyproject.toml index 11e5df2..963a1a3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -24,7 +24,10 @@ classifiers = [ dependencies = [ - "deadline >= 0.59.0,< 0.60", + # run_pre_gui_hooks / PreGuiHookContext / apply_pre_gui_output (used by the pre-GUI hook + # integration in vred_submitter.py) first shipped in deadline-cloud 0.60.1 (aws-deadline/ + # deadline-cloud#1255); they are absent from 0.60.0, so the floor must be >= 0.60.1. + "deadline >= 0.60.1,< 0.61", ] [project.scripts] diff --git a/src/deadline/vred_submitter/vred_submitter.py b/src/deadline/vred_submitter/vred_submitter.py index 0a77691..579813a 100644 --- a/src/deadline/vred_submitter/vred_submitter.py +++ b/src/deadline/vred_submitter/vred_submitter.py @@ -32,6 +32,7 @@ from deadline.client.api import ( get_deadline_cloud_library_telemetry_client, ) +from deadline.client.config import get_setting, str2bool from deadline.client.exceptions import DeadlineOperationError, UserInitiatedCancel from deadline.client.job_bundle._yaml import deadline_yaml_dump from deadline.client.job_bundle.parameters import JobParameter @@ -40,6 +41,12 @@ SubmitJobToDeadlineDialog, JobBundlePurpose, ) +from deadline.client.ui.pre_gui_hooks import ( + apply_pre_gui_output, + PreGuiHookContext, + qt_hook_confirmation, + run_pre_gui_hooks, +) from PySide6.QtCore import Qt @@ -238,6 +245,29 @@ def _create_submitter_dialog( shared_parameter_values = {Constants.CONDA_PACKAGES_JOB_PARAM: conda_packages} if conda_channels: shared_parameter_values[Constants.CONDA_CHANNELS_JOB_PARAM] = conda_channels + + # Run pre-GUI hooks so studios can pre-populate dialog fields before it opens. VRED has + # no on-disk job bundle at this point, so hooks are sourced from DEADLINE_HOOKS_DIR only + # (bundle_dir=None), gated by settings.allow_environment_hooks. The confirmation prompt is + # skipped when auto_accept is set; otherwise the standard dialog is shown. + confirm_callback = ( + None + if str2bool(get_setting("settings.auto_accept")) + else qt_hook_confirmation(self.parent_window) + ) + pre_gui_output = run_pre_gui_hooks( + PreGuiHookContext( + bundle_dir=None, + job_name=render_settings.name, + submitter_name="vred", + parameters=dict(shared_parameter_values), + ), + confirm_callback=confirm_callback, + ) + # RenderSubmitterUISettings has no .parameters list, so apply_pre_gui_output routes every + # hook parameter into shared_parameter_values and writes name/description onto the settings. + apply_pre_gui_output(pre_gui_output, render_settings, shared_parameter_values) + # Need to apply these settings prior in order to ensure that Qt Controls are sized as expected! _global_dpi_scale.factor = get_dpi_scale_factor() submitter_dialog = SubmitJobToDeadlineDialog( diff --git a/test/unit/test_pre_gui_hooks.py b/test/unit/test_pre_gui_hooks.py new file mode 100644 index 0000000..4e7a197 --- /dev/null +++ b/test/unit/test_pre_gui_hooks.py @@ -0,0 +1,92 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + +"""Unit tests for the VRED submitter's pre-GUI hook integration. + +``VREDSubmitter._create_submitter_dialog`` calls deadline-cloud's ``run_pre_gui_hooks`` +(env-only, since VRED has no on-disk bundle) and then maps the merged output onto its own +``RenderSubmitterUISettings`` + the dialog's shared parameter values via deadline-cloud's +generic ``apply_pre_gui_output``. + +The full submitter needs a running VRED and Qt, so it is exercised in the integration suite. +Here we pin the contract that matters for VRED: its ``RenderSubmitterUISettings`` has assignable +``name`` / ``description`` and **no** ``.parameters`` list, so ``apply_pre_gui_output`` must write +name/description onto the settings and route every hook parameter into the shared values dict. +We drive the real core function against the real settings dataclass rather than re-testing core +internals. ``apply_pre_gui_output`` ships in deadline-cloud >= 0.60.0 (the floor this change sets). +""" + +from deadline.client.ui.pre_gui_hooks import apply_pre_gui_output + +from vred_submitter.data_classes import RenderSubmitterUISettings + + +def _settings() -> RenderSubmitterUISettings: + s = RenderSubmitterUISettings() + s.name = "Original" + s.description = "" + return s + + +def test_settings_dataclass_has_no_parameters_list(): + """The premise for VRED's mapping: RenderSubmitterUISettings has no .parameters list, so + apply_pre_gui_output treats every hook parameter as a shared value.""" + assert not hasattr(RenderSubmitterUISettings(), "parameters") + + +def test_name_and_description_applied_to_settings(): + """A hook's name/description overwrite the settings fields (VRED has no .parameters list, + so these land directly on the dataclass).""" + settings = _settings() + shared = {"CondaPackages": "vredcore=2024*"} + + apply_pre_gui_output({"name": "PREGUI RAN", "description": "from pipeline"}, settings, shared) + + assert settings.name == "PREGUI RAN" + assert settings.description == "from pipeline" + + +def test_hook_parameters_merged_into_shared_values(): + """With no template-parameter list, all hook parameters (queue params, deadline: properties) + flow into the shared values the dialog is seeded with, overriding defaults on key collision.""" + settings = _settings() + shared = {"CondaPackages": "vredcore=2024*", "CondaChannels": "deadline-cloud"} + + apply_pre_gui_output( + { + "parameters": { + "deadline:priority": 88, + "CondaPackages": "vredcore=2025* custom_pkg", # overrides the default + } + }, + settings, + shared, + ) + + assert shared["deadline:priority"] == 88 + assert shared["CondaPackages"] == "vredcore=2025* custom_pkg" + assert shared["CondaChannels"] == "deadline-cloud" # untouched keys preserved + + +def test_empty_output_is_a_noop(): + """No pre-GUI hook output leaves the settings and shared values unchanged.""" + settings = _settings() + shared = {"CondaPackages": "pkg"} + + apply_pre_gui_output({}, settings, shared) + + assert settings.name == "Original" + assert settings.description == "" + assert shared == {"CondaPackages": "pkg"} + + +def test_partial_output_only_touches_present_keys(): + """Only the keys present in the output are applied; others keep their prior values.""" + settings = _settings() + settings.description = "keep me" + shared: dict = {} + + apply_pre_gui_output({"name": "NewName"}, settings, shared) + + assert settings.name == "NewName" + assert settings.description == "keep me" # not overwritten + assert shared == {} # no parameters in output From 659c81a8b478b6afc3a6bd060ea13d90a41dbe0b Mon Sep 17 00:00:00 2001 From: Leon Li <2182521+leon-li-inspire@users.noreply.github.com> Date: Fri, 10 Jul 2026 23:05:39 +0000 Subject: [PATCH 2/3] test: cover _create_submitter_dialog pre-GUI wiring; fix version in docstring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback on the pre-GUI hook tests: - Add TestCreateSubmitterDialogPreGuiWiring, unit coverage for the DCC-owned control flow in _create_submitter_dialog that the helper-only tests missed: the auto_accept branch selecting confirm_callback (qt_hook_confirmation vs None), the PreGuiHookContext construction (bundle_dir=None, submitter_name, job_name, parameters), and that apply_pre_gui_output is applied onto the same render_settings / shared values the dialog is seeded with. A regression that dropped the apply call, flipped the auto_accept condition, or mispopulated the context now fails a unit test. deadline-cloud is mocked, so the tests run headless. - Group the existing helper tests under TestApplyPreGuiOutputForVred. - Fix the module docstring: apply_pre_gui_output first ships in 0.60.1 (the floor this change sets), not 0.60.0 — it is absent from 0.60.0. Aligns with the pyproject floor. Signed-off-by: Leon Li <2182521+leon-li-inspire@users.noreply.github.com> --- test/unit/test_pre_gui_hooks.py | 250 +++++++++++++++++++++++--------- 1 file changed, 182 insertions(+), 68 deletions(-) diff --git a/test/unit/test_pre_gui_hooks.py b/test/unit/test_pre_gui_hooks.py index 4e7a197..f0450e2 100644 --- a/test/unit/test_pre_gui_hooks.py +++ b/test/unit/test_pre_gui_hooks.py @@ -8,16 +8,28 @@ generic ``apply_pre_gui_output``. The full submitter needs a running VRED and Qt, so it is exercised in the integration suite. -Here we pin the contract that matters for VRED: its ``RenderSubmitterUISettings`` has assignable -``name`` / ``description`` and **no** ``.parameters`` list, so ``apply_pre_gui_output`` must write -name/description onto the settings and route every hook parameter into the shared values dict. -We drive the real core function against the real settings dataclass rather than re-testing core -internals. ``apply_pre_gui_output`` ships in deadline-cloud >= 0.60.0 (the floor this change sets). +This module covers the DCC-owned pieces at unit level: + +* ``TestApplyPreGuiOutputForVred`` pins the contract that matters for VRED: its + ``RenderSubmitterUISettings`` has assignable ``name`` / ``description`` and **no** + ``.parameters`` list, so ``apply_pre_gui_output`` must write name/description onto the settings + and route every hook parameter into the shared values dict. It drives the real core function + against the real settings dataclass rather than re-testing core internals. +* ``TestCreateSubmitterDialogPreGuiWiring`` covers the wiring added to + ``_create_submitter_dialog`` — the ``auto_accept`` branch selecting the confirm callback, the + ``PreGuiHookContext`` construction, and that ``apply_pre_gui_output`` is applied onto + ``render_settings`` / the shared values before the dialog is built — with deadline-cloud mocked. + +``apply_pre_gui_output`` first ships in deadline-cloud 0.60.1 (the floor this change sets); it is +absent from 0.60.0. """ +from unittest.mock import Mock, patch + from deadline.client.ui.pre_gui_hooks import apply_pre_gui_output from vred_submitter.data_classes import RenderSubmitterUISettings +from vred_submitter.vred_submitter import VREDSubmitter def _settings() -> RenderSubmitterUISettings: @@ -27,66 +39,168 @@ def _settings() -> RenderSubmitterUISettings: return s -def test_settings_dataclass_has_no_parameters_list(): - """The premise for VRED's mapping: RenderSubmitterUISettings has no .parameters list, so - apply_pre_gui_output treats every hook parameter as a shared value.""" - assert not hasattr(RenderSubmitterUISettings(), "parameters") - - -def test_name_and_description_applied_to_settings(): - """A hook's name/description overwrite the settings fields (VRED has no .parameters list, - so these land directly on the dataclass).""" - settings = _settings() - shared = {"CondaPackages": "vredcore=2024*"} - - apply_pre_gui_output({"name": "PREGUI RAN", "description": "from pipeline"}, settings, shared) - - assert settings.name == "PREGUI RAN" - assert settings.description == "from pipeline" - - -def test_hook_parameters_merged_into_shared_values(): - """With no template-parameter list, all hook parameters (queue params, deadline: properties) - flow into the shared values the dialog is seeded with, overriding defaults on key collision.""" - settings = _settings() - shared = {"CondaPackages": "vredcore=2024*", "CondaChannels": "deadline-cloud"} - - apply_pre_gui_output( - { - "parameters": { - "deadline:priority": 88, - "CondaPackages": "vredcore=2025* custom_pkg", # overrides the default - } - }, - settings, - shared, - ) - - assert shared["deadline:priority"] == 88 - assert shared["CondaPackages"] == "vredcore=2025* custom_pkg" - assert shared["CondaChannels"] == "deadline-cloud" # untouched keys preserved - - -def test_empty_output_is_a_noop(): - """No pre-GUI hook output leaves the settings and shared values unchanged.""" - settings = _settings() - shared = {"CondaPackages": "pkg"} - - apply_pre_gui_output({}, settings, shared) - - assert settings.name == "Original" - assert settings.description == "" - assert shared == {"CondaPackages": "pkg"} - - -def test_partial_output_only_touches_present_keys(): - """Only the keys present in the output are applied; others keep their prior values.""" - settings = _settings() - settings.description = "keep me" - shared: dict = {} - - apply_pre_gui_output({"name": "NewName"}, settings, shared) - - assert settings.name == "NewName" - assert settings.description == "keep me" # not overwritten - assert shared == {} # no parameters in output +class TestApplyPreGuiOutputForVred: + """The generic core ``apply_pre_gui_output`` driven against VRED's real settings dataclass.""" + + def test_settings_dataclass_has_no_parameters_list(self): + """The premise for VRED's mapping: RenderSubmitterUISettings has no .parameters list, so + apply_pre_gui_output treats every hook parameter as a shared value.""" + assert not hasattr(RenderSubmitterUISettings(), "parameters") + + def test_name_and_description_applied_to_settings(self): + """A hook's name/description overwrite the settings fields (VRED has no .parameters list, + so these land directly on the dataclass).""" + settings = _settings() + shared = {"CondaPackages": "vredcore=2024*"} + + apply_pre_gui_output( + {"name": "PREGUI RAN", "description": "from pipeline"}, settings, shared + ) + + assert settings.name == "PREGUI RAN" + assert settings.description == "from pipeline" + + def test_hook_parameters_merged_into_shared_values(self): + """With no template-parameter list, all hook parameters (queue params, deadline: + properties) flow into the shared values the dialog is seeded with, overriding defaults on + key collision.""" + settings = _settings() + shared = {"CondaPackages": "vredcore=2024*", "CondaChannels": "deadline-cloud"} + + apply_pre_gui_output( + { + "parameters": { + "deadline:priority": 88, + "CondaPackages": "vredcore=2025* custom_pkg", # overrides the default + } + }, + settings, + shared, + ) + + assert shared["deadline:priority"] == 88 + assert shared["CondaPackages"] == "vredcore=2025* custom_pkg" + assert shared["CondaChannels"] == "deadline-cloud" # untouched keys preserved + + def test_empty_output_is_a_noop(self): + """No pre-GUI hook output leaves the settings and shared values unchanged.""" + settings = _settings() + shared = {"CondaPackages": "pkg"} + + apply_pre_gui_output({}, settings, shared) + + assert settings.name == "Original" + assert settings.description == "" + assert shared == {"CondaPackages": "pkg"} + + def test_partial_output_only_touches_present_keys(self): + """Only the keys present in the output are applied; others keep their prior values.""" + settings = _settings() + settings.description = "keep me" + shared: dict = {} + + apply_pre_gui_output({"name": "NewName"}, settings, shared) + + assert settings.name == "NewName" + assert settings.description == "keep me" # not overwritten + assert shared == {} # no parameters in output + + +# Patch targets live on the submitter module, since it imports these names directly. +_MOD = "vred_submitter.vred_submitter" + + +@patch(f"{_MOD}.SubmitJobToDeadlineDialog") +@patch(f"{_MOD}.get_dpi_scale_factor", return_value=1.0) +@patch(f"{_MOD}.get_major_version", return_value="2024") +@patch(f"{_MOD}.os.getenv", return_value=None) +@patch(f"{_MOD}.apply_pre_gui_output") +@patch(f"{_MOD}.run_pre_gui_hooks", return_value={}) +@patch(f"{_MOD}.qt_hook_confirmation") +@patch(f"{_MOD}.get_setting") +class TestCreateSubmitterDialogPreGuiWiring: + """Unit coverage for the pre-GUI wiring added to ``_create_submitter_dialog``. + + deadline-cloud is mocked (as it is in conftest), so these tests pin the DCC-owned control + flow — the ``auto_accept`` branch, the ``PreGuiHookContext`` build, and applying the merged + output — that ``TestApplyPreGuiOutputForVred`` and the integration suite don't cover. + """ + + @staticmethod + def _submitter(): + with patch(f"{_MOD}.get_yaml_contents", return_value={"steps": []}): + return VREDSubmitter(Mock()) + + def test_hooks_run_and_output_applied_before_dialog( + self, + mock_get_setting, + mock_qt_conf, + mock_run_hooks, + mock_apply, + mock_getenv, + mock_version, + mock_dpi, + mock_dialog, + ): + """run_pre_gui_hooks is invoked with a VRED PreGuiHookContext, and its output is applied + onto the settings + shared values via apply_pre_gui_output before the dialog is built.""" + mock_get_setting.return_value = "false" # auto_accept off + mock_run_hooks.return_value = {"name": "FromHook"} + settings = _settings() + + self._submitter()._create_submitter_dialog(settings, (Mock(), Mock())) + + mock_run_hooks.assert_called_once() + context = mock_run_hooks.call_args.args[0] + assert context.bundle_dir is None # VRED has no on-disk bundle at pre-GUI time + assert context.submitter_name == "vred" + assert context.job_name == settings.name + assert context.parameters["CondaPackages"] == "vredcore=2024*" + + # The merged output is applied onto the same settings + shared-values the dialog receives. + mock_apply.assert_called_once() + applied_output, applied_settings, applied_shared = mock_apply.call_args.args + assert applied_output == {"name": "FromHook"} + assert applied_settings is settings + seeded_shared = mock_dialog.call_args.kwargs["initial_shared_parameter_values"] + assert applied_shared is seeded_shared + + def test_auto_accept_off_uses_qt_confirmation( + self, + mock_get_setting, + mock_qt_conf, + mock_run_hooks, + mock_apply, + mock_getenv, + mock_version, + mock_dpi, + mock_dialog, + ): + """With settings.auto_accept unset, the standard Qt confirmation dialog is the callback.""" + mock_get_setting.return_value = "false" + + self._submitter()._create_submitter_dialog(_settings(), (Mock(), Mock())) + + mock_get_setting.assert_called_once_with("settings.auto_accept") + mock_qt_conf.assert_called_once() + assert mock_run_hooks.call_args.kwargs["confirm_callback"] is mock_qt_conf.return_value + + def test_auto_accept_on_skips_confirmation( + self, + mock_get_setting, + mock_qt_conf, + mock_run_hooks, + mock_apply, + mock_getenv, + mock_version, + mock_dpi, + mock_dialog, + ): + """With settings.auto_accept set, confirm_callback is None (no prompt) and the Qt + confirmation is never constructed.""" + mock_get_setting.return_value = "true" + + self._submitter()._create_submitter_dialog(_settings(), (Mock(), Mock())) + + mock_qt_conf.assert_not_called() + assert mock_run_hooks.call_args.kwargs["confirm_callback"] is None From 65be1a5970295e356496939f24c4521e114ca9e0 Mon Sep 17 00:00:00 2001 From: Leon Li <2182521+leon-li-inspire@users.noreply.github.com> Date: Mon, 13 Jul 2026 21:06:32 +0000 Subject: [PATCH 3/3] refactor: Address PR review on the VRED pre-GUI hook integration Mirror the review fixes made on the Maya submitter (deadline-cloud-for-maya#437): - Extract _pre_gui_hook_confirm_callback(parent): moves the settings.auto_accept branch out of _create_submitter_dialog into a small module-level helper so it can be unit-tested headlessly. - Drop the multi-line dependency comment above the deadline pin in pyproject.toml (the >= 0.60.1 floor speaks for itself now that the release is out). - Tests: add TestPreGuiHookConfirmCallback covering both auto_accept branches, including one that exercises the real qt_hook_confirmation and asserts the QMessageBox confirmation actually fires (parented to the window, Yes -> proceed) rather than only checking a non-None callback was selected. Slim the dialog- wiring test to the context build + confirm-callback delegation + apply wiring. Not applicable to VRED (unlike Maya): VRED imports pre_gui_hooks at module top with no `# pylint: disable=import-error`, and its tests never used pytest.importorskip, so the "stale pylint disable" and "drop the skip" review items have no VRED equivalent. Signed-off-by: Leon Li <2182521+leon-li-inspire@users.noreply.github.com> --- pyproject.toml | 3 - src/deadline/vred_submitter/vred_submitter.py | 19 ++- test/unit/test_pre_gui_hooks.py | 114 +++++++++--------- 3 files changed, 71 insertions(+), 65 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 963a1a3..24775ea 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -24,9 +24,6 @@ classifiers = [ dependencies = [ - # run_pre_gui_hooks / PreGuiHookContext / apply_pre_gui_output (used by the pre-GUI hook - # integration in vred_submitter.py) first shipped in deadline-cloud 0.60.1 (aws-deadline/ - # deadline-cloud#1255); they are absent from 0.60.0, so the floor must be >= 0.60.1. "deadline >= 0.60.1,< 0.61", ] diff --git a/src/deadline/vred_submitter/vred_submitter.py b/src/deadline/vred_submitter/vred_submitter.py index 579813a..e9a6b7f 100644 --- a/src/deadline/vred_submitter/vred_submitter.py +++ b/src/deadline/vred_submitter/vred_submitter.py @@ -55,6 +55,18 @@ _global_logger = get_logger(__name__) +def _pre_gui_hook_confirm_callback(parent): + """Choose the confirmation callback for pre-GUI hooks based on the auto_accept setting. + + Returns ``None`` (run hooks without prompting) when ``settings.auto_accept`` is enabled, + otherwise the standard Qt confirmation dialog from ``qt_hook_confirmation``. Kept as a small + helper so the auto_accept branch can be unit-tested headlessly. + """ + if str2bool(get_setting("settings.auto_accept")): + return None + return qt_hook_confirmation(parent) + + class VREDSubmitter: def __init__(self, parent_window: Any, window_flags: Qt.WindowFlags = Qt.WindowFlags()): @@ -250,11 +262,6 @@ def _create_submitter_dialog( # no on-disk job bundle at this point, so hooks are sourced from DEADLINE_HOOKS_DIR only # (bundle_dir=None), gated by settings.allow_environment_hooks. The confirmation prompt is # skipped when auto_accept is set; otherwise the standard dialog is shown. - confirm_callback = ( - None - if str2bool(get_setting("settings.auto_accept")) - else qt_hook_confirmation(self.parent_window) - ) pre_gui_output = run_pre_gui_hooks( PreGuiHookContext( bundle_dir=None, @@ -262,7 +269,7 @@ def _create_submitter_dialog( submitter_name="vred", parameters=dict(shared_parameter_values), ), - confirm_callback=confirm_callback, + confirm_callback=_pre_gui_hook_confirm_callback(self.parent_window), ) # RenderSubmitterUISettings has no .parameters list, so apply_pre_gui_output routes every # hook parameter into shared_parameter_values and writes name/description onto the settings. diff --git a/test/unit/test_pre_gui_hooks.py b/test/unit/test_pre_gui_hooks.py index f0450e2..b80f5e0 100644 --- a/test/unit/test_pre_gui_hooks.py +++ b/test/unit/test_pre_gui_hooks.py @@ -15,10 +15,13 @@ ``.parameters`` list, so ``apply_pre_gui_output`` must write name/description onto the settings and route every hook parameter into the shared values dict. It drives the real core function against the real settings dataclass rather than re-testing core internals. +* ``TestPreGuiHookConfirmCallback`` covers ``_pre_gui_hook_confirm_callback`` — the + ``settings.auto_accept`` branch — including that the returned callback actually fires the Qt + confirmation dialog (via the real ``qt_hook_confirmation``) when auto_accept is disabled. * ``TestCreateSubmitterDialogPreGuiWiring`` covers the wiring added to - ``_create_submitter_dialog`` — the ``auto_accept`` branch selecting the confirm callback, the - ``PreGuiHookContext`` construction, and that ``apply_pre_gui_output`` is applied onto - ``render_settings`` / the shared values before the dialog is built — with deadline-cloud mocked. + ``_create_submitter_dialog`` — the ``PreGuiHookContext`` construction and that + ``apply_pre_gui_output`` is applied onto ``render_settings`` / the shared values before the + dialog is built — with deadline-cloud mocked. ``apply_pre_gui_output`` first ships in deadline-cloud 0.60.1 (the floor this change sets); it is absent from 0.60.0. @@ -29,7 +32,7 @@ from deadline.client.ui.pre_gui_hooks import apply_pre_gui_output from vred_submitter.data_classes import RenderSubmitterUISettings -from vred_submitter.vred_submitter import VREDSubmitter +from vred_submitter.vred_submitter import _pre_gui_hook_confirm_callback, VREDSubmitter def _settings() -> RenderSubmitterUISettings: @@ -110,20 +113,55 @@ def test_partial_output_only_touches_present_keys(self): _MOD = "vred_submitter.vred_submitter" +class TestPreGuiHookConfirmCallback: + """The ``settings.auto_accept`` branch in ``_pre_gui_hook_confirm_callback``.""" + + @patch(f"{_MOD}.get_setting", return_value="true") + def test_none_when_auto_accept_enabled(self, mock_get_setting): + """With settings.auto_accept enabled, hooks run without a confirmation prompt.""" + assert _pre_gui_hook_confirm_callback(parent=None) is None + mock_get_setting.assert_called_once_with("settings.auto_accept") + + @patch("qtpy.QtWidgets.QMessageBox") + @patch(f"{_MOD}.get_setting", return_value="false") + def test_dialog_fires_when_auto_accept_disabled(self, mock_get_setting, mock_msgbox): + """With settings.auto_accept disabled, invoking the returned callback actually shows the + confirmation dialog (QMessageBox.question), parented to the passed-in window. + + This exercises the real ``qt_hook_confirmation`` callback rather than mocking it out, so it + verifies the prompt fires — not merely that a non-None callback was selected. + ``run_pre_gui_hooks`` invokes ``confirm_callback(sources)`` with the hook sources; an empty + list is enough to reach the dialog. The user's answer maps from the QMessageBox reply. + """ + mock_msgbox.question.return_value = mock_msgbox.Yes + + callback = _pre_gui_hook_confirm_callback(parent="mainwin") + assert callback is not None + + result = callback([]) # no hook sources needed to reach the dialog + + assert mock_msgbox.question.call_count == 1 + # The dialog is parented to the window passed into the submitter. + assert mock_msgbox.question.call_args[0][0] == "mainwin" + # "Yes" reply → proceed. + assert result is True + + @patch(f"{_MOD}.SubmitJobToDeadlineDialog") @patch(f"{_MOD}.get_dpi_scale_factor", return_value=1.0) @patch(f"{_MOD}.get_major_version", return_value="2024") @patch(f"{_MOD}.os.getenv", return_value=None) @patch(f"{_MOD}.apply_pre_gui_output") @patch(f"{_MOD}.run_pre_gui_hooks", return_value={}) -@patch(f"{_MOD}.qt_hook_confirmation") -@patch(f"{_MOD}.get_setting") +@patch(f"{_MOD}._pre_gui_hook_confirm_callback") class TestCreateSubmitterDialogPreGuiWiring: - """Unit coverage for the pre-GUI wiring added to ``_create_submitter_dialog``. + """Unit coverage for the pre-GUI wiring in ``_create_submitter_dialog``. - deadline-cloud is mocked (as it is in conftest), so these tests pin the DCC-owned control - flow — the ``auto_accept`` branch, the ``PreGuiHookContext`` build, and applying the merged - output — that ``TestApplyPreGuiOutputForVred`` and the integration suite don't cover. + deadline-cloud is mocked (as in conftest), so these tests pin the DCC-owned wiring — the + ``PreGuiHookContext`` build, the confirm-callback selection delegated to + ``_pre_gui_hook_confirm_callback``, and applying the merged output — that + ``TestApplyPreGuiOutputForVred`` / ``TestPreGuiHookConfirmCallback`` and the integration suite + don't cover. """ @staticmethod @@ -133,8 +171,7 @@ def _submitter(): def test_hooks_run_and_output_applied_before_dialog( self, - mock_get_setting, - mock_qt_conf, + mock_confirm_cb, mock_run_hooks, mock_apply, mock_getenv, @@ -142,13 +179,14 @@ def test_hooks_run_and_output_applied_before_dialog( mock_dpi, mock_dialog, ): - """run_pre_gui_hooks is invoked with a VRED PreGuiHookContext, and its output is applied - onto the settings + shared values via apply_pre_gui_output before the dialog is built.""" - mock_get_setting.return_value = "false" # auto_accept off + """run_pre_gui_hooks is invoked with a VRED PreGuiHookContext and the confirm callback from + _pre_gui_hook_confirm_callback; its output is applied onto the settings + shared values via + apply_pre_gui_output before the dialog is built.""" mock_run_hooks.return_value = {"name": "FromHook"} settings = _settings() - self._submitter()._create_submitter_dialog(settings, (Mock(), Mock())) + submitter = self._submitter() + submitter._create_submitter_dialog(settings, (Mock(), Mock())) mock_run_hooks.assert_called_once() context = mock_run_hooks.call_args.args[0] @@ -157,6 +195,10 @@ def test_hooks_run_and_output_applied_before_dialog( assert context.job_name == settings.name assert context.parameters["CondaPackages"] == "vredcore=2024*" + # The confirm callback is delegated to the helper, parented to the submitter's window. + mock_confirm_cb.assert_called_once_with(submitter.parent_window) + assert mock_run_hooks.call_args.kwargs["confirm_callback"] is mock_confirm_cb.return_value + # The merged output is applied onto the same settings + shared-values the dialog receives. mock_apply.assert_called_once() applied_output, applied_settings, applied_shared = mock_apply.call_args.args @@ -164,43 +206,3 @@ def test_hooks_run_and_output_applied_before_dialog( assert applied_settings is settings seeded_shared = mock_dialog.call_args.kwargs["initial_shared_parameter_values"] assert applied_shared is seeded_shared - - def test_auto_accept_off_uses_qt_confirmation( - self, - mock_get_setting, - mock_qt_conf, - mock_run_hooks, - mock_apply, - mock_getenv, - mock_version, - mock_dpi, - mock_dialog, - ): - """With settings.auto_accept unset, the standard Qt confirmation dialog is the callback.""" - mock_get_setting.return_value = "false" - - self._submitter()._create_submitter_dialog(_settings(), (Mock(), Mock())) - - mock_get_setting.assert_called_once_with("settings.auto_accept") - mock_qt_conf.assert_called_once() - assert mock_run_hooks.call_args.kwargs["confirm_callback"] is mock_qt_conf.return_value - - def test_auto_accept_on_skips_confirmation( - self, - mock_get_setting, - mock_qt_conf, - mock_run_hooks, - mock_apply, - mock_getenv, - mock_version, - mock_dpi, - mock_dialog, - ): - """With settings.auto_accept set, confirm_callback is None (no prompt) and the Qt - confirmation is never constructed.""" - mock_get_setting.return_value = "true" - - self._submitter()._create_submitter_dialog(_settings(), (Mock(), Mock())) - - mock_qt_conf.assert_not_called() - assert mock_run_hooks.call_args.kwargs["confirm_callback"] is None