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

Add unused port helpers for UDP #99

Merged
merged 5 commits into from Jan 7, 2022

Conversation

dbuse
Copy link
Contributor

@dbuse dbuse commented Oct 2, 2018

Extends the unused_tcp_port and unused_tcp_port_factory mechanisms for UDP ports.

Extends the unused_tcp_port and unused_tcp_port_factory mechanisms for
UDP ports.
@eirnym
Copy link

eirnym commented Oct 3, 2018

Could you make socket_type setting explicit for TCP as well? I understand that it's default, but it would be much easier to read

Also both factories unused_udp_port_factory and unused_tcp_port_factory are essencially the same. The only difference is socket_type.

Additionally, please, spread documentation to fixtures.

@dbuse
Copy link
Contributor Author

dbuse commented Oct 3, 2018

Could you make socket_type setting explicit for TCP as well? I understand that it's default, but it would be much easier to read

Agreed, that makes sense.

Also both factories unused_udp_port_factory and unused_tcp_port_factory are essencially the same. The only difference is socket_type.

True. My intention was to stay fully backward-compatible. So changing unused_tcp_port_factory to accept a socket type would invalidate the tcp' in its name. But I could extract the implementation into a common function (like with the _unused port' function) and make the fixtures a slim wrapper.

Additionally, please, spread documentation to fixtures.

Also agreed.

I'll look into the code changes soon.

pytest_asyncio/plugin.py Outdated Show resolved Hide resolved
@asvetlov asvetlov merged commit d48569e into pytest-dev:master Jan 7, 2022
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 this pull request may close these issues.

None yet

3 participants