Skip to content

PlugValueWidgetTest : Fix handling of background updates - #7081

Open
johnhaddon wants to merge 2 commits into
GafferHQ:mainfrom
johnhaddon:plugValueWidgetTestFixes
Open

PlugValueWidgetTest : Fix handling of background updates#7081
johnhaddon wants to merge 2 commits into
GafferHQ:mainfrom
johnhaddon:plugValueWidgetTestFixes

Conversation

@johnhaddon

Copy link
Copy Markdown
Member

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 triggering the update from inside the UIThreadCallHandler's scope we avoid this possibility.

@johnhaddon
johnhaddon requested a review from ericmehl August 6, 2026 15:40
@johnhaddon johnhaddon self-assigned this Aug 6, 2026
@github-project-automation github-project-automation Bot moved this to Pending Review in Work in Progress Aug 6, 2026
@johnhaddon

Copy link
Copy Markdown
Member Author

Looks like I failed to notice that other tests were using PlugValueWidgetTest.waitForUpdate(). I'll fix those tomorrow.

@johnhaddon
johnhaddon force-pushed the plugValueWidgetTestFixes branch from d58e136 to c4933b7 Compare August 7, 2026 11:28
@johnhaddon

johnhaddon commented Aug 7, 2026

Copy link
Copy Markdown
Member Author

I've pushed a new version which should take care of the other spots that use waitForUpdate(). It's a tiny bit annoying in that a lot of code gets indented one level further. A possible alternative to that would be for TestCase.setup() to install a UIThreadCallHandler that is removed by TestCase.tearDown(), so we always have one scoped and can drop the indentation. This would be a bigger change, but might be worth it.

It uses a new central GafferUITest.TestCase.uiThreadCallHandler that lets us avoid the race condition, and also makes it easier to handle UI thread calls in general.

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.
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Pending Review

Development

Successfully merging this pull request may close these issues.

1 participant