python-trio / trio-websocket

WebSocket client and server implementation for Python Trio
MIT License
70 stars 26 forks source link

Issue 96 #100

Closed mehaase closed 5 years ago

mehaase commented 5 years ago

I was unable to write a unit test that reliably triggered this bug, so instead I cleaned up my repro script and checked it into the repo.

coveralls commented 5 years ago

Pull Request Test Coverage Report for Build 17


Changes Missing Coverage Covered Lines Changed/Added Lines %
trio_websocket/_impl.py 4 9 44.44%
<!-- Total: 5 10 50.0% -->
Totals Coverage Status
Change from base Build 15: -0.3%
Covered Lines: 379
Relevant Lines: 412

💛 - Coveralls
mehaase commented 5 years ago

I don't think it would help to put the delay inside the server implementation, because the delay works fine where it is now (in the test itself). The problem is that the bug itself is non-deterministic: in the test code I posted above, it triggers the bug on average once for every 5-10 connections. I wrote a unit test that executes 1000 connections, and this does reliably trigger the bug (but not with probability 1.0), but it makes for a very slow test.

It occurred to me that a fuzzing test might be more useful in the long run. It would have broader applicability than a single test just for this issue. It wouldn't be part of the unit test suite, but it could be executed manually prior to each release.