Skip to content
11 changes: 5 additions & 6 deletions src/deadline/client/ui/job_bundle_submitter.py
Original file line number Diff line number Diff line change
Expand Up @@ -411,18 +411,17 @@ def on_create_job_bundle_callback(
)

if job_parameters:
# We want to validate the job parameters after the queue parameters are loaded.
# Connect a parameter validation function to the queue parameter loading completion
def validate_parameters_after_queue_load(refresh_id: int, queue_parameters: list):

def validate_parameters_after_queue_load(queue_parameters: list):
"""Validate CLI parameters against loaded queue parameters and set parameter values"""
if not _validate_and_warn_about_parameters(
job_parameters, initial_settings.parameters, queue_parameters, submitter_dialog
):
# User chose to cancel, close the dialog
# User cancelled at the validation warning.
submitter_dialog.close()

# Connect to the queue parameters update signal
submitter_dialog.shared_job_settings._queue_parameters_update.connect(
# Validate CLI params once the controller finishes loading queue params.
submitter_dialog.shared_job_settings._controller.queue_parameters_updated.connect(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This now connects to the global singleton controller (DeadlineUIController.getInstance()) instead of a per-widget signal, which changes the lifecycle and firing semantics in two ways worth checking:

  1. Fires more than once, including with an empty list. queue_parameters_updated is emitted whenever queue params reload — not just after the initial load. In particular the controller emits queue_parameters_updated.emit([]) on farm/queue selection changes (select_farm, _deadline_controller.py:547), when no farm/queue is selected (:475), and on fetch error (:509). Each such emission re-runs validate_parameters_after_queue_load with queue_parameters=[], so any CLI --parameter that is actually a queue parameter (not in the job template) gets flagged as "unrecognized," popping the warning dialog spuriously — and closing the whole submitter dialog if the user answers No. A user simply switching farm/queue in the open dialog can trigger this.

  2. Never disconnected. Because the singleton outlives the dialog, this connection is never torn down. validate_parameters_after_queue_load captures submitter_dialog in its closure, so after the dialog is closed the slot keeps firing against a stale/closed dialog (and connections accumulate if multiple submitter dialogs are opened over the singleton's lifetime).

Consider connecting with a single-shot semantics (disconnect after the first non-empty load) or gating on if queue_parameters: / dialog liveness, and/or scoping the connection to the dialog's lifetime.

@crowecawcaw crowecawcaw Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The validation callback skips clearing emissions (empty queue_parameters), so farm/queue switches, no-selection, and fetch-error paths do not trigger a spurious "unrecognized parameters" warning. It runs single-shot (disconnects after validating) and is torn down on submitter_dialog.destroyed, so it cannot fire against a stale dialog or accumulate connections on the singleton controller.

validate_parameters_after_queue_load
)

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.

"""GUI test for the ``bundle gui-submit --parameter`` validation wiring.

Verifies ``show_job_bundle_submitter`` connects the CLI ``--parameter`` validation callback to
the controller's ``queue_parameters_updated`` signal. Uses a real ``SharedJobSettingsWidget``
(not a MagicMock) so the connection is actually resolved — a mocked dialog would let a broken
connect pass silently.
"""

import os

from unittest.mock import MagicMock, patch

import pytest

from deadline.client.ui.controllers._deadline_controller import DeadlineUIController
from deadline.client.ui.controllers._thread_pool import DeadlineThreadPool
from deadline.client.ui.dataclasses import JobBundleSettings
from deadline.client.ui.job_bundle_submitter import show_job_bundle_submitter
from deadline.client.ui.widgets.shared_job_settings_tab import SharedJobSettingsWidget

MODULE = "deadline.client.ui.job_bundle_submitter"


@pytest.fixture(autouse=True)
def _reset_singletons():
"""Reset UI singletons before/after each test so the controller is clean."""
DeadlineUIController.resetInstance()
DeadlineThreadPool.reset()
yield
DeadlineUIController.resetInstance()
DeadlineThreadPool.shutdown(wait_for_done=True, timeout_ms=2000)
DeadlineThreadPool.reset()


def _make_bundle(tmp_path):
bundle_dir = str(tmp_path / "bundle")
os.makedirs(bundle_dir)
with open(os.path.join(bundle_dir, "template.yaml"), "w") as f:
f.write("name: Bundle Job\nsteps: []\n")
return bundle_dir


class TestGuiSubmitCliParameterValidationWiring:
"""The --parameter validation callback is wired to the real queue-parameters signal."""

def _run(self, qtbot, tmp_path, *, job_parameters, validate_side_effect, emit_after=None):
"""Drive show_job_bundle_submitter with a REAL SharedJobSettingsWidget standing in for
the dialog's shared_job_settings. No farm/queue is configured (fresh_deadline_config),
so the widget does not kick off a background load.

If ``emit_after`` is provided, the controller's queue_parameters_updated signal is
emitted with it *inside* the patched context so the connected validation callback runs
against the mocked ``_validate_and_warn_about_parameters``. Returns
(dialog, widget, validate_mock)."""
bundle_dir = _make_bundle(tmp_path)

settings = JobBundleSettings(input_job_bundle_dir=bundle_dir, name="n")
real_widget = SharedJobSettingsWidget(
initial_settings=settings, initial_shared_parameter_values={}
)
qtbot.addWidget(real_widget)

class FakeDialog:
def __init__(self, **kwargs):
self.shared_job_settings = real_widget
self.closed = False

def show(self):
pass

def close(self):
self.closed = True

template = {"name": "Bundle Job", "steps": []}
validate_mock = MagicMock(side_effect=validate_side_effect)

with (
patch(f"{MODULE}.validate_directory_symlink_containment"),
patch(
f"{MODULE}.read_yaml_or_json_object",
side_effect=lambda _dir, name, *a, **k: template if name == "template" else None,
),
patch(f"{MODULE}.read_job_bundle_parameters", return_value=[]),
patch(f"{MODULE}.run_pre_gui_hooks", return_value={}),
patch(f"{MODULE}.SubmitJobToDeadlineDialog", side_effect=FakeDialog),
patch(f"{MODULE}.QApplication"),
patch(f"{MODULE}.QMessageBox"),
patch(f"{MODULE}._get_setting", side_effect=lambda name, config=None: "false"),
patch(f"{MODULE}._config_file") as cfg,
patch(f"{MODULE}._validate_and_warn_about_parameters", validate_mock),
):
cfg.str2bool.side_effect = lambda v: str(v).lower() == "true"
dialog = show_job_bundle_submitter(
input_job_bundle_dir=bundle_dir, job_parameters=job_parameters
)
# Emitting inside the patched block keeps _validate_and_warn_about_parameters mocked
# when the connected callback fires.
if emit_after is not None:
real_widget._controller.queue_parameters_updated.emit(emit_after)
return dialog, real_widget, validate_mock

def test_parameter_path_does_not_raise_attribute_error(
self, qtbot, fresh_deadline_config, tmp_path
):
"""Wiring the --parameter path must not raise AttributeError (the C6 hard crash)."""
dialog, _widget, _validate = self._run(
qtbot,
tmp_path,
job_parameters=[{"name": "Foo", "value": "bar"}],
validate_side_effect=lambda *a, **k: True,
)
assert dialog is not None

def test_queue_parameters_update_invokes_validator_with_param_list(
self, qtbot, fresh_deadline_config, tmp_path
):
"""Emitting the controller's queue_parameters_updated signal runs the validator with the
emitted queue-parameter list."""
queue_parameters = [{"name": "CondaChannels", "type": "STRING"}]
_dialog, _widget, validate_mock = self._run(
qtbot,
tmp_path,
job_parameters=[{"name": "Foo", "value": "bar"}],
validate_side_effect=lambda *a, **k: True,
emit_after=queue_parameters,
)

validate_mock.assert_called_once()
# Signature: (job_parameters, job_template_parameters, queue_parameters, parent_widget)
assert validate_mock.call_args.args[2] == queue_parameters

def test_validator_cancel_closes_dialog(self, qtbot, fresh_deadline_config, tmp_path):
"""When the validator returns False (user cancels), the dialog is closed."""
dialog, _widget, _validate = self._run(
qtbot,
tmp_path,
job_parameters=[{"name": "Foo", "value": "bar"}],
validate_side_effect=lambda *a, **k: False,
emit_after=[{"name": "Foo"}],
)

assert dialog.closed is True
Loading