enthought / pyface

pyface: traits-capable windowing framework
Other
105 stars 55 forks source link

Don't do unnecessary condition evaluations in assertEventuallyTrueInGui #1168

Closed mdickinson closed 1 year ago

mdickinson commented 1 year ago

GuiTestAssistant.assertEventuallyTrueInGui does an unnecessary extra condition evaluation after the event loop has exited. In the worst case, when the condition is only valid during a running event loop, this can cause spurious test failures.

This PR updates the code so that it doesn't re-evaluate the condition once the condition has evaluated to True for the first time.

Design note: the bool() call is in there to avoid hanging onto references in case the user-supplied condition returns something more complicated that a simple bool.


EDIT: I've also updated the PR to use the exit method instead of the quit method within event_loop_until_condition; all we want here is to exit the event loop that was started using self.qt_app.exec_ - not to tear down anything else.

mdickinson commented 1 year ago

CI is failing for unrelated reasons. I suggest that we pin our PySide6 version in CI in the short term.

mdickinson commented 1 year ago

1169 should fix the unrelated CI failures, so that we can see whether there are any related failures.

mdickinson commented 1 year ago

Looks like this makes some existing macOS tests unhappy. Investigating.

mdickinson commented 1 year ago

Marking as draft, since there seem to be some unexpected interactions with the ModalDialogTester: see #1170.

corranwebster commented 1 year ago

The failures for the font and color dialogs seem to be happening in the tests for the utility functions get_color and get_font which wrap the dialogs, rather than the main tests for the dialogs themselves.

Possibly there is an issue with needed objects being gc'd or not cleaned up correctly?

corranwebster commented 1 year ago

This should resolve #1170 as well.

mdickinson commented 1 year ago

Thanks, @corranwebster. I'm happy for this to be merged if you are.