Improves and adds tests for Invariant Perspective UI by jellybean2004 · Pull Request #3888 · SasView/sasview

jellybean2004 · 2026-03-02T11:29:37Z

Description

Replaces the monolithic InvariantPerspectiveTest.py with a structured test suite covering the full InvariantWindow lifecycle — from widget initialisation through calculation and plotting. The suite is split into focused modules with shared fixtures in conftest.py.

New test files

File	Coverage
`conftest.py`	Shared fixtures: `DummyManager`, `MainWindowStub`, `window_class`, `real_data`, `small_data`
`InitializedStateTest.py`	Default widget values, validators, tooltips, read-only/disabled states, radio-button groups, progress-bar initial state, `update_from_model`, `updateFromGui`, serialisation, `closeEvent`, `reset`
`InvariantLoadedDataTest.py`	Data loading and removal, duplicate-load guard, invalid-data guards, extrapolation-slider state on load, calculate-button enable/disable logic, invalid-input feedback
`RealDataTest.py`	Real-data setup helpers (`UIHelpersMixin`), Q-range display, low-/high-Q extrapolation limits (getters/setters), bounds and ordering validation, `updateGuiFromFile`, serialisation (`serializePage`, `serializeCurrentPage`, `serializeAll`)
`CalculationTest.py`	`calculate_thread` (base case, contrast/vol-frac modes, exceptions, Porod constant, low/high/both extrapolation success), `plot_result`, `calculate_invariant` (thread dispatch, callback wiring), `on_calculation_failed`, `deferredPlot`, `compute_low`/`compute_high` (function selection, exception handling)

Changes to `InvariantPerspective.py`

Minor refactors to improve testability and correctness:

on_calculation_failed type annotation corrected (Exception → Failure)
calculate_thread extrapolation argument type broadened to str | None
compute_low / compute_high extracted as standalone methods returning (qstar, qstar_err, success) tuples, making them independently testable
set_low_q_extrapolation_upper_limit / set_high_q_extrapolation_lower_limit index calculations expanded for clarity

Removed

InvariantPerspectiveTest.py — superseded by the new suite

note: written using Copilot on VS Code

How Has This Been Tested?

Full suite run with pytest against the UnitTesting/ directory
Coverage collected with pytest --cov; new tests bring the perspective from partial coverage to covering most primary code paths
Manual cross-check of extrapolation, contrast/volume-fraction, and Porod-constant flows against the running application

Review Checklist:

Documentation

There is nothing that needs documenting
Documentation changes are in this PR
There is an issue open for the documentation (link?)

Installers

There is a chance this will affect the installers, if so
- Windows installer (GH artifact) has been tested (installed and worked)
- MacOSX installer (GH artifact) has been tested (installed and worked)
- Wheels installer (GH artifact) has been tested (installed and worked)

Licensing (untick if necessary)

The introduced changes comply with SasView license (BSD 3-Clause)

Copilot

Pull request overview

This PR replaces the legacy monolithic Invariant perspective test with a modular pytest suite and includes small refactors in InvariantPerspective.py to make calculation/extrapolation paths more testable.

Changes:

Added a new pytest suite under Invariant/UnitTesting/ with shared fixtures in conftest.py.
Refactored invariant calculation/extrapolation logic (extracted compute_low/compute_high, adjusted error handling/signatures).
Updated UI and existing tests to align with the refactor (e.g., TabbedInvariantUI.ui, InvariantDetailsTest.py), and removed the old InvariantPerspectiveTest.py.

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`src/sas/qtgui/Perspectives/Invariant/UnitTesting/conftest.py`	Adds shared Qt/test fixtures for the new test suite.
`src/sas/qtgui/Perspectives/Invariant/UnitTesting/InitializedStateTest.py`	Adds coverage for default widget/model state and UI behavior.
`src/sas/qtgui/Perspectives/Invariant/UnitTesting/InvariantLoadedDataTest.py`	Adds coverage around data loading/removal and calculate enablement/validation.
`src/sas/qtgui/Perspectives/Invariant/UnitTesting/RealDataTest.py`	Adds “real data” flow tests including extrapolation validation and serialization.
`src/sas/qtgui/Perspectives/Invariant/UnitTesting/CalculationTest.py`	Adds calculation thread/plotting/error handling tests.
`src/sas/qtgui/Perspectives/Invariant/UnitTesting/InvariantPerspectiveTest.py`	Removes the old monolithic test file.
`src/sas/qtgui/Perspectives/Invariant/UnitTesting/InvariantDetailsTest.py`	Minor string/formatting cleanup.
`src/sas/qtgui/Perspectives/Invariant/UI/TabbedInvariantUI.ui`	Makes `txtName` read-only (UI consistency with tests).
`src/sas/qtgui/Perspectives/Invariant/InvariantPerspective.py`	Refactors calculation/extrapolation + small slot wiring improvements for testability.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-02T11:34:32Z

