Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove a test that uses UITester #1788

Merged
merged 3 commits into from
Apr 29, 2024
Merged

Conversation

mdickinson
Copy link
Member

@mdickinson mdickinson commented Apr 24, 2024

This PR removes a test that's using UITester from TraitsUI. This test exposed the Traits test suite to a class of possible issues with 3rd party GUI frameworks (in this case PySide6); historically, UITester has also been a particularly fragile part of TraitsUI.

In its place is a test that at least executes the Enum.create_editor method, but doesn't attempt to create an instance of the editor.

We also fix an issue with test dependencies and the latest release of Sphinx: defusedxml is now required to run the tests for the Traits documenter.

Closes #1787

@mdickinson
Copy link
Member Author

Updated to add back a test that at least exercises the Enum.create_editor method.

@mdickinson
Copy link
Member Author

@flongford Do you have bandwidth to review?

CI is unfortunately still failing because of an orthogonal issue, that's fixed separately in #1789.

Copy link

@flongford flongford left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Reducing the scope of the failing test makes sense considering the ultimate aim to remove traits dependency on traitsui anyway.

Good luck resolving those MacOS CI failures...

@mdickinson
Copy link
Member Author

The issue with Python 3.8 and Python 3.9 no longer being available on macos-latest runners has been resolved upstream. I'm made a tiny commit to this PR to add "defusedxml" to the test dependencies - without this, some of the Sphinx tests were failing.

If the CI passes, I'll merge.

@mdickinson
Copy link
Member Author

I've also updated the PR description.

Merging!

@mdickinson mdickinson merged commit 8a36a21 into main Apr 29, 2024
29 checks passed
@mdickinson mdickinson deleted the fix/remove-use-of-ui-tester branch April 29, 2024 07:23
mdickinson added a commit that referenced this pull request May 7, 2024
Since #1788, we have only one test module that makes use of the Qt event
loop. That test module contains tests for the behaviour of handlers that
use `dispatch='ui'` mechanism to redispatch off-thread notifications to
the ui thread.

This PR reworks that test module, with some significant collateral
damage along the way.

In detail:

- reworks that test module (`test_ui_notifiers`) to avoid the need for
the Qt event loop; instead, it tests against a `ui_handler` based on
asyncio, which redispatches to the running asyncio event loop
- adds a `get_ui_handler` counterpart to `set_ui_handler`, and exposes
both functions in `traits.api`
- adds type hints for `get_ui_handler` and `set_ui_handler`
- removes two public module globals from `trait_notifiers`: `ui_handler`
has been made private, while `ui_thread` is removed altogether
- fixes a bug where ui dispatch didn't do the right thing (PR #1740 was
incomplete; this bug should have been caught at review time on that PR)
- makes another couple of drive-by cleanups, removing a very old check
for `threading.local()` being a dict (which it hasn't been in living
memory), and tidying up some uses of thread identity.
mdickinson added a commit that referenced this pull request May 7, 2024
Since #1788 and #1792, we should no longer need a working Qt backend
when testing. Accordingly, this PR:

- removes PySide6 as a test dependency
- removes the need to `apt-get` Qt packages on Ubuntu runners
- removes the use of `xvfb-run` on Ubuntu runners
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Traits test suite failing with PySide 6.7.0
2 participants