From 327a82b2544be9e45c0a5350c01bdd1117c7de8b Mon Sep 17 00:00:00 2001 From: John Haddon Date: Fri, 7 Aug 2026 13:08:01 +0100 Subject: [PATCH 1/2] UITest.TestCase : Manage UIThreadCallHandler in `setUp()/tearDown()` This has a few benefits : - We can simplify a few tests which were making their own handlers. - It is more robust to badly behaved tests that don't handle UI thread calls. By catching them in our own handler, we prevent them spilling into the next test, where a call to `waitForIdle()` might trigger them unexpectedly. - In future, we can add an assertion that all tests are correctly handling calls. - It enables a race-free replacement for `PlugValueWidgetTest.waitForUpdate()`, to appear in the next commit. --- Changes.md | 5 + python/GafferUITest/AnnotationsGadgetTest.py | 270 +++++++++---------- python/GafferUITest/ContextTrackerTest.py | 9 +- python/GafferUITest/GraphGadgetTest.py | 5 +- python/GafferUITest/TestCase.py | 7 + 5 files changed, 146 insertions(+), 150 deletions(-) diff --git a/Changes.md b/Changes.md index 38b80aad9a..17ec2a1ef1 100644 --- a/Changes.md +++ b/Changes.md @@ -25,6 +25,11 @@ Fixes - Fixed handling of connections between floats and color/vector components [^2]. - Fixed bug preventing attributes from being deleted from lights during an interactive render [^2]. +API +--- + +- GafferUITest.TestCase : Added `uiThreadCallHandler` member. This should be used by any tests which need to handle calls to the UI thread. + Build ----- diff --git a/python/GafferUITest/AnnotationsGadgetTest.py b/python/GafferUITest/AnnotationsGadgetTest.py index c80b28aefc..25f40ff5e0 100644 --- a/python/GafferUITest/AnnotationsGadgetTest.py +++ b/python/GafferUITest/AnnotationsGadgetTest.py @@ -146,42 +146,40 @@ def error( *unused ) : errorCondition.notify() script["errorNode"].errorSignal().connect( error ) - with GafferTest.ParallelAlgoTest.UIThreadCallHandler() as callHandler : - - graphGadget = GafferUI.GraphGadget( script ) - gadget = graphGadget.annotationsGadget() + graphGadget = GafferUI.GraphGadget( script ) + gadget = graphGadget.annotationsGadget() - # Value must be computed in background, so initially we expect a placeholder - self.assertEqual( gadget.annotationText( script["node"] ), "test : ---" ) + # Value must be computed in background, so initially we expect a placeholder + self.assertEqual( gadget.annotationText( script["node"] ), "test : ---" ) - # But if we wait for the background update we should get some updated text. - callHandler.assertCalled() - self.assertEqual( gadget.annotationText( script["node"] ), "test : 0" ) + # But if we wait for the background update we should get some updated text. + self.uiThreadCallHandler.assertCalled() + self.assertEqual( gadget.annotationText( script["node"] ), "test : 0" ) - # Same applies when the plug is dirtied. We expect a placeholder first. - script["node"]["op1"].setValue( 1 ) - self.assertEqual( gadget.annotationText( script["node"] ), "test : ---" ) + # Same applies when the plug is dirtied. We expect a placeholder first. + script["node"]["op1"].setValue( 1 ) + self.assertEqual( gadget.annotationText( script["node"] ), "test : ---" ) - # Then we get the real value when the computation is done. - callHandler.assertCalled() - self.assertEqual( gadget.annotationText( script["node"] ), "test : 1" ) + # Then we get the real value when the computation is done. + self.uiThreadCallHandler.assertCalled() + self.assertEqual( gadget.annotationText( script["node"] ), "test : 1" ) - # And when the plug is dirtied by an upstream change we again expect - # placeholder text at first. - script["node"]["op1"].setInput( script["errorNode"]["out3"] ) - self.assertEqual( gadget.annotationText( script["node"] ), "test : ---" ) + # And when the plug is dirtied by an upstream change we again expect + # placeholder text at first. + script["node"]["op1"].setInput( script["errorNode"]["out3"] ) + self.assertEqual( gadget.annotationText( script["node"] ), "test : ---" ) - # But this time we don't expect to get updated text, because the computation - # will error. - with errorCondition : - errorCondition.wait() + # But this time we don't expect to get updated text, because the computation + # will error. + with errorCondition : + errorCondition.wait() - # Handle UI thread calls made by StandardNodeGadget to show errors, - # and assert that there are no more calls. - callHandler.assertCalled() - callHandler.assertCalled() + # Handle UI thread calls made by StandardNodeGadget to show errors, + # and assert that there are no more calls. + self.uiThreadCallHandler.assertCalled() + self.uiThreadCallHandler.assertCalled() - callHandler.assertDone() + self.uiThreadCallHandler.assertDone() self.assertEqual( gadget.annotationText( script["node"] ), "test : ---" ) @@ -222,56 +220,54 @@ def waitAndClear( event, timeout = None ) : Gaffer.MetadataAlgo.addAnnotation( script["node"], "user", Gaffer.MetadataAlgo.Annotation( "{sum}" ) ) - with GafferTest.ParallelAlgoTest.UIThreadCallHandler() as callHandler : - - graphGadget = GafferUI.GraphGadget( script ) - viewportGadget = GafferUI.ViewportGadget( graphGadget ) - gadget = graphGadget.annotationsGadget() + graphGadget = GafferUI.GraphGadget( script ) + viewportGadget = GafferUI.ViewportGadget( graphGadget ) + gadget = graphGadget.annotationsGadget() - # Value must be computed in background, so initially we expect a placeholder - self.assertEqual( gadget.annotationText( script["node"] ), "---" ) + # Value must be computed in background, so initially we expect a placeholder + self.assertEqual( gadget.annotationText( script["node"] ), "---" ) - # Wait for compute to start, and make a graph edit to cancel it. - waitAndClear( AnnotationsGadgetTest.expressionStartedEvent ) - script["node"]["op2"].setValue( 2 ) + # Wait for compute to start, and make a graph edit to cancel it. + waitAndClear( AnnotationsGadgetTest.expressionStartedEvent ) + script["node"]["op2"].setValue( 2 ) - # We expect a call on the UI thread to re-dirty the annotation. + # We expect a call on the UI thread to re-dirty the annotation. - renderRequests = GafferTest.CapturingSlot( viewportGadget.renderRequestSignal() ) - callHandler.assertCalled() - self.assertEqual( len( renderRequests ), 1 ) - self.assertEqual( gadget.annotationText( script["node"] ), "---" ) + renderRequests = GafferTest.CapturingSlot( viewportGadget.renderRequestSignal() ) + self.uiThreadCallHandler.assertCalled() + self.assertEqual( len( renderRequests ), 1 ) + self.assertEqual( gadget.annotationText( script["node"] ), "---" ) - # A new background task should have been launched to compute the - # text again. If we let the expression run to completion then we - # should get the final text. - waitAndClear( AnnotationsGadgetTest.expressionStartedEvent ) - GafferUITest.AnnotationsGadgetTest.expressionContinueEvent.set() + # A new background task should have been launched to compute the + # text again. If we let the expression run to completion then we + # should get the final text. + waitAndClear( AnnotationsGadgetTest.expressionStartedEvent ) + GafferUITest.AnnotationsGadgetTest.expressionContinueEvent.set() - callHandler.assertCalled() - self.assertEqual( gadget.annotationText( script["node"] ), "4" ) + self.uiThreadCallHandler.assertCalled() + self.assertEqual( gadget.annotationText( script["node"] ), "4" ) - # Try one more time. But this time do the cancellation by - # modifying a completely unrelated plug. + # Try one more time. But this time do the cancellation by + # modifying a completely unrelated plug. - script["node"]["op2"].setValue( 3 ) - self.assertEqual( gadget.annotationText( script["node"] ), "---" ) - waitAndClear( AnnotationsGadgetTest.expressionStartedEvent ) + script["node"]["op2"].setValue( 3 ) + self.assertEqual( gadget.annotationText( script["node"] ), "---" ) + waitAndClear( AnnotationsGadgetTest.expressionStartedEvent ) - script["node2"]["op1"].setValue( 1 ) # Cancels + script["node2"]["op1"].setValue( 1 ) # Cancels - renderRequests = GafferTest.CapturingSlot( viewportGadget.renderRequestSignal() ) - callHandler.assertCalled() - self.assertEqual( len( renderRequests ), 1 ) - self.assertEqual( gadget.annotationText( script["node"] ), "---" ) + renderRequests = GafferTest.CapturingSlot( viewportGadget.renderRequestSignal() ) + self.uiThreadCallHandler.assertCalled() + self.assertEqual( len( renderRequests ), 1 ) + self.assertEqual( gadget.annotationText( script["node"] ), "---" ) - waitAndClear( AnnotationsGadgetTest.expressionStartedEvent ) - GafferUITest.AnnotationsGadgetTest.expressionContinueEvent.set() + waitAndClear( AnnotationsGadgetTest.expressionStartedEvent ) + GafferUITest.AnnotationsGadgetTest.expressionContinueEvent.set() - callHandler.assertCalled() - self.assertEqual( gadget.annotationText( script["node"] ), "6" ) + self.uiThreadCallHandler.assertCalled() + self.assertEqual( gadget.annotationText( script["node"] ), "6" ) - callHandler.assertDone() + self.uiThreadCallHandler.assertDone() def testContextSensitiveText( self ) : @@ -279,27 +275,25 @@ def testContextSensitiveText( self ) : script["node"] = GafferTest.FrameNode() Gaffer.MetadataAlgo.addAnnotation( script["node"], "user", Gaffer.MetadataAlgo.Annotation( "{output}" ) ) - with GafferTest.ParallelAlgoTest.UIThreadCallHandler() as callHandler : - - graphGadget = GafferUI.GraphGadget( script ) - gadget = graphGadget.annotationsGadget() + graphGadget = GafferUI.GraphGadget( script ) + gadget = graphGadget.annotationsGadget() - # Value must be computed in background, so initially we expect a placeholder - self.assertEqual( gadget.annotationText( script["node"] ), "---" ) + # Value must be computed in background, so initially we expect a placeholder + self.assertEqual( gadget.annotationText( script["node"] ), "---" ) - # But if we wait for the background update we should get some updated text. - callHandler.assertCalled() - self.assertEqual( gadget.annotationText( script["node"] ), "1" ) + # But if we wait for the background update we should get some updated text. + self.uiThreadCallHandler.assertCalled() + self.assertEqual( gadget.annotationText( script["node"] ), "1" ) - # Same applies when the context is changed. We expect a placeholder first. - script.context().setFrame( 10 ) - self.assertEqual( gadget.annotationText( script["node"] ), "---" ) + # Same applies when the context is changed. We expect a placeholder first. + script.context().setFrame( 10 ) + self.assertEqual( gadget.annotationText( script["node"] ), "---" ) - # Then we get the real value when the computation is done. - callHandler.assertCalled() - self.assertEqual( gadget.annotationText( script["node"] ), "10" ) + # Then we get the real value when the computation is done. + self.uiThreadCallHandler.assertCalled() + self.assertEqual( gadget.annotationText( script["node"] ), "10" ) - callHandler.assertDone() + self.uiThreadCallHandler.assertDone() def testContextTracking( self ) : @@ -313,31 +307,29 @@ def testContextTracking( self ) : script.setFocus( script["timeWarp"] ) - with GafferTest.ParallelAlgoTest.UIThreadCallHandler() as callHandler : - - graphGadget = GafferUI.GraphGadget( script ) - gadget = graphGadget.annotationsGadget() + graphGadget = GafferUI.GraphGadget( script ) + gadget = graphGadget.annotationsGadget() - # Value must be computed in background, so initially we expect a placeholder - self.assertEqual( gadget.annotationText( script["node"] ), "---" ) + # Value must be computed in background, so initially we expect a placeholder + self.assertEqual( gadget.annotationText( script["node"] ), "---" ) - # But if we wait for the background update we should get some updated text. - callHandler.assertCalled() - self.assertEqual( gadget.annotationText( script["node"] ), "1" ) + # But if we wait for the background update we should get some updated text. + self.uiThreadCallHandler.assertCalled() + self.assertEqual( gadget.annotationText( script["node"] ), "1" ) - # Same applies when the TimeWarp is changed. First we wait for the - # ContextTracker to update, and then we get the placeholder text - # when the annotation update starts. - script["timeWarp"]["offset"].setValue( 10 ) - self.waitForIdle() - callHandler.assertCalled() - self.assertEqual( gadget.annotationText( script["node"] ), "---" ) + # Same applies when the TimeWarp is changed. First we wait for the + # ContextTracker to update, and then we get the placeholder text + # when the annotation update starts. + script["timeWarp"]["offset"].setValue( 10 ) + self.waitForIdle() + self.uiThreadCallHandler.assertCalled() + self.assertEqual( gadget.annotationText( script["node"] ), "---" ) - # Then we get the real value when the computation is done. - callHandler.assertCalled() - self.assertEqual( gadget.annotationText( script["node"] ), "11" ) + # Then we get the real value when the computation is done. + self.uiThreadCallHandler.assertCalled() + self.assertEqual( gadget.annotationText( script["node"] ), "11" ) - callHandler.assertDone() + self.uiThreadCallHandler.assertDone() def testSubstitutedTextRenderRequests( self ) : @@ -384,24 +376,22 @@ def testDestroyGadgetWhileBackgroundThreadRuns( self ) : Gaffer.MetadataAlgo.addAnnotation( script["node"], "user", Gaffer.MetadataAlgo.Annotation( "{sum}" ) ) - with GafferTest.ParallelAlgoTest.UIThreadCallHandler() as callHandler : - - graphGadget = GafferUI.GraphGadget( script ) - gadget = graphGadget.annotationsGadget() + graphGadget = GafferUI.GraphGadget( script ) + gadget = graphGadget.annotationsGadget() - # Value must be computed in background, so initially we expect a placeholder. - self.assertEqual( gadget.annotationText( script["node"], "user" ), "---" ) + # Value must be computed in background, so initially we expect a placeholder. + self.assertEqual( gadget.annotationText( script["node"], "user" ), "---" ) - # Wait for the UI thread call that will be scheduled to update the gadget. - call = callHandler.receive() - # But then delete our references to the gadget _before_ we execute - # the call. This simulates a user removing a GraphEditor while the - # AnnotationsGadget is still updating. If we don't handle lifetimes - # well, then this could crash. - del graphGadget, gadget - call() + # Wait for the UI thread call that will be scheduled to update the gadget. + call = self.uiThreadCallHandler.receive() + # But then delete our references to the gadget _before_ we execute + # the call. This simulates a user removing a GraphEditor while the + # AnnotationsGadget is still updating. If we don't handle lifetimes + # well, then this could crash. + del graphGadget, gadget + call() - callHandler.assertDone() + self.uiThreadCallHandler.assertDone() def testRemoveNodeWhileBackgroundThreadRuns( self ) : @@ -410,24 +400,22 @@ def testRemoveNodeWhileBackgroundThreadRuns( self ) : Gaffer.MetadataAlgo.addAnnotation( script["node"], "test", Gaffer.MetadataAlgo.Annotation( "{sum}" ) ) - with GafferTest.ParallelAlgoTest.UIThreadCallHandler() as callHandler : - - graphGadget = GafferUI.GraphGadget( script ) - gadget = graphGadget.annotationsGadget() + graphGadget = GafferUI.GraphGadget( script ) + gadget = graphGadget.annotationsGadget() - # Value must be computed in background, so initially we expect a placeholder - self.assertEqual( gadget.annotationText( script["node"], "test" ), "---" ) + # Value must be computed in background, so initially we expect a placeholder + self.assertEqual( gadget.annotationText( script["node"], "test" ), "---" ) - # Wait for the UI thread call that will be scheduled to update the gadget. - call = callHandler.receive() - # But then delete the node _before_ we execute the call. This - # simulates a user deleting a node while the AnnotationsGadget is - # still updating. If we don't handle lifetimes well, then this could - # crash. - del script["node"] - call() + # Wait for the UI thread call that will be scheduled to update the gadget. + call = self.uiThreadCallHandler.receive() + # But then delete the node _before_ we execute the call. This + # simulates a user deleting a node while the AnnotationsGadget is + # still updating. If we don't handle lifetimes well, then this could + # crash. + del script["node"] + call() - callHandler.assertDone() + self.uiThreadCallHandler.assertDone() def testRemoveAnnotationWhileBackgroundThreadRuns( self ) : @@ -438,20 +426,18 @@ def testRemoveAnnotationWhileBackgroundThreadRuns( self ) : for i in range( 0, numAnnotations ) : Gaffer.MetadataAlgo.addAnnotation( script["node"], f"test{i}", Gaffer.MetadataAlgo.Annotation( "{sum}" ) ) - with GafferTest.ParallelAlgoTest.UIThreadCallHandler() as callHandler : - - graphGadget = GafferUI.GraphGadget( script ) - gadget = graphGadget.annotationsGadget() + graphGadget = GafferUI.GraphGadget( script ) + gadget = graphGadget.annotationsGadget() - # Value must be computed in background, so initially we expect a placeholder - self.assertEqual( gadget.annotationText( script["node"], "test0" ), "---" ) + # Value must be computed in background, so initially we expect a placeholder + self.assertEqual( gadget.annotationText( script["node"], "test0" ), "---" ) - # Remove annotations while the background task runs. - for i in range( 0, numAnnotations ) : - Gaffer.MetadataAlgo.removeAnnotation( script["node"], f"test{i}" ) + # Remove annotations while the background task runs. + for i in range( 0, numAnnotations ) : + Gaffer.MetadataAlgo.removeAnnotation( script["node"], f"test{i}" ) - # And wait for the task to complete. - callHandler.assertCalled() + # And wait for the task to complete. + self.uiThreadCallHandler.assertCalled() self.assertEqual( gadget.annotationText( script["node"], "test0" ), "" ) diff --git a/python/GafferUITest/ContextTrackerTest.py b/python/GafferUITest/ContextTrackerTest.py index 1a483efd24..11a5e6e9b2 100644 --- a/python/GafferUITest/ContextTrackerTest.py +++ b/python/GafferUITest/ContextTrackerTest.py @@ -818,11 +818,10 @@ def testCancellation( self ) : # the background task. with ContextTrackerTest.expressionStartedCondition : - with GafferTest.ParallelAlgoTest.UIThreadCallHandler() as handler : - script["node"]["op1"].setValue( 1 ) - handler.assertCalled() # Handle UI thread call made when background task detects cancellation. - self.waitForIdle() # Handle idle event used to restart update. - ContextTrackerTest.expressionStartedCondition.wait() + script["node"]["op1"].setValue( 1 ) + self.uiThreadCallHandler.assertCalled() # Handle UI thread call made when background task detects cancellation. + self.waitForIdle() # Handle idle event used to restart update. + ContextTrackerTest.expressionStartedCondition.wait() # Again, the update won't have completed because the expression is stuck. self.assertFalse( tracker.isTracked( script["node"] ) ) diff --git a/python/GafferUITest/GraphGadgetTest.py b/python/GafferUITest/GraphGadgetTest.py index 174d94e6a7..9a96823b83 100644 --- a/python/GafferUITest/GraphGadgetTest.py +++ b/python/GafferUITest/GraphGadgetTest.py @@ -1414,9 +1414,8 @@ def assertHighlighting( self, graphGadget, expectedState ) : # to wait for it to finish. contextTracker = GafferUI.ContextTracker.acquireForFocus( graphGadget.getRoot().scriptNode() ) if contextTracker.updatePending() : - with GafferTest.ParallelAlgoTest.UIThreadCallHandler() as uiCallHandler : - self.waitForIdle() - uiCallHandler.assertCalled() + self.waitForIdle() + self.uiThreadCallHandler.assertCalled() actualState = { k : not graphGadget.nodeGadget( graphGadget.getRoot()[k] ).getContents().getDimmed() diff --git a/python/GafferUITest/TestCase.py b/python/GafferUITest/TestCase.py index 836dfe25f4..0f2db90a56 100644 --- a/python/GafferUITest/TestCase.py +++ b/python/GafferUITest/TestCase.py @@ -83,6 +83,13 @@ def messageHandler( type, context, message ) : "Viewport display is likely to show banding - please resolve graphics driver issue." ) + self.uiThreadCallHandler = GafferTest.ParallelAlgoTest.UIThreadCallHandler() + self.uiThreadCallHandler.__enter__() + ## \todo We could call `self.uiThreadCallHandler.assertDone()` here, so + # that `__exit__()` checks that the tests are calling `assertCalled()` + # appropriately. That will likely mean fixing a few tests. + self.addCleanup( self.uiThreadCallHandler.__exit__, None, None, None ) + def tearDown( self ) : GafferTest.TestCase.tearDown( self ) From 74b63c2b5e63bd94c63804e92c27c33f81597878 Mon Sep 17 00:00:00 2001 From: John Haddon Date: Fri, 7 Aug 2026 13:16:20 +0100 Subject: [PATCH 2/2] PlugValueWidgetTests : Fix handling of background updates This fixes test failures that looked like this : ``` Traceback (most recent call last): File "D:\a\gaffer\gaffer\build\python\GafferUITest\PlugValueWidgetTest.py", line 453, in testContextTrackerUpdates self.waitForUpdate( widget ) File "D:\a\gaffer\gaffer\build\python\GafferUITest\PlugValueWidgetTest.py", line 62, in waitForUpdate handler.assertCalled() File "D:\a\gaffer\gaffer\build\python\GafferTest\ParallelAlgoTest.py", line 92, in assertCalled self.receive( timeout )() ^^^^^^^^^^^^^^^^^^^^^^^ File "D:\a\gaffer\gaffer\build\python\GafferTest\ParallelAlgoTest.py", line 86, in receive raise AssertionError( "UIThread call not made within {} seconds".format( timeout ) ) AssertionError: UIThread call not made within 30.0 seconds ``` Our old approach of calling `waitForUpdate()` after running the code to trigger the update was bogus - it was possible for the update to have finished before we even got to calling `waitForUpdate()`. By using the already-scoped `self.uiThreadCallHandler` instead of making a temporary one, we avoid this race condition. --- Changes.md | 9 ++++- .../GafferUITest/BoolPlugValueWidgetTest.py | 14 +++---- python/GafferUITest/ColorChooserTest.py | 8 ++-- .../NumericPlugValueWidgetTest.py | 18 ++++----- python/GafferUITest/PlugValueWidgetTest.py | 37 ++++++------------- .../GafferUITest/StandardNodeToolbarTest.py | 2 +- .../GafferUITest/StringPlugValueWidgetTest.py | 20 +++++----- python/GafferUITest/TestCase.py | 10 +++++ 8 files changed, 61 insertions(+), 57 deletions(-) diff --git a/Changes.md b/Changes.md index 17ec2a1ef1..95cd3d45aa 100644 --- a/Changes.md +++ b/Changes.md @@ -28,7 +28,14 @@ Fixes API --- -- GafferUITest.TestCase : Added `uiThreadCallHandler` member. This should be used by any tests which need to handle calls to the UI thread. +- GafferUITest.TestCase : + - Added `uiThreadCallHandler` member. This should be used by any tests which need to handle calls to the UI thread. + - Added `waitForPlugValueWidgetUpdate()` method. + +Breaking Changes +---------------- + +- PlugValueWidgetTest : Removed `waitForUpdate()` method. Use `GafferUITest.TestCase.waitForPlugValueWidgetUpdate()` instead. Build ----- diff --git a/python/GafferUITest/BoolPlugValueWidgetTest.py b/python/GafferUITest/BoolPlugValueWidgetTest.py index ce7fe8c5ff..07cd2ec3bf 100644 --- a/python/GafferUITest/BoolPlugValueWidgetTest.py +++ b/python/GafferUITest/BoolPlugValueWidgetTest.py @@ -55,19 +55,19 @@ def test( self ) : self.assertEqual( w.boolWidget().getState(), False ) n["user"]["p1"].setValue( True ) - GafferUITest.PlugValueWidgetTest.waitForUpdate( w ) + self.waitForPlugValueWidgetUpdate( w ) self.assertEqual( w.boolWidget().getState(), True ) w.setPlugs( n["user"].children() ) - GafferUITest.PlugValueWidgetTest.waitForUpdate( w ) + self.waitForPlugValueWidgetUpdate( w ) self.assertEqual( w.boolWidget().getState(), w.boolWidget().State.Indeterminate ) n["user"]["p2"].setValue( True ) - GafferUITest.PlugValueWidgetTest.waitForUpdate( w ) + self.waitForPlugValueWidgetUpdate( w ) self.assertEqual( w.boolWidget().getState(), True ) w.setPlugs( [] ) - GafferUITest.PlugValueWidgetTest.waitForUpdate( w ) + self.waitForPlugValueWidgetUpdate( w ) self.assertEqual( w.boolWidget().getState(), w.boolWidget().State.Indeterminate ) def testInitialValue( self ) : @@ -78,7 +78,7 @@ def testInitialValue( self ) : for v in ( True, False ) : n["user"]["p"].setValue( v ) w = GafferUI.BoolPlugValueWidget( n["user"]["p"] ) - GafferUITest.PlugValueWidgetTest.waitForUpdate( w ) + self.waitForPlugValueWidgetUpdate( w ) self.assertEqual( w.boolWidget().getState(), v ) def testErrorHandling( self ) : @@ -93,9 +93,9 @@ def testErrorHandling( self ) : script["b"] = GafferTest.BadNode() script["n"]["user"]["p"].setInput( script["b"]["out3"] ) - GafferUITest.PlugValueWidgetTest.waitForUpdate( w ) + self.waitForPlugValueWidgetUpdate( w ) self.assertTrue( w.boolWidget().getErrored() ) script["n"]["user"]["p"].setInput( None ) - GafferUITest.PlugValueWidgetTest.waitForUpdate( w ) + self.waitForPlugValueWidgetUpdate( w ) self.assertFalse( w.boolWidget().getErrored() ) diff --git a/python/GafferUITest/ColorChooserTest.py b/python/GafferUITest/ColorChooserTest.py index 1650d27ddd..2cb8ec1805 100644 --- a/python/GafferUITest/ColorChooserTest.py +++ b/python/GafferUITest/ColorChooserTest.py @@ -251,8 +251,8 @@ def testSaveDefaultOptions( self ) : rgbaWidget = GafferUI.ColorPlugValueWidget( script["node"]["rgbaPlug"] ) rgbaWidget.setColorChooserVisible( True ) - GafferUITest.PlugValueWidgetTest.waitForUpdate( rgbWidget._ColorPlugValueWidget__colorChooser ) - GafferUITest.PlugValueWidgetTest.waitForUpdate( rgbaWidget._ColorPlugValueWidget__colorChooser ) + self.waitForPlugValueWidgetUpdate( rgbWidget._ColorPlugValueWidget__colorChooser ) + self.waitForPlugValueWidgetUpdate( rgbaWidget._ColorPlugValueWidget__colorChooser ) # Default state for c in "rgbhsvtmi" : @@ -286,8 +286,8 @@ def testSaveDefaultOptions( self ) : rgbaWidget = GafferUI.ColorPlugValueWidget( script["node"]["rgbaPlug"] ) rgbaWidget.setColorChooserVisible( True ) - GafferUITest.PlugValueWidgetTest.waitForUpdate( rgbWidget._ColorPlugValueWidget__colorChooser ) - GafferUITest.PlugValueWidgetTest.waitForUpdate( rgbaWidget._ColorPlugValueWidget__colorChooser ) + self.waitForPlugValueWidgetUpdate( rgbWidget._ColorPlugValueWidget__colorChooser ) + self.waitForPlugValueWidgetUpdate( rgbaWidget._ColorPlugValueWidget__colorChooser ) for c in "rgbhsv" : self.assertTrue( self.__sliderFromWidget( rgbWidget, c ).getVisible() ) diff --git a/python/GafferUITest/NumericPlugValueWidgetTest.py b/python/GafferUITest/NumericPlugValueWidgetTest.py index 8d58a42aba..d335c61472 100644 --- a/python/GafferUITest/NumericPlugValueWidgetTest.py +++ b/python/GafferUITest/NumericPlugValueWidgetTest.py @@ -51,12 +51,12 @@ def test( self ) : w = GafferUI.NumericPlugValueWidget( n["i"] ) self.assertTrue( w.getPlug().isSame( n["i"] ) ) - GafferUITest.PlugValueWidgetTest.waitForUpdate( w ) + self.waitForPlugValueWidgetUpdate( w ) self.assertTrue( isinstance( w.numericWidget().getValue(), int ) ) w.setPlug( n["f"] ) self.assertTrue( w.getPlug().isSame( n["f"] ) ) - GafferUITest.PlugValueWidgetTest.waitForUpdate( w ) + self.waitForPlugValueWidgetUpdate( w ) self.assertTrue( isinstance( w.numericWidget().getValue(), float ) ) w = GafferUI.NumericPlugValueWidget( plugs = [] ) @@ -65,7 +65,7 @@ def test( self ) : w.setPlug( n["f"] ) self.assertTrue( w.getPlug().isSame( n["f"] ) ) - GafferUITest.PlugValueWidgetTest.waitForUpdate( w ) + self.waitForPlugValueWidgetUpdate( w ) self.assertTrue( isinstance( w.numericWidget().getValue(), float ) ) self.assertEqual( w.numericWidget().getEditable(), True ) @@ -80,11 +80,11 @@ def testEditMultiplePlugs( self ) : n["user"]["i1"].setValue( 2 ) n["user"]["i2"].setValue( 2 ) - GafferUITest.PlugValueWidgetTest.waitForUpdate( w ) + self.waitForPlugValueWidgetUpdate( w ) self.assertEqual( w.numericWidget().getText(), "2" ) n["user"]["i1"].setValue( 1 ) - GafferUITest.PlugValueWidgetTest.waitForUpdate( w ) + self.waitForPlugValueWidgetUpdate( w ) self.assertEqual( w.numericWidget().getText(), "" ) self.assertEqual( w.numericWidget()._qtWidget().placeholderText(), "---" ) @@ -104,7 +104,7 @@ def testChangeToMixedPlugsDoesntOverwriteExistingPlugValues( self ) : n["user"]["i2"].setValue( 2 ) w = GafferUI.NumericPlugValueWidget( n["user"]["i1"] ) - GafferUITest.PlugValueWidgetTest.waitForUpdate( w ) + self.waitForPlugValueWidgetUpdate( w ) self.assertEqual( w.numericWidget().getText(), "1" ) self.assertEqual( w.numericWidget()._qtWidget().placeholderText(), "" ) @@ -113,7 +113,7 @@ def testChangeToMixedPlugsDoesntOverwriteExistingPlugValues( self ) : self.assertEqual( n["user"]["i1"].getValue(), 1 ) self.assertEqual( n["user"]["i2"].getValue(), 2 ) - GafferUITest.PlugValueWidgetTest.waitForUpdate( w ) + self.waitForPlugValueWidgetUpdate( w ) self.assertEqual( w.numericWidget().getText(), "" ) self.assertEqual( w.numericWidget()._qtWidget().placeholderText(), "---" ) @@ -126,7 +126,7 @@ def testMixedOrInvalidValuesPreservesExisting( self ) : n["user"]["i2"].setValue( 2 ) w = GafferUI.NumericPlugValueWidget( n["user"]["i1"] ) - GafferUITest.PlugValueWidgetTest.waitForUpdate( w ) + self.waitForPlugValueWidgetUpdate( w ) self.assertEqual( w.numericWidget().getValue(), 1 ) w.numericWidget().setText( "" ) @@ -135,7 +135,7 @@ def testMixedOrInvalidValuesPreservesExisting( self ) : self.assertEqual( n["user"]["i1"].getValue(), 1 ) w = GafferUI.NumericPlugValueWidget( n["user"].children() ) - GafferUITest.PlugValueWidgetTest.waitForUpdate( w ) + self.waitForPlugValueWidgetUpdate( w ) self.assertEqual( w.numericWidget().getText(), "" ) w.numericWidget()._qtWidget().editingFinished.emit() diff --git a/python/GafferUITest/PlugValueWidgetTest.py b/python/GafferUITest/PlugValueWidgetTest.py index 9df688d665..80b25de897 100644 --- a/python/GafferUITest/PlugValueWidgetTest.py +++ b/python/GafferUITest/PlugValueWidgetTest.py @@ -48,19 +48,6 @@ class PlugValueWidgetTest( GafferUITest.TestCase ) : - @staticmethod - def waitForUpdate( widget ) : - - with GafferTest.ParallelAlgoTest.UIThreadCallHandler() as handler : - - # Updates are done lazily, so we need to flush any pending updates. - widget._PlugValueWidget__callUpdateFromValues.flush( widget ) - - # And updates for computed values are done in the background, so we - # need to wait until they're done. - if any( isinstance( p, Gaffer.ValuePlug ) and Gaffer.PlugAlgo.dependsOnCompute( p ) for p in widget.getPlugs() ) : - handler.assertCalled() - def testContext( self ) : s = Gaffer.ScriptNode() @@ -72,7 +59,7 @@ def testContext( self ) : self.assertEqual( w.context(), s.context() ) s.context().setFrame( 10 ) - self.waitForUpdate( w ) + self.waitForPlugValueWidgetUpdate( w ) self.assertEqual( w.numericWidget().getValue(), 10 ) self.assertEqual( w.context(), s.context() ) @@ -279,19 +266,19 @@ def testUpdates( self ) : # Changing the context shouldn't trigger an update, because the # plug value isn't computed. script.context().setFrame( 2 ) - self.waitForUpdate( widget ) + self.waitForPlugValueWidgetUpdate( widget ) self.assertEqual( widget.updateCount, 1 ) # Changing the plug should trigger an update. widget.setPlug( script["add"]["op2"] ) - self.waitForUpdate( widget ) + self.waitForPlugValueWidgetUpdate( widget ) self.assertEqual( widget.updateCount, 2 ) self.assertEqual( widget.updateContexts[1], script.context() ) # Changing the context still shouldn't trigger an update, because the # plug value isn't computed. script.context().setFrame( 3 ) - self.waitForUpdate( widget ) + self.waitForPlugValueWidgetUpdate( widget ) self.assertEqual( widget.updateCount, 2 ) # Changing the plug again should trigger an update again. This time we @@ -299,7 +286,7 @@ def testUpdates( self ) : # one when it completes. This is because the plug's value is computed # and we don't want to block the UI thread with computes. widget.setPlug( script["add"]["sum"] ) - self.waitForUpdate( widget ) + self.waitForPlugValueWidgetUpdate( widget ) self.assertEqual( widget.updateCount, 4 ) self.assertEqual( widget.updateContexts[2], script.context() ) self.assertEqual( widget.updateContexts[3], script.context() ) @@ -307,7 +294,7 @@ def testUpdates( self ) : # And now changing the context should trigger an update, since computed # values may be context-sensitive. script.context().setFrame( 4 ) - self.waitForUpdate( widget ) + self.waitForPlugValueWidgetUpdate( widget ) self.assertEqual( widget.updateCount, 6 ) self.assertEqual( widget.updateContexts[4], script.context() ) self.assertEqual( widget.updateContexts[5], script.context() ) @@ -399,7 +386,7 @@ def testContextForEditorSettings( self ) : # Editor not viewing anything yet, so we just use the default # script context. - self.waitForUpdate( widget ) + self.waitForPlugValueWidgetUpdate( widget ) self.assertEqual( widget.updateCount, 2 ) # One at the start of the background update, and one on completion self.assertEqual( widget.updateContexts[1], script.context() ) @@ -407,7 +394,7 @@ def testContextForEditorSettings( self ) : # tracked for it. editor.settings()["in"].setInput( script["node"]["sum"] ) - self.waitForUpdate( widget ) + self.waitForPlugValueWidgetUpdate( widget ) self.assertEqual( widget.updateCount, 4 ) self.assertEqual( widget.updateContexts[3], contextTracker.context( script["node"] ) ) self.assertIn( "testVariable", widget.updateContexts[3] ) @@ -416,7 +403,7 @@ def testContextForEditorSettings( self ) : # has been tracked for that. editor.settings()["in"].setInput( script["contextVariables"]["out"] ) - self.waitForUpdate( widget ) + self.waitForPlugValueWidgetUpdate( widget ) self.assertEqual( widget.updateCount, 6 ) self.assertEqual( widget.updateContexts[5], contextTracker.context( script["contextVariables"] ) ) self.assertNotIn( "testVariable", widget.updateContexts[5] ) @@ -450,7 +437,7 @@ def testContextTrackerUpdates( self ) : # to indicate the start of the background update and one when # it finishes. window.setVisible( True ) - self.waitForUpdate( widget ) + self.waitForPlugValueWidgetUpdate( widget ) self.assertEqual( widget.updateCount, 2 ) self.assertEqual( widget.updateContexts[-1], script.context() ) @@ -460,7 +447,7 @@ def testContextTrackerUpdates( self ) : contextTracker = GafferUI.ContextTracker.acquireForFocus( script ) with GafferUITest.ContextTrackerTest.UpdateHandler() as h : script.setFocus( script["contextVariables0"] ) - self.waitForUpdate( widget ) + self.waitForPlugValueWidgetUpdate( widget ) self.assertEqual( widget.updateCount, 4 ) self.assertEqual( widget.updateContexts[-1], contextTracker.context( script["add"]["sum"] ) ) @@ -478,7 +465,7 @@ def testContextTrackerUpdates( self ) : with GafferUITest.ContextTrackerTest.UpdateHandler() as h : script.setFocus( script["contextVariables2"] ) - self.waitForUpdate( widget ) + self.waitForPlugValueWidgetUpdate( widget ) self.assertEqual( widget.updateCount, 6 ) self.assertEqual( widget.updateContexts[-1], contextTracker.context( script["add"]["sum"] ) ) diff --git a/python/GafferUITest/StandardNodeToolbarTest.py b/python/GafferUITest/StandardNodeToolbarTest.py index a9a0a419dd..35ed6e9272 100644 --- a/python/GafferUITest/StandardNodeToolbarTest.py +++ b/python/GafferUITest/StandardNodeToolbarTest.py @@ -69,6 +69,6 @@ def testNoUnnecessaryUpdates( self ) : toolbar = GafferUI.StandardNodeToolbar( node ) widget = toolbar._StandardNodeToolbar__layout.plugValueWidget( plug ) - GafferUITest.PlugValueWidgetTest.waitForUpdate( widget ) + self.waitForPlugValueWidgetUpdate( widget ) self.assertEqual( widget.updateCount, 1 ) self.assertEqual( widget.updateContexts[0], script.context() ) diff --git a/python/GafferUITest/StringPlugValueWidgetTest.py b/python/GafferUITest/StringPlugValueWidgetTest.py index 95036d1505..b9c9fbf0fd 100644 --- a/python/GafferUITest/StringPlugValueWidgetTest.py +++ b/python/GafferUITest/StringPlugValueWidgetTest.py @@ -56,12 +56,12 @@ def test( self ) : w = GafferUI.StringPlugValueWidget( n["user"]["p1"] ) self.assertEqual( w.getPlug(), n["user"]["p1"] ) self.assertEqual( w.getPlugs(), { n["user"]["p1"] } ) - GafferUITest.PlugValueWidgetTest.waitForUpdate( w ) + self.waitForPlugValueWidgetUpdate( w ) self.assertEqual( w.textWidget().getText(), "p1" ) self.assertEqual( w.textWidget()._qtWidget().placeholderText(), "" ) n["user"]["p1"].setValue( "x" ) - GafferUITest.PlugValueWidgetTest.waitForUpdate( w ) + self.waitForPlugValueWidgetUpdate( w ) self.assertEqual( w.textWidget().getText(), "x" ) self.assertEqual( w.textWidget()._qtWidget().placeholderText(), "" ) @@ -69,34 +69,34 @@ def test( self ) : self.assertEqual( n["user"]["p1"].getValue(), "x" ) self.assertEqual( n["user"]["p2"].getValue(), "p2" ) - GafferUITest.PlugValueWidgetTest.waitForUpdate( w ) + self.waitForPlugValueWidgetUpdate( w ) self.assertEqual( w.textWidget().getText(), "" ) self.assertEqual( w.textWidget()._qtWidget().placeholderText(), "---" ) w = GafferUI.StringPlugValueWidget( n["user"].children() ) self.assertEqual( w.getPlugs(), { n["user"]["p1"], n["user"]["p2"] } ) - GafferUITest.PlugValueWidgetTest.waitForUpdate( w ) + self.waitForPlugValueWidgetUpdate( w ) self.assertEqual( w.textWidget().getText(), "" ) self.assertEqual( w.textWidget()._qtWidget().placeholderText(), "---" ) n["user"]["p2"].setValue( "x" ) - GafferUITest.PlugValueWidgetTest.waitForUpdate( w ) + self.waitForPlugValueWidgetUpdate( w ) self.assertEqual( w.textWidget().getText(), "x" ) self.assertEqual( w.textWidget()._qtWidget().placeholderText(), "" ) n["user"]["p1"].setValue( "" ) n["user"]["p2"].setValue( "" ) - GafferUITest.PlugValueWidgetTest.waitForUpdate( w ) + self.waitForPlugValueWidgetUpdate( w ) self.assertEqual( w.textWidget().getText(), "" ) self.assertEqual( w.textWidget()._qtWidget().placeholderText(), "" ) Gaffer.Metadata.registerValue( n["user"]["p1"], "stringPlugValueWidget:placeholderText", "test" ) - GafferUITest.PlugValueWidgetTest.waitForUpdate( w ) + self.waitForPlugValueWidgetUpdate( w ) self.assertEqual( w.textWidget().getText(), "" ) self.assertEqual( w.textWidget()._qtWidget().placeholderText(), "" ) Gaffer.Metadata.registerValue( n["user"]["p2"], "stringPlugValueWidget:placeholderText", "test" ) - GafferUITest.PlugValueWidgetTest.waitForUpdate( w ) + self.waitForPlugValueWidgetUpdate( w ) self.assertEqual( w.textWidget().getText(), "" ) self.assertEqual( w.textWidget()._qtWidget().placeholderText(), "test" ) @@ -113,7 +113,7 @@ def testMixedValuesPreserved( self ) : Gaffer.Metadata.registerValue( n["user"]["p2"], "stringPlugValueWidget:placeholderText", "test" ) w = GafferUI.StringPlugValueWidget( { n["user"]["p1"], n["user"]["p2"] } ) - GafferUITest.PlugValueWidgetTest.waitForUpdate( w ) + self.waitForPlugValueWidgetUpdate( w ) self.assertEqual( w.textWidget().getText(), "" ) self.assertEqual( w.textWidget()._qtWidget().placeholderText(), "---" ) @@ -152,7 +152,7 @@ def testExceptionHandling( self ) : # We want that to be reflected in the UI. w = GafferUI.StringPlugValueWidget( script["n"]["p"] ) - GafferUITest.PlugValueWidgetTest.waitForUpdate( w ) + self.waitForPlugValueWidgetUpdate( w ) self.assertEqual( w.textWidget().getText(), "" ) self.assertTrue( w.textWidget().getErrored() ) diff --git a/python/GafferUITest/TestCase.py b/python/GafferUITest/TestCase.py index 0f2db90a56..a01558b434 100644 --- a/python/GafferUITest/TestCase.py +++ b/python/GafferUITest/TestCase.py @@ -120,6 +120,16 @@ def f() : GafferUI.EventLoop.addIdleCallback( f ) GafferUI.EventLoop.mainEventLoop().start() + def waitForPlugValueWidgetUpdate( self, widget ) : + + # Updates are done lazily, so we need to flush any pending updates. + widget._PlugValueWidget__callUpdateFromValues.flush( widget ) + + # And updates for computed values are done in the background, so we + # need to wait until they're done. + if any( isinstance( p, Gaffer.ValuePlug ) and Gaffer.PlugAlgo.dependsOnCompute( p ) for p in widget.getPlugs() ) : + self.uiThreadCallHandler.assertCalled() + def assertExampleFilesExist( self ) : examples = GafferUI.Examples.registeredExamples()