Fix trio.socket.SocketType.bind to work with PathLike arguments again #3042

regnarg · 2024-07-31T10:40:08Z

codecov · 2024-07-31T10:43:38Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.63%. Comparing base (20f9291) to head (d95981c).
Report is 238 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3042   +/-   ##
=======================================
  Coverage   99.63%   99.63%           
=======================================
  Files         120      121    +1     
  Lines       17832    17841    +9     
  Branches     3204     3206    +2     
=======================================
+ Hits        17767    17776    +9     
  Misses         46       46           
  Partials       19       19

Files with missing lines	Coverage Δ
src/trio/_socket.py	`100.00% <100.00%> (ø)`
src/trio/_tests/test_socket.py	`100.00% <100.00%> (ø)`

... and 2 files with indirect coverage changes

---- 🚨 Try these New Features:

Flaky Tests Detection - Detect and resolve failed and flaky tests

A5rocks

Could you add a regression test? I'm not sure exactly how to, but it's probably fine to use a tmpfile as a socket and actually do the IO.

EDIT: it looks like we test open_unix_socket already, maybe just make the name types a 2x2 parametrized thing? (not sure about keeping it in test_highlevel_open_unix_stream but honestly it's either that or test_socket and that's better.)

trio/src/trio/_tests/test_highlevel_open_unix_stream.py

Lines 54 to 76 in 20f9291

    
           async def test_open_unix_socket() -> None: 
        
               for name_type in [Path, str]: 
        
                   name = tempfile.mktemp() 
        
                   serv_sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) 
        
                   with serv_sock: 
        
                       serv_sock.bind(name) 
        
                       try: 
        
                           serv_sock.listen(1) 
        
                           # The actual function we're testing 
        
                           unix_socket = await open_unix_socket(name_type(name)) 
        
                           async with unix_socket: 
        
                               client, _ = serv_sock.accept() 
        
                               with client: 
        
                                   await unix_socket.send_all(b"test") 
        
                                   assert client.recv(2048) == b"test" 
        
                                   client.sendall(b"response") 
        
                                   received = await unix_socket.receive_some(2048) 
        
                                   assert received == b"response" 
        
                       finally: 
        
                           os.unlink(name)

CoolCat467 · 2024-07-31T17:02:11Z

pre-commit.ci autofix

Tests python-trio#3041

regnarg · 2024-07-31T17:31:36Z

Could you add a regression test?

It took a few tries but test is there and seems to pass.

CoolCat467 · 2024-07-31T18:17:35Z

Why so many force pushes?

regnarg · 2024-07-31T18:46:46Z

Because I did not figure out how to successfully run the tests locally so it took me several tries to get the CI to pass. And I do not need to clutter commit history with trivial fixes of trivial mistakes.

CoolCat467 · 2024-07-31T19:49:02Z

Not really an issue, we use squash and merge most of the time I think

A5rocks · 2024-07-31T23:34:29Z

Because I did not figure out how to successfully run the tests locally so it took me several tries to get the CI to pass. And I do not need to clutter commit history with trivial fixes of trivial mistakes.

Was there any specific issues? We should make this better for new contributors. Was it just a matter of finding the right pytest invocation?

A5rocks

Sorry, I keep forgetting: can you add a changelog entry? (A newsfragment)

A5rocks · 2024-08-06T17:20:50Z

@regnarg just a bump for this PR!

A5rocks · 2024-08-21T03:45:47Z

Could someone else check the newsfragment looks fine?

CoolCat467

Looks good to me.

import os
reveal_type(os.fspath)

with mypy yields

note: Revealed type is "Overload(
    def (path: builtins.str) -> builtins.str,
    def (path: builtins.bytes) -> builtins.bytes,
    def [AnyStr in (builtins.str, builtins.bytes)] (path: os.PathLike[AnyStr`-1]) -> AnyStr`-1
)"

so return is indeed str or bytes if pathlike is given

fixed

trio-bot · 2024-08-21T04:40:50Z

Hey @regnarg, it looks like that was the first time we merged one of your PRs! Thanks so much! 🎉 🎂

If you want to keep contributing, we'd love to have you. So, I just sent you an invitation to join the python-trio organization on Github! If you accept, then here's what will happen:

Github will automatically subscribe you to notifications on all our repositories. (But you can unsubscribe again if you don't want the spam.)
You'll be able to help us manage issues (add labels, close them, etc.)
You'll be able to review and merge other people's pull requests
You'll get a [member] badge next to your name when participating in the Trio repos, and you'll have the option of adding your name to our member's page and putting our icon on your Github profile (details)

If you want to read more, here's the relevant section in our contributing guide.

Alternatively, you're free to decline or ignore the invitation. You'll still be able to contribute as much or as little as you like, and I won't hassle you about joining again. But if you ever change your mind, just let us know and we'll send another invitation. We'd love to have you, but more importantly we want you to do whatever's best for you.

If you have any questions, well... I am just a humble Python script, so I probably can't help. But please do post a comment here, or in our chat, or on our forum, whatever's easiest, and someone will help you out!

Fix trio.socket.SocketType.bind to work with PathLike arguments again

555eb5f

fixes python-trio#3041

A5rocks reviewed Jul 31, 2024

View reviewed changes

regnarg force-pushed the fix-unix-path-bind branch 2 times, most recently from 027a0be to 7a55d0e Compare July 31, 2024 16:26

Test passing various types to trio.socket.SocketType.{bind,connect}

87a001f

Tests python-trio#3041

regnarg force-pushed the fix-unix-path-bind branch from c14b7bc to 87a001f Compare July 31, 2024 17:16

A5rocks approved these changes Jul 31, 2024

View reviewed changes

A5rocks previously requested changes Jul 31, 2024

View reviewed changes

Add a newsfragment

1f2909d

A5rocks self-requested a review August 21, 2024 03:45

Appease pre-commit

d95981c

CoolCat467 approved these changes Aug 21, 2024

View reviewed changes

A5rocks merged commit 57c95a5 into python-trio:main Aug 21, 2024

	async def test_open_unix_socket() -> None:
	for name_type in [Path, str]:
	name = tempfile.mktemp()
	serv_sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
	with serv_sock:
	serv_sock.bind(name)
	try:
	serv_sock.listen(1)

	# The actual function we're testing
	unix_socket = await open_unix_socket(name_type(name))

	async with unix_socket:
	client, _ = serv_sock.accept()
	with client:
	await unix_socket.send_all(b"test")
	assert client.recv(2048) == b"test"

	client.sendall(b"response")
	received = await unix_socket.receive_some(2048)
	assert received == b"response"
	finally:
	os.unlink(name)

Uh oh!

Fix trio.socket.SocketType.bind to work with PathLike arguments again #3042

Fix trio.socket.SocketType.bind to work with PathLike arguments again #3042

Uh oh!

Conversation

regnarg commented Jul 31, 2024

Uh oh!

codecov bot commented Jul 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

A5rocks left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CoolCat467 commented Jul 31, 2024

Uh oh!

regnarg commented Jul 31, 2024

Uh oh!

CoolCat467 commented Jul 31, 2024

Uh oh!

regnarg commented Jul 31, 2024

Uh oh!

CoolCat467 commented Jul 31, 2024

Uh oh!

A5rocks commented Jul 31, 2024

Uh oh!

A5rocks left a comment

Choose a reason for hiding this comment

Uh oh!

A5rocks commented Aug 6, 2024

Uh oh!

A5rocks commented Aug 21, 2024

Uh oh!

CoolCat467 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

trio-bot bot commented Aug 21, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Jul 31, 2024 •

edited

Loading

A5rocks left a comment •

edited

Loading

CoolCat467 left a comment •

edited

Loading