crossbario / autobahn-python

WebSocket and WAMP in Python for Twisted and asyncio
https://crossbar.io/autobahn
MIT License
2.48k stars 768 forks source link

Failing tests for RawSocketServerProtocol #811

Open Jenselme opened 7 years ago

Jenselme commented 7 years ago

Hi,

I am in the process of updating autobahn to 0.18.1 for fedora and I encounter failing tests on the build server and on my system. Tests output:

=================================== FAILURES ===================================
_________________________ Test.test_raw_socket_server1 _________________________

self = <test_asyncio_rawsocket.Test testMethod=test_raw_socket_server1>

    def test_raw_socket_server1(self):

>       server = RawSocketServerProtocol(max_size=10000)
E       TypeError: __init__() got an unexpected keyword argument 'max_size'

autobahn/asyncio/test/test_asyncio_rawsocket.py:87: TypeError
______________________ Test.test_raw_socket_server_errors ______________________

self = <test_asyncio_rawsocket.Test testMethod=test_raw_socket_server_errors>

    def test_raw_socket_server_errors(self):

>       server = RawSocketServerProtocol(max_size=10000)
E       TypeError: __init__() got an unexpected keyword argument 'max_size'

autobahn/asyncio/test/test_asyncio_rawsocket.py:110: TypeError
=============== 2 failed, 161 passed, 1 skipped in 0.53 seconds ================

The tests are defined there and from what I see in the definition of RawSocketProtocol here the max_size parameter doesn't exist which expected. What I find strange is: according to travis everything is fine.

oberstet commented 7 years ago

There is an issue here, and the reason that Travis did not catch it is that the @pytest.mark.skipif(os.environ.get('USE_ASYNCIO', False), reason="Only for asyncio") decorator here somehow does not work anymore.

It always evaluates to False, despite the env var being set for asyncio tests.

In the Travis reports, eg here, the tests are marked as "skipped":

.tox/py35-asyncio/lib/python3.5/site-packages/autobahn/asyncio/test/test_asyncio_rawsocket.py ssssssss
.tox/py35-asyncio/lib/python3.5/site-packages/autobahn/asyncio/test/test_asyncio_websocket.py ss

This is very subtle, and is easily overlooked.


So I'd say there are actually 3 issues here:

The last one is important. We can't practically look into dozens of reports to watch out for "s" characters manually ..

oberstet commented 7 years ago

@meejah opinions/ideas? could you have a look into this?

meejah commented 7 years ago

Okay, yes, subtle: the environment variable is set, but to the empty string ... which is "non truthy" to Python. So the correct check is: os.environ.get('USE_ASYNCIO', False) is False. PR incoming soon for this, still fixing the actual underlying failure.

meejah commented 7 years ago

Hmm, one of the tests is still failing. I'm not sure if the test or the code is wrong...