src/sas/qtgui/Perspectives/Invariant/InvariantPerspective.py

+            if self._porod and self._porod > 0:
+                try:
+                    # choose contrast_for_surface safely
+                    if self.rbContrast.isChecked():
+                        contrast_for_surface = self._contrast
+                        contrast_for_surface_err = self._contrast_err
+                    elif self.rbVolFrac.isChecked() and (contrast_out != "ERROR" and contrast_out_error != "ERROR"):
+                        contrast_for_surface = contrast_out
+                        contrast_for_surface_err = contrast_out_error


In calculate_thread(), contrast_out/contrast_out_error are only defined inside the if self.rbVolFrac.isChecked() and self._volfrac1: block, but are referenced later when computing contrast_for_surface. If VolFrac mode is selected but _volfrac1 is falsy (e.g. 0.0) this will raise UnboundLocalError. Initialize contrast_out/contrast_out_error to a safe default (e.g. "ERROR") before the conditional, or restructure the logic so the Porod block never references potentially-unset locals.

Copilot · 2026-03-02T11:34:32Z

src/sas/qtgui/Perspectives/Invariant/InvariantPerspective.py

+            if low_success:
+                qmin_ext = float(self.extrapolation_parameters.ex_q_min)
+                if self._low_points is None:
+                    self.set_low_q_extrapolation_upper_limit(float(self.txtGuinierEnd_ex.text()))
+                extrapolated_data = self._calculator.get_extra_data_low(self._low_points, q_start=qmin_ext)
+                power_low = self._calculator.get_extrapolation_power(range="low")
+                title = f"Low-Q extrapolation [{self._data.name}]"
+                self.low_extrapolation_plot = self._manager.createGuiData(extrapolated_data)
+                # set attributes on the plot object in worker thread (non-GUI data only)
+                self.low_extrapolation_plot.name = title
+                self.low_extrapolation_plot.title = title
+                self.low_extrapolation_plot.symbol = "Line"
+                self.low_extrapolation_plot.has_errors = False
+                # copy labels/units (data-only)
+                self.low_extrapolation_plot._xaxis = temp_data._xaxis
+                self.low_extrapolation_plot._xunit = temp_data._xunit
+                self.low_extrapolation_plot._yaxis = temp_data._yaxis
+                self.low_extrapolation_plot._yunit = temp_data._yunit
+                if self._low_fit:
+                    _safe_update_model(WIDGETS.W_LOWQ_POWER_VALUE_EX, power_low)
+
+            if high_success:
+                qmax_plot = float(self.extrapolation_parameters.point_3)
+                power_high = self._calculator.get_extrapolation_power(range="high")
+                high_out_data = self._calculator.get_extra_data_high(q_end=qmax_plot, npts=500)
+                title = f"High-Q extrapolation [{self._data.name}]"
+                self.high_extrapolation_plot = self._manager.createGuiData(high_out_data)
+                # set attributes on the plot object in worker thread (non-GUI data only)


Plot creation is still happening in the worker thread via self._manager.createGuiData(...), which returns/creates Qt objects (QStandardItem). Even if attribute assignments are “data-only”, constructing Qt objects off the GUI thread is unsafe. Schedule plot creation and any interaction with QStandardItems on the GUI thread (e.g., create Data1D in the worker thread, then reactor.callFromThread to wrap it with createGuiData).

src/sas/qtgui/Perspectives/Invariant/InvariantPerspective.py

