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

Hypothesis test fail because of change in Hypothesis #145

Closed
simonfagerholm opened this issue Mar 20, 2020 · 3 comments · Fixed by #148
Closed

Hypothesis test fail because of change in Hypothesis #145

simonfagerholm opened this issue Mar 20, 2020 · 3 comments · Fixed by #148

Comments

@simonfagerholm
Copy link
Contributor

simonfagerholm commented Mar 20, 2020

See PR #142 where it first occurs, but it will happen on all builds after last of feb because of HypothesisWorks/hypothesis#2356.

@Zac-HD maybe you can help with a fix as you are involved in both this and hypothesis package.

Basically the problem is that the even_loop is function scope.

@Zac-HD
Copy link
Member

Zac-HD commented Mar 23, 2020

We'll need to do one of two things:

  • If the event_loop fixture can safely be shared between tests, make it session (or module) scoped.
  • If it can't be shared between tests, the current situation is already broken and this warning has just exposed that latent bug!

I suspect that the latter case is correct, and the response is simply "you cannot use Hypothesis with a fixture-provided event loop" (and so test_can_use_fixture_provided_event_loop can be deleted). In this case we may also need to replace the Hypothesis-specific part with get_event_loop() to use asyncio.run() instead for proper isolation.

@simonfagerholm
Copy link
Contributor Author

In the documentation I spotted this:

All scopes are supported, but if you use a non-function scope you will need to redefine the event_loop fixture to have the same or broader scope. Async fixtures need the event loop, and so must have the same or narrower scope than the event_loop fixture.

So I think alternative 1 would be possible. I can fix the test in a PR if you agree that that would be best, @Zac-HD.

@Zac-HD
Copy link
Member

Zac-HD commented Mar 23, 2020

Yep, using a broader scope sounds good then 👍

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 a pull request may close this issue.

2 participants