Skip to content

Run Session class tests in an event loop #881

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

Merged
merged 3 commits into from
Mar 22, 2023
Merged

Conversation

sgillies
Copy link
Contributor

@sgillies sgillies commented Mar 21, 2023

Resolves #879

  • This PR is as small and focused as possible
  • This PR does not break any examples or I have updated them

Review please @jreiberkyle

Sorry, something went wrong.

@sgillies sgillies self-assigned this Mar 21, 2023
@sgillies sgillies requested a review from jreiberkyle March 21, 2023 18:41

Changed:
- Session class unit tests are marked to be run within an event loop and are
no longer skipped (#881).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Change log updated

@@ -16,6 +17,7 @@ async def test_session_get_client(client_name, client_class):
assert isinstance(client, client_class)


@pytest.mark.asyncio
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I lost these along the way. It would have helped if pytest failed instead of skipping. Maybe we can configure it to do so. I don't immediately see how, it seems particular to async functions.

Copy link
Contributor

Choose a reason for hiding this comment

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

agreed. will keep an eye out for a way to do this.

@jreiberkyle jreiberkyle changed the base branch from main to rc3dev-fixes March 21, 2023 20:25
Copy link
Contributor

@jreiberkyle jreiberkyle left a comment

Choose a reason for hiding this comment

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

fixes the skipped tests, thanks!

@jreiberkyle
Copy link
Contributor

as this is approved with no comments, I'll merge to unblock #882

@jreiberkyle jreiberkyle merged commit 0d5e420 into rc3dev-fixes Mar 22, 2023
@jreiberkyle jreiberkyle deleted the issue879 branch March 22, 2023 03:03
@jreiberkyle jreiberkyle mentioned this pull request Apr 14, 2023
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.

get_client test warnings / tests skipped
2 participants