src/sas/qtgui/Perspectives/Invariant/UnitTesting/CalculationTest.py

Copilot · 2026-03-02T11:34:33Z

src/sas/qtgui/Perspectives/Invariant/InvariantPerspective.py

        try:
-            qstar_data, qstar_data_err = self._calculator.get_qstar_with_error()
-        except Exception as ex:
-            calculation_failed = True
-            msg += f"Invariant calculation failed: {str(ex)}"
-            qstar_data, qstar_data_err = "ERROR", "ERROR"
-
-        reactor.callFromThread(self.update_model_from_thread, WIDGETS.D_DATA_QSTAR, qstar_data)
-        reactor.callFromThread(self.update_model_from_thread, WIDGETS.D_DATA_QSTAR_ERR, qstar_data_err)
+            self.update_from_model()



calculate_thread() runs in a worker thread, but it calls update_from_model() and the compute_low/compute_high() helpers read from Qt objects (self.model, QLineEdit.text()). Accessing Qt widgets/models from a non-GUI thread is not thread-safe and can cause sporadic crashes. Capture all needed values on the GUI thread before deferToThread (e.g., read model/widget values into plain attributes or a dataclass) and make the worker thread operate only on non-Qt data.

krzywon · 2026-03-02T15:14:32Z

@jellybean2004 - will this conflict with #3867 at all? You might want to rebase off that branch and point your PR to there for now, just to be sure.

add to init

jellybean2004 · 2026-03-06T09:07:41Z

@DrPaulSharp @krzywon, I have updated the QRangeSlider Test for Invariant and moved the testfile out of quarantine.

jellybean2004 requested a review from Copilot March 2, 2026 11:29

Copilot started reviewing on behalf of jellybean2004 March 2, 2026 11:30 View session

Copilot AI reviewed Mar 2, 2026

View reviewed changes

jellybean2004 force-pushed the inv_ui_tests branch 2 times, most recently from e83761c to 6d15e56 Compare March 2, 2026 15:51

jellybean2004 changed the base branch from main to gui-tests-activation March 3, 2026 08:40

DrPaulSharp mentioned this pull request Mar 4, 2026

[MAINT] Reactivate GUI tests from the CI #3867

Merged

8 tasks

Base automatically changed from gui-tests-activation to main March 5, 2026 14:42

jellybean2004 force-pushed the inv_ui_tests branch from 6d15e56 to 5293bac Compare March 5, 2026 15:15

jellybean2004 added 9 commits March 6, 2026 08:50

Initialization tests

f410739

add to init

Tests loaded data

7fc9ca7

fixes data loading

96d61fa

tests with real data

c6b0bc9

Initialisation tests addition and correction

9b58601

Adds tests related to the calculation button

5ca91b5

Tests for invariant calculations and plots

1fbb7bc

Removes old tests

8f5668b

Format

95f8b95

jellybean2004 force-pushed the inv_ui_tests branch from 5293bac to 04cabcd Compare March 6, 2026 08:51

jellybean2004 added 3 commits March 6, 2026 08:56

Adds a tests for a few missing methods

fd42bba

Moves old InvariantDetailsTest to another folder

8f3dda1

Copilot review suggstions

ebb06ab

jellybean2004 force-pushed the inv_ui_tests branch from 04cabcd to ebb06ab Compare March 6, 2026 08:56

Updates QRangeSlider tests for Invariant

085791d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improves and adds tests for Invariant Perspective UI#3888

Improves and adds tests for Invariant Perspective UI#3888
jellybean2004 wants to merge 13 commits intomainfrom
inv_ui_tests

jellybean2004 commented Mar 2, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 2, 2026

Uh oh!

Copilot AI Mar 2, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 2, 2026

Uh oh!

krzywon commented Mar 2, 2026

Uh oh!

jellybean2004 commented Mar 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

jellybean2004 commented Mar 2, 2026

Description

New test files

Changes to InvariantPerspective.py

Removed

How Has This Been Tested?

Review Checklist:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

krzywon commented Mar 2, 2026

Uh oh!

jellybean2004 commented Mar 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Changes to `InvariantPerspective.py`