Skip to content

Commit c5185b8

Browse files
refactor: Address pre-GUI hook PR review in the Cinema 4D submitter
Mirror the review fixes applied to the Maya submitter (deadline-cloud-for-maya PR #437): - Extract the auto_accept confirm-callback selection into _pre_gui_hook_confirm_callback so the branch can be unit-tested headlessly. It returns None (run hooks without prompting) when settings.auto_accept is enabled, otherwise qt_hook_confirmation(parent). - Add unit tests for both paths: auto_accept enabled -> None, and auto_accept disabled -> the confirmation dialog actually fires (QMessageBox.question is invoked, parented to the passed-in window, and a "Yes" reply maps to True). The disabled-path test exercises the real qt_hook_confirmation callback rather than mocking it out. The other Maya review items do not require changes here: the pylint disable on the pre_gui_hooks import and the redundant pyproject version comment were already removed, C4D's test never used pytest.importorskip, and deadline exposes no constant for the "settings.auto_accept" config name (core itself uses the bare string). Signed-off-by: Leon Li <2182521+leon-li-inspire@users.noreply.github.com>
1 parent cc8b9a7 commit c5185b8

2 files changed

Lines changed: 58 additions & 7 deletions

File tree

src/deadline/cinema4d_submitter/cinema4d_render_submitter.py

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -940,6 +940,19 @@ def export_to_temp_folder(temp_dir: str, asset_references: AssetReferences) -> N
940940
asset_references.input_filenames = temp_assets
941941

942942

943+
def _pre_gui_hook_confirm_callback(parent):
944+
"""Choose the confirmation callback for pre-GUI hooks based on the auto_accept setting.
945+
946+
Returns ``None`` (run hooks without prompting) when ``settings.auto_accept`` is enabled,
947+
otherwise the standard Qt confirmation dialog from ``qt_hook_confirmation``. Kept as a small
948+
helper so the auto_accept branch can be unit-tested headlessly.
949+
"""
950+
if str2bool(get_setting("settings.auto_accept")):
951+
return None
952+
953+
return qt_hook_confirmation(parent)
954+
955+
943956
def _show_submitter(temp_dir: str, parent=None, f=Qt.WindowType.Tool): # type: ignore[call-overload]
944957
"""
945958
Creates and returns a submission dialog for rendering jobs.
@@ -1033,17 +1046,14 @@ def on_create_job_bundle_callback(
10331046
# no on-disk job bundle at this point, so hooks are sourced from DEADLINE_HOOKS_DIR only
10341047
# (bundle_dir=None), gated by settings.allow_environment_hooks. The confirmation prompt is
10351048
# skipped when auto_accept is set; otherwise the standard dialog is shown.
1036-
confirm_callback = (
1037-
None if str2bool(get_setting("settings.auto_accept")) else qt_hook_confirmation(parent)
1038-
)
10391049
pre_gui_output = run_pre_gui_hooks(
10401050
PreGuiHookContext(
10411051
bundle_dir=None,
10421052
job_name=render_settings.name,
10431053
submitter_name="cinema4d",
10441054
parameters=dict(shared_parameter_values),
10451055
),
1046-
confirm_callback=confirm_callback,
1056+
confirm_callback=_pre_gui_hook_confirm_callback(parent),
10471057
)
10481058
# RenderSubmitterUISettings has no `.parameters` list, so apply_pre_gui_output routes
10491059
# name/description onto it and every hook parameter into shared_parameter_values.

test/unit/deadline_submitter_for_cinema4d/test_pre_gui_hooks.py

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,24 @@
55
``_show_submitter`` calls deadline-cloud's ``run_pre_gui_hooks`` (env-only, since Cinema 4D has
66
no on-disk bundle at pre-GUI time) and then applies the merged output with deadline-cloud's
77
generic ``apply_pre_gui_output``. The full submitter needs a running Cinema 4D, so it is
8-
exercised in the integration suite; here we verify the DCC-relevant contract headless: Cinema
9-
4D's ``RenderSubmitterUISettings`` has no ``.parameters`` list, so every hook parameter must
10-
flow to the dialog's shared parameter values (name/description land on the settings object).
8+
exercised in the integration suite; here we verify the DCC-owned pieces headless:
9+
10+
* ``apply_pre_gui_output`` routes hook output correctly against Cinema 4D's own
11+
``RenderSubmitterUISettings`` — which has no ``.parameters`` list, so every hook parameter must
12+
land in the shared parameter values (name/description land on the settings object). This guards
13+
against a regression where ``RenderSubmitterUISettings`` gains a ``parameters`` attribute that
14+
would misroute hook params.
15+
* ``_pre_gui_hook_confirm_callback`` honours the ``settings.auto_accept`` setting.
16+
1117
The c4d / Qt modules are stubbed by ``test/unit/deadline_submitter_for_cinema4d/__init__`` so
1218
the module imports.
1319
"""
1420

21+
from unittest.mock import patch
22+
1523
from deadline.client.ui.pre_gui_hooks import apply_pre_gui_output
1624

25+
from deadline.cinema4d_submitter import cinema4d_render_submitter
1726
from deadline.cinema4d_submitter.data_classes import RenderSubmitterUISettings
1827

1928

@@ -81,3 +90,35 @@ def test_partial_output_only_touches_present_keys():
8190
assert settings.name == "NewName"
8291
assert settings.description == "keep me" # not overwritten
8392
assert shared == {} # no parameters in output
93+
94+
95+
@patch.object(cinema4d_render_submitter, "get_setting", return_value="true")
96+
def test_confirm_callback_none_when_auto_accept_enabled(mock_get_setting):
97+
"""With settings.auto_accept enabled, hooks run without a confirmation prompt."""
98+
assert cinema4d_render_submitter._pre_gui_hook_confirm_callback(parent=None) is None
99+
mock_get_setting.assert_called_once_with("settings.auto_accept")
100+
101+
102+
@patch("qtpy.QtWidgets.QMessageBox")
103+
@patch.object(cinema4d_render_submitter, "get_setting", return_value="false")
104+
def test_confirmation_dialog_fires_when_auto_accept_disabled(mock_get_setting, mock_msgbox):
105+
"""With settings.auto_accept disabled, invoking the returned callback actually shows the
106+
confirmation dialog (QMessageBox.question), parented to the passed-in window.
107+
108+
This exercises the real ``qt_hook_confirmation`` callback rather than mocking it out, so it
109+
verifies the prompt fires — not merely that a non-None callback was selected. ``run_pre_gui_hooks``
110+
invokes ``confirm_callback(sources)`` with the hook sources; an empty list is enough to reach
111+
the dialog. The user's answer is mapped from the QMessageBox reply.
112+
"""
113+
mock_msgbox.question.return_value = mock_msgbox.Yes
114+
115+
callback = cinema4d_render_submitter._pre_gui_hook_confirm_callback(parent="mainwin")
116+
assert callback is not None
117+
118+
result = callback([]) # no hook sources needed to reach the dialog
119+
120+
assert mock_msgbox.question.call_count == 1
121+
# The dialog is parented to the window passed into the submitter.
122+
assert mock_msgbox.question.call_args[0][0] == "mainwin"
123+
# "Yes" reply → proceed.
124+
assert result is True

0 commit comments

Comments
 (0)