Merge branch 'master' into perf_resize_text

docs/source/index.rst

@@ -56,3 +56,4 @@ Related Projects
     widgets.rst
     plugins.rst
     utils.rst
+    unit_tests.rst

docs/source/unit_tests.rst

@@ -0,0 +1,48 @@
+################
+Unit Test Quirks
+################
+
+Typhos, from time to time, has had issues with its unit tests.
+These often manifest as test failures and segmentation faults that only occur
+when running the tests on a cloud platform.
+
+By and large, these are related to difficulty with cleaning up resources from
+tests that allocate qt widgets.
+
+
+Things to Know for Test Writers
+-------------------------------
+
+- Always use the ``qtbot`` fixture (from the ``pytest-qt`` package)
+- Always call ``qtbot.add_widget(widget)`` on any widget you create in your test.
+  This helps clean up your widget after the test is complete.
+- Use the ``qapp`` fixture and call ``qapp.processEvents()`` if you need "something"
+  in the qt world to happen.
+- Use the ``noapp`` fixture if you need to test code that calls ``qapp.exec_()`` or
+  ``qapp.exit()``. Calling this code with no fixture will break the test suite for
+  all future tests than need the ``qapp``.
+- If your test is segfaulting, try using the ``@pytest.mark.no_gc`` decorator
+  to skip the manual garbage collection step from the ``pytest_runtest_call`` hook
+  in ``conftest.py``. In some cases (e.g. the positioner widgets) this is an ill-timed
+  redundant call.
+- If an external package's widgets (and none of ours) are showing up in the
+  widget cleanup check (also in the ``pytest_runtest_call`` hook), try using
+  the ``@pytest.mark.no_cleanup_check`` decorator. If these come from ``typhos``
+  it's fairly important to fix the issue, but if they come from an external
+  package it's hard to do something about it.
+
+
+Local vs Cloud
+--------------
+
+There are a few major differences between local and cloud builds, even
+on the same architecture:
+
+- Cloud builds set the environment variable for offscreen rendering (no rendering).
+  This slightly changes the timing and drastically changes the implementation of
+  the qt drawing primitives. You can set this yourself locally via
+  ``export QT_QPA_PLUGIN=offscreen``.
+- Cloud builds use the latest versions of packages, which may differ from the ones
+  you have installed locally.
+- Ideally, the test suite should pass both on local hardware with the default
+  qpa plugin and also on the cloud.

docs/source/upcoming_release_notes/607-tst_fix_ci_saga.rst

@@ -0,0 +1,22 @@
+607 tst_fix_ci_saga
+###################
+
+API Breaks
+----------
+- N/A
+
+Features
+--------
+- N/A
+
+Bugfixes
+--------
+- N/A
+
+Maintenance
+-----------
+- Fix issues with cloud-only CI failures and segfaults.
+
+Contributors
+------------
+- zllentz

pytest.ini

@@ -1,3 +1,6 @@
 [pytest]
-filterwarnings= default
-                once::DeprecationWarning
+filterwarnings = default
+                 once::DeprecationWarning
+addopts = --benchmark-skip
+markers = no_gc: mark a test as unstable with the per-test gc
+          no_cleanup_check: mark a test to skip the widget cleanup assert

typhos/tests/conftest.py

@@ -117,7 +117,7 @@ def pytest_runtest_call(item: pytest.Item):
     starting_widgets = get_top_level_widgets()
     if starting_widgets:
         num_start = len(_dereference_list(starting_widgets))
-        logger.debug(f"\n\nPre test - {num_start} widgets exist:")
+        logger.debug(f"\n\nPre test - {num_start} widgets exist before {item.name}:")
         _dump_widgets(starting_widgets)
 
     yield
@@ -129,8 +129,15 @@ def pytest_runtest_call(item: pytest.Item):
 
     t0 = time.monotonic()
     while time.monotonic() - t0 < 1.0 and len(_dereference_list(ending_widgets)) > 0:
-        gc.collect()
+        if not list(item.iter_markers(name="no_gc")):
+            # Some tests segfault on gc.collect when QT_QPA_PLATFORM=offscreen (!?)
+            # Mark these with pytest.mark.no_gc to skip this call
+            # Note that some other tests definitely need this gc.collect() call!
+            # And some tests function just fine no matter whether we call this or not!
+            gc.collect()
         app.processEvents()
+        # Throttle while loop to avoid weird gc things
+        time.sleep(0.1)
 
     widgets_to_check = list(
         weakref.ref(w) for w in
@@ -198,7 +205,11 @@ def pytest_runtest_call(item: pytest.Item):
             logger.error(failure_text)
 
     try:
-        assert not final_widgets, failure_text
+        if not list(item.iter_markers(name="no_cleanup_check")):
+            # Pyqtgraph stopped cleaning up properly and I can't do anything about it
+            # Rather than disable this check entirely, allow specific tests to skip it
+            # Use @pytest.mark.no_cleanup_check to skip
+            assert not final_widgets, failure_text
     finally:
         for widget in final_widgets:
             try:
@@ -325,6 +336,7 @@ class RandomSignal(SynPeriodicSignal):
     def __init__(self, *args, **kwargs):
         super().__init__(func=lambda: np.random.uniform(0, 100),
                          period=10, period_jitter=4, **kwargs)
+        self.start_simulation()
 
 
 class MockDevice(Device):

typhos/tests/test_benchmark.py

@@ -56,6 +56,10 @@ def inner_benchmark(unit_test_name, qtbot, request):
 
 def test_profiler(capsys):
     """Super basic test that hits most functions here"""
+    if sys.version_info >= (3, 12):
+        pytest.xfail(
+            reason="Known issue: profiler doesn't quite work properly on Python 3.12",
+        )
     with profiler_context(['typhos.benchmark.utils']):
         utils.get_native_functions(utils)
     output = capsys.readouterr()

typhos/tests/test_positioner.py

@@ -1,3 +1,13 @@
+"""
+Module for testing the positioner widgets
+
+Note the heavy usage of @pytest.mark.no_gc here:
+On Python 3.10+ these tests can cause segmentation faults if we manually
+(redundantly) call upon the garbage collector in conftest.
+
+Any test that uses a positioner widget needs to have this mark.
+This is not yet fully understood.
+"""
 from unittest.mock import Mock
 
 import pytest
@@ -51,6 +61,7 @@ def motor_widget(qtbot):
         widget._status_thread.wait()
 
 
+@pytest.mark.no_gc
 def test_positioner_widget_no_limits(qtbot, motor):
     setwidget = TyphosPositionerWidget.from_device(motor)
     qtbot.addWidget(setwidget)
@@ -59,6 +70,7 @@ def test_positioner_widget_no_limits(qtbot, motor):
         assert getattr(setwidget.ui, widget).isHidden()
 
 
+@pytest.mark.no_gc
 def test_positioner_widget_fixed_limits(qtbot, motor):
     motor.limits = (-10, 10)
     widget = TyphosPositionerWidget.from_device(motor)
@@ -69,6 +81,7 @@ def test_positioner_widget_fixed_limits(qtbot, motor):
 
 @show_widget
 @pytest.mark.skip()
+@pytest.mark.no_gc
 def test_positioner_widget_with_signal_limits(motor_widget):
     motor, widget = motor_widget
     # Check limit switches
@@ -80,15 +93,17 @@ def test_positioner_widget_with_signal_limits(motor_widget):
     return widget
 
 
+@pytest.mark.no_gc
 def test_positioner_widget_readback(motor_widget):
     motor, widget = motor_widget
     assert motor.readback.name in widget.ui.user_readback.channel
 
 
+@pytest.mark.no_gc
 def test_positioner_widget_stop(motor_widget):
     motor, widget = motor_widget
     widget.stop()
-    assert motor.stop.called_with(success=True)
+    motor.stop.assert_called_with(success=True)
 
 
 class NoMoveSoftPos(SoftPositioner, Device):
@@ -103,6 +118,7 @@ def _setup_move(self, *args, **kwargs):
         ...
 
 
+@pytest.mark.no_gc
 def test_positioner_widget_stop_no_error(motor_widget):
     _, widget = motor_widget
     motor = NoMoveSoftPos(name='motor')
@@ -120,6 +136,7 @@ def test_positioner_widget_stop_no_error(motor_widget):
     status.wait(timeout=1)
 
 
+@pytest.mark.no_gc
 def test_positioner_widget_set(motor_widget):
     motor, widget = motor_widget
     # Check motion
@@ -128,6 +145,7 @@ def test_positioner_widget_set(motor_widget):
     assert motor.position == 4
 
 
+@pytest.mark.no_gc
 def test_positioner_widget_positive_tweak(motor_widget):
     motor, widget = motor_widget
     widget.ui.tweak_value.setText('1')
@@ -136,6 +154,7 @@ def test_positioner_widget_positive_tweak(motor_widget):
     assert motor.position == 1
 
 
+@pytest.mark.no_gc
 def test_positioner_widget_negative_tweak(motor_widget):
     motor, widget = motor_widget
     widget.ui.tweak_value.setText('1')
@@ -144,6 +163,7 @@ def test_positioner_widget_negative_tweak(motor_widget):
     assert motor.position == -1
 
 
+@pytest.mark.no_gc
 def test_positioner_widget_moving_property(motor_widget, qtbot):
     motor, widget = motor_widget
     assert not widget.moving
@@ -154,6 +174,7 @@ def test_positioner_widget_moving_property(motor_widget, qtbot):
     qtbot.waitUntil(lambda: not widget.moving, timeout=1000)
 
 
+@pytest.mark.no_gc
 def test_positioner_widget_last_move(motor_widget):
     motor, widget = motor_widget
     assert not widget.successful_move
@@ -166,6 +187,7 @@ def test_positioner_widget_last_move(motor_widget):
     assert widget.failed_move
 
 
+@pytest.mark.no_gc
 def test_positioner_widget_moving_text_changes(motor_widget, qtbot):
     motor, widget = motor_widget
 
@@ -184,6 +206,7 @@ def get_moving_text():
     assert end_text == start_text
 
 
+@pytest.mark.no_gc
 def test_positioner_widget_alarm_text_changes(motor_widget, qtbot):
     motor, widget = motor_widget
     alarm_texts = []
@@ -216,6 +239,7 @@ def check_alarm_text_at_level(level):
         assert alarm_texts.count(text) == 1
 
 
+@pytest.mark.no_gc
 def test_positioner_widget_alarm_kind_level(motor_widget, qtbot):
     motor, widget = motor_widget
     # Alarm widget has its own tests
@@ -227,12 +251,14 @@ def test_positioner_widget_alarm_kind_level(motor_widget, qtbot):
         assert widget.ui.alarm_circle.kindLevel == kind_level
 
 
+@pytest.mark.no_gc
 def test_positioner_widget_clear_error(motor_widget, qtbot):
     motor, widget = motor_widget
     widget.clear_error()
     qtbot.waitUntil(lambda: motor.clear_error.called, timeout=500)
 
 
+@pytest.mark.no_gc
 def test_positioner_widget_move_error(motor_widget, qtbot):
     motor, widget = motor_widget
     bad_position = motor.high_limit.get() + 1

typhos/tests/test_widgets.py

@@ -67,6 +67,7 @@ def test_signal_dialog_button_repeated_show(qtbot, widget_button):
 
 
 @pydm_version_xfail
+@pytest.mark.no_cleanup_check
 @pytest.mark.parametrize('button_type', [WaveformDialogButton,
                                          ImageDialogButton],
                          ids=['Waveform', 'Image'])