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

test_remote_address fails #1427

Closed
mtelka opened this issue Dec 18, 2023 · 9 comments
Closed

test_remote_address fails #1427

mtelka opened this issue Dec 18, 2023 · 9 comments

Comments

@mtelka
Copy link

mtelka commented Dec 18, 2023

I'm packaging websockets for OpenIndiana and when I ran tests I found following failures:

======================================================================
FAIL: test_remote_address (tests.sync.test_connection.ClientConnectionTests)
Connection has a remote_address attribute.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "$(BUILD_DIR)/tests/sync/test_connection.py", line 665, in test_remote_address
    self.assertIsNotNone(self.connection.remote_address)
AssertionError: unexpectedly None

======================================================================
FAIL: test_remote_address (tests.sync.test_connection.ServerConnectionTests)
Connection has a remote_address attribute.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "$(BUILD_DIR)/tests/sync/test_connection.py", line 665, in test_remote_address
    self.assertIsNotNone(self.connection.remote_address)
AssertionError: unexpectedly None

----------------------------------------------------------------------
Ran 1297 tests in 9.735s

FAILED (failures=2, skipped=8)
@aaugustin
Copy link
Member

This is likely related to the implementation of socket.socketpair and/or socket.socket.getpeername on OpenIndiana.

That's what I'm using for testing and how remote_connection is implemented respectively.

The documentation of getpeername acknowledges that it isn't supported on some systems. OpenIndiana may be one of those systems.

Do you a concrete proposal that doesn't make the code or the tests more complex?

@aaugustin
Copy link
Member

aaugustin commented Dec 19, 2023

For clarity: to me this is an OpenIndiana and/or Python issue; websockets isn't the right level to fix this.

If there's an easy solution that adds no maintenance cost and helps you, of course I'll consider it.

The onus of proposing it is on you, though, because you're the one who wants to run the tests on OpenIndiana (and I wish you didn't, as explained in #1426).

@mtelka
Copy link
Author

mtelka commented Dec 19, 2023

This is likely related to the implementation of socket.socketpair and/or socket.socket.getpeername on OpenIndiana.

Could you please suggest some simple test that would prove the problem is either in socket.socketpair and/or socket.socket.getpeername on OpenIndiana?

I tried this:

$ python
Python 3.9.16 (main, Feb 19 2023, 15:42:40) 
[GCC 10.4.0] on sunos5
Type "help", "copyright", "credits" or "license" for more information.
>>> import socket
>>> socket.socketpair()
(<socket.socket fd=5, family=AddressFamily.AF_UNIX, type=SocketKind.SOCK_STREAM, proto=0>, <socket.socket fd=4, family=AddressFamily.AF_UNIX, type=SocketKind.SOCK_STREAM, proto=0>)

But that's apparently not enough.

Thank you.

@aaugustin
Copy link
Member

Try this:

>>> local, remote = socket.socketpair()
>>> local.getpeername()
''
>>>

I expect that you'll get None instead of the empty string.

This shows that the test isn't very good... I will think about it.

@aaugustin
Copy link
Member

Perhaps I should rewrite that test to use a real IP socket.

@mtelka
Copy link
Author

mtelka commented Dec 19, 2023

Yes, exactly, None:

$ python
Python 3.9.16 (main, Feb 19 2023, 15:42:40) 
[GCC 10.4.0] on sunos5
Type "help", "copyright", "credits" or "license" for more information.
>>> import socket
>>> local, remote = socket.socketpair()
>>> local.getpeername()
>>> print(local.getpeername())
None
>>>

@aaugustin
Copy link
Member

Thank you for confirming.

Honestly this test doesn't test anything beyond the existence of a remote_address method that doesn't crash. I could remove the assertion that it doesn't return None. Anyway the return value is "whatever getpeername returns"; that's platform specific.

@aaugustin
Copy link
Member

I mocked socket methods with platform-specific behavior to make the tests more robust. Hopefully that'll do the job on OpenIndiana. Please let me know if it doesn't.

@mtelka
Copy link
Author

mtelka commented Jan 1, 2024

I just tested websockets 12.0 + b3c5195 and I can confirm that the issue is gone.

Thank you!

aaugustin added a commit that referenced this issue Sep 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants