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.

Sign up for GitHub

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

Jump to bottom

chore: add showcase e2e tests for async rest #2167

Merged
merged 4 commits into from
Sep 17, 2024
Merged

chore: add showcase e2e tests for async rest #2167

merged 4 commits into from
Sep 17, 2024

Conversation

ohmayr
Copy link
Contributor

@ohmayr ohmayr commented Sep 17, 2024

No description provided.

@product-auto-label product-auto-label bot added the size: m Pull request size is medium. label Sep 17, 2024
@ohmayr ohmayr force-pushed the add-showcase-testing branch 4 times, most recently from aaacccd to 279fc56 Compare September 17, 2024 15:46
@ohmayr ohmayr marked this pull request as ready for review September 17, 2024 16:33
@ohmayr ohmayr requested a review from a team as a code owner September 17, 2024 16:33
ohmayr
ohmayr commented Sep 17, 2024
View reviewed changes
Copy link
Contributor Author

@ohmayr ohmayr left a comment

Choose a reason for hiding this comment

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

This issue link and any follow up links within this file will be updated to:
#2168

ohmayr
ohmayr commented Sep 17, 2024
View reviewed changes
tests/system/test_streams.py Outdated Show resolved Hide resolved
tests/system/test_streams.py Show resolved Hide resolved
tests/system/test_retry.py Outdated Show resolved Hide resolved
tests/system/test_pagination.py Outdated Show resolved Hide resolved
vchudnov-g
vchudnov-g approved these changes Sep 17, 2024
View reviewed changes
Copy link
Contributor

@vchudnov-g vchudnov-g left a comment

Choose a reason for hiding this comment

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

The only non-trivial comment is about the alternative templates. Please address/discuss that before merging.

.github/workflows/tests.yaml Outdated Show resolved Hide resolved
gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/__init__.py.j2 Show resolved Hide resolved
tests/system/test_client_context_manager.py
@@ -50,7 +51,7 @@ async def test_client_async(async_echo):
@pytest.mark.asyncio
async def test_client_destroyed_async(async_echo):
await async_echo.__aexit__(None, None, None)
with pytest.raises(grpc._cython.cygrpc.UsageError):
with pytest.raises((grpc._cython.cygrpc.UsageError, exceptions.TransportError)):
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious: how does pytest decide what to pass to the echo or async_echo parameters in these test functions? I'm used to seeing the @parametrize annotation, and I see there's also the pytest_generate_tests option, but I don't see that in our codebase. So what's at play here? Is nox somehow doing the work?

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'm not sure what the ask here is exactly. echo and async_echo are defined as fixtures within conftest.py.

Essentially, we pre-construct the clients (for the two transport types that we support which are parameterized) and then use them here in our test cases to call methods.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's the information I needed: the parameters are filled in automatically by the fixtures we define in conftest.py. (I haven't internalized all the pytest usage patterns yet. ;-) )

tests/system/test_unary.py Outdated Show resolved Hide resolved
Base automatically changed from implement-generic-async-rest-method to async-rest-support-in-gapics September 17, 2024 18:20
@product-auto-label product-auto-label bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels Sep 17, 2024
@ohmayr ohmayr force-pushed the add-showcase-testing branch 3 times, most recently from 439152e to cb8372b Compare September 17, 2024 19:52
@ohmayr ohmayr merged commit 4567c64 into async-rest-support-in-gapics Sep 17, 2024
73 checks passed
@ohmayr ohmayr deleted the add-showcase-testing branch September 17, 2024 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vchudnov-g vchudnov-g vchudnov-g approved these changes

@parthea parthea parthea approved these changes

Assignees

@parthea parthea

@vchudnov-g vchudnov-g

Labels
size: l Pull request size is large.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ohmayr @parthea @vchudnov-g