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

[draft] feature/asynctest #626

Draft
wants to merge 2 commits into
base: beta
Choose a base branch
from

Conversation

ubunatic
Copy link
Contributor

  • add AsyncTestBase test class
  • add AsyncTestBase.await* funcs to be used for waiting for UI conditions

Using the await* funcs, we can rewrite the "horribly complicated" (quote Tobi) tests. This should allow us to get rid of the throttle func, i.e., any fixed delays.

The benefit of this await-style testing is that you can set conservative timeouts and would still be able to finish early if all values snap into place quickly. Theoretically, this will produce less flaky tests and an overall test speedup.

After some trial phase for some component tests, we could start a new test suite with that and then slowly move over scenarios.

@sezanzeb sezanzeb marked this pull request as draft February 20, 2023 17:59
@sezanzeb
Copy link
Owner

sezanzeb commented Feb 20, 2023

Thanks for looking into tests.

"horribly complicated" (quote Tobi)

Some of them are really long. I just tried to find the worst of them all, but it seems we already got rid of it at some point luckily.

The test setup improved recently when fixture handling was improved, when it was refactored to multiple files and other stuff.

And there have been some improvements to the way test-events and such are created.

So it has been a lot worse in the past actually. Maybe it actually isn't that bad anymore, but I'm not sure yet.

@jonasBoss
Copy link
Collaborator

I refactored a lot of the tests in #263 and #375, so yeah it has been much worse.

It might be worth looking into #592 for this. I think having everything on a single thread can help a lot with the tests. Often times we are sleeping in tests, because we wait for another thread to receive events trough a pipe. Instead we could replace the pipes with async Queues which should make the test more predictable.
I have currently no time to work on this. So if anyone wants to work on #592 feel free to do so.


I think async assertions are a good improvement 👍.

on the GUI side there is one issue that remains: https://gitlab.gnome.org/GNOME/pygobject/-/merge_requests/189
glib and asyncio are not really compatible.

I worked around this:
https://github.com/jonasBoss/key-mapper/blob/49a1b8fbb076e40abfa4692e3ed1fb81ddf46509/tests/integration/test_gui.py#L375

async def throttle(self, time_=10):
    """Give GTK some time in ms to process everything."""
    # since the injector runs a async event loop in the same thread (for tests)
    # iterate the async and the glib evnet loop simultaneously.
    # Hopefully this does not introduce a bunch of race conditions.

@sezanzeb
Copy link
Owner

Is there still something missing here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants