fix: connect gui-submit --parameter validation to live queue signal

src/deadline/client/ui/job_bundle_submitter.py

@@ -411,20 +411,54 @@ 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):
+        # The controller is a global singleton that outlives this dialog, and its
+        # queue_parameters_updated signal also fires with [] to clear stale state
+        # (farm/queue switch, nothing selected). A queue may genuinely have zero
+        # queue parameters, so we can't gate on payload non-emptiness — that would
+        # never validate against an empty-but-real load. Instead we recognize a
+        # completed load by the queue_parameters_loading(False) emission that the
+        # controller sends immediately before queue_parameters_updated on fetch
+        # completion (success or error); clearing emissions have no such prefix.
+        # Both emits happen consecutively in the same main-thread slot, so nothing
+        # can interleave between them. Validate single-shot on the first completed
+        # load, then disconnect so the closure over submitter_dialog can't fire
+        # against a closed dialog later.
+        controller = submitter_dialog.shared_job_settings._controller
+        # Mutable cell: True right after a load finishes (loading(False)), reset
+        # when a new load starts (loading(True)).
+        load_finished = [False]
+
+        def track_queue_parameters_loading(is_loading: bool):
+            load_finished[0] = not is_loading
+
+        def disconnect_validation_signals():
+            for signal, slot in (
+                (controller.queue_parameters_loading, track_queue_parameters_loading),
+                (controller.queue_parameters_updated, validate_parameters_after_queue_load),
+            ):
+                try:
+                    signal.disconnect(slot)
+                except (TypeError, RuntimeError):
+                    # Already disconnected (validation ran before the dialog was destroyed).
+                    pass
+
+        def validate_parameters_after_queue_load(queue_parameters: list):
             """Validate CLI parameters against loaded queue parameters and set parameter values"""
+            if not load_finished[0]:
+                # A clearing emission, not a completed load. Keep waiting.
+                return
+            disconnect_validation_signals()
             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_parameters_after_queue_load
-        )
+        # Validate CLI params once the controller finishes loading queue params.
+        controller.queue_parameters_loading.connect(track_queue_parameters_loading)
+        controller.queue_parameters_updated.connect(validate_parameters_after_queue_load)
+        # If the dialog goes away before any load completes, tear the connections down.
+        submitter_dialog.destroyed.connect(disconnect_validation_signals)
 
     submitter_dialog.show()
     return submitter_dialog

test/unit/deadline_client/ui/gui/test_gui_submitter_cli_parameters.py

@@ -0,0 +1,243 @@
+# 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 qtpy.QtCore import QObject  # type: ignore
+
+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, controller signals are emitted *inside* the patched
+        context so the connected validation callback runs against the mocked
+        ``_validate_and_warn_about_parameters``. It is a list of (kind, payload) tuples
+        emitted in sequence, mirroring the controller's real emission patterns:
+
+        - ("load", payload): a completed fetch — queue_parameters_loading(True), then
+          queue_parameters_loading(False), then queue_parameters_updated(payload)
+          (see DeadlineUIController._on_queue_parameters_success/_error).
+        - ("clear", payload): a bare queue_parameters_updated(payload) with no loading
+          prefix, as emitted on farm/queue switch or when nothing is selected.
+
+        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(QObject):
+            # QObject supplies the real ``destroyed`` signal the production code
+            # connects its cleanup to.
+            def __init__(self, **kwargs):
+                super().__init__()
+                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:
+                controller = real_widget._controller
+                for kind, payload in emit_after:
+                    if kind == "load":
+                        # A real fetch: loading toggles True -> False, then updated.
+                        controller.queue_parameters_loading.emit(True)
+                        controller.queue_parameters_loading.emit(False)
+                    else:
+                        assert kind == "clear"
+                    controller.queue_parameters_updated.emit(payload)
+        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
+    ):
+        """A completed queue-parameter load runs the validator with the loaded 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=[("load", 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=[("load", [{"name": "Foo"}])],
+        )
+
+        assert dialog.closed is True
+
+    def test_clearing_emission_does_not_invoke_validator(
+        self, qtbot, fresh_deadline_config, tmp_path
+    ):
+        """A clearing emission ([] with no loading prefix) — e.g. farm/queue switch or nothing
+        selected — must not run validation (which would spuriously flag queue params as
+        unrecognized)."""
+        _dialog, _widget, validate_mock = self._run(
+            qtbot,
+            tmp_path,
+            job_parameters=[{"name": "Foo", "value": "bar"}],
+            validate_side_effect=lambda *a, **k: True,
+            emit_after=[("clear", [])],
+        )
+
+        validate_mock.assert_not_called()
+
+    def test_empty_but_real_load_still_invokes_validator(
+        self, qtbot, fresh_deadline_config, tmp_path
+    ):
+        """A queue that genuinely has zero queue parameters still validates: a completed load
+        (loading True -> False, then updated([])) runs the validator so an unrecognized CLI
+        --parameter is flagged rather than slipping through."""
+        _dialog, _widget, validate_mock = self._run(
+            qtbot,
+            tmp_path,
+            job_parameters=[{"name": "Foo", "value": "bar"}],
+            validate_side_effect=lambda *a, **k: True,
+            emit_after=[("load", [])],
+        )
+
+        validate_mock.assert_called_once()
+        assert validate_mock.call_args.args[2] == []
+
+    def test_validator_runs_single_shot_on_first_completed_load(
+        self, qtbot, fresh_deadline_config, tmp_path
+    ):
+        """Validation waits through clearing emissions, runs once on the first completed load,
+        and disconnects so later reloads don't re-validate."""
+        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=[
+                ("clear", []),
+                ("load", queue_parameters),
+                ("load", [{"name": "Other"}]),
+            ],
+        )
+
+        validate_mock.assert_called_once()
+        assert validate_mock.call_args.args[2] == queue_parameters
+
+    def test_dialog_destroyed_disconnects_validator(self, qtbot, fresh_deadline_config, tmp_path):
+        """Destroying the dialog before queue params load tears down the connection, so the
+        stale closure never fires against the singleton controller."""
+        dialog, widget, validate_mock = self._run(
+            qtbot,
+            tmp_path,
+            job_parameters=[{"name": "Foo", "value": "bar"}],
+            validate_side_effect=lambda *a, **k: True,
+        )
+
+        # Simulate the dialog being destroyed before any load completes.
+        dialog.destroyed.emit()
+
+        with patch(
+            "deadline.client.ui.job_bundle_submitter._validate_and_warn_about_parameters",
+            validate_mock,
+        ):
+            # A full completed-load emission pattern, which would run the validator
+            # if the connections were still live.
+            widget._controller.queue_parameters_loading.emit(True)
+            widget._controller.queue_parameters_loading.emit(False)
+            widget._controller.queue_parameters_updated.emit([{"name": "CondaChannels"}])
+
+        validate_mock.assert_not_called()