Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.64%. Comparing base (8e66133) to head (3cb7af9).

@@           Coverage Diff           @@
##           master     #324   +/-   ##
=======================================
  Coverage   95.63%   95.64%           
=======================================
  Files           4        4           
  Lines         619      620    +1     
=======================================
+ Hits          592      593    +1     
  Misses         27       27           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Thank you for checking! 🙏

Code also looks good. 💯

Could you try adding a test for it? IMO there would be a separate httpserver fixture created for that.
I just like to see that happy-path works in a simplest way, eg a handler which returns the thread name and we compare the thread names are not equal or something like that.

If you don't want to do that, I can do it as well.

Zsolt

hi @delthas

I pushed tests for your PR, I hope you don't mind it - I force-pushed your branch as I had to update ruff in pre-commit, so I rebased your PR to origin/master.

I'll write a howto and update docs in a separate PR, but I'll let you some time (a few days) to review the tests. Feel free to share if you have any better ideas for the tests. Probably it could be made better by having proper synchronization primitives, at the moment it could be flaky if the client sends requests slower.

Thanks,
Zsolt

Hi,

Thanks for working on this MR and adding the tests.

I just reviewed the tests and they look good to me; this is exactly what I'd do (and what I did on my end when I tested the feature): add a handler that sleeps, run many connections, check that the time is less than handlind them sequentially. On my end I used multiprocessing ThreadPool submit to start all the requests in parallel; here I see we handle them sequentially but that should also work.

Thanks!

It just came to my mind that in pytest-httpserver we use a session scoped fixture so having that "werzeug joins the threads on stop" possibly won't be sufficient - as the server is not stopped and started between the tests (in my test it is not tested).

I just want to verify this before the release. I think we should add some join call to the clear method.

Theoretically there would be no issue leaving a thread behind as the handlers registry gets cleared between tests, so there's no chance the old handler getting called, but I feel it would be better to not leave behind any running thread.

Zsolt

hi @delthas ,

I checked the issue described in the last comment. This leaves behind the running handler threads, that's apparent, but I found no way to wait on those threads. Given the current state of socketserver.py it is not possible to query the running threads safely.

But this is not a big issue as those handler threads will exit at least at the point when the server is stopped, and they won't receive any new requests as the handlers "registry" is cleared between the tests. I will make a note about this in the documentation, though.

So this PR can be merged.

Thanks for the contribution,

Zsolt

Thanks!