fix tests for strict exception groups #42
Conversation
VincentVanlaer
commented
Apr 2, 2024
•
VincentVanlaer
commented
- fix tests to work with strict and non-strict exception groups
- add python 3.12 to build, drop python 3.7 (end of life)
- upgrade dev dependencies where needed for newer python versions
VincentVanlaer
force-pushed
the
test-dependencies
branch
from
34603e8 to
3ef067b
Compare
Required to do testing on python 3.11. Since anyio 4.0 always wraps single exceptions with exception groups, we need to handle those in certain pytest.raises calls. Luckily, trio has some helpers for this that work irrespectively of whether the async loop is trio or not.
Anyio 4.0 has dropped python 3.7 as it is EOL.
VincentVanlaer
force-pushed
the
test-dependencies
branch
from
6833707 to
4d4d2d4
Compare
This way tests can still be run with older versions of trio and anyio
| from purerpc.test_utils import run_purerpc_service_in_process, run_grpc_service_in_process, \ | ||
| async_iterable_to_list, random_payload, grpc_client_parallelize, purerpc_channel, purerpc_client_parallelize, grpc_channel | ||
| from .exceptiongroups import unwrap_exceptiongroups_single |
There was a problem hiding this comment.
Should unwrap_exceptiongroups_single go in test_utils? Like test_utils.async_iterable_to_list...
Though I'm not sure if test_utils is a public API cause unwrapping exceptiongroups is none of purerpc's business and maybe shouldn't go in public API -- but neither should turning an async iterable into a list.
There was a problem hiding this comment.
Yeah, the reason I put it in a different location is because I didn't want to potentially expose it to users.
belm0
force-pushed
the
test-dependencies
branch
8 times, most recently
from
686006e to
af7e581
Compare
belm0
changed the title
|
@VincentVanlaer I explained it in a thread of another project, but merely patching tests to handle strict exception groups doesn't provide any assurance that the runtime code supports strict exceptions. For example, where the API was previously raising a bare exception, it might now raise an ExceptionGroup (the API should probably never raise ExceptionGroup, since internal use of nurseries/concurrency is an implementation detail). Likewise, internally to the runtime, if there are any |
| with pytest.raises(anyio.ClosedResourceError), unwrap_exceptiongroups_single(): | ||
| async with purerpc.insecure_channel("localhost", port) as channel: | ||
| stub = echo_grpc.EchoStub(channel) |
There was a problem hiding this comment.
Especially here. Now the purerpc API itself may raise ExceptionGroup. I don't think this implementation detail should be exposed, and the API should continue to raise ClosedResourceError.
I explained it in a thread of another project, but merely patching tests to handle strict exception groups doesn't provide any assurance that the runtime code supports strict exceptions. For example, where the API was previously raising a bare exception, it might now raise an ExceptionGroup (the API should probably never raise ExceptionGroup, since internal use of nurseries/concurrency is an implementation detail). Likewise, internally to the runtime, if there are any except statements catching bare exceptions, they need to be ported to excptiongroup catches.
|
My main goal wasn't to fully assure the exception groups are as expected, but to have the dependencies be up to date to support python 3.11 (grpc is problematic, as it seems you have noticed) for runnign tests locally. I should have explained that in more detail, my bad! I agree with your assessment about exception groups though. Thanks for making the final changes! |
|
@VincentVanlaer what version of Trio is used in your project? I'm thinking of doing a purerpc release with the restriction |
|
I'm currently using the latest trio release, and haven't encountered any problems. I'll make an issue to discuss this further this evening. |
