cherrypy / cheroot

Cheroot is the high-performance, pure-Python HTTP server used by CherryPy. Docs -->
https://cheroot.cherrypy.dev
BSD 3-Clause "New" or "Revised" License
185 stars 90 forks source link

PyPy3.10 test regression: test_remains_alive_post_unhandled_exception fails #695

Open mgorny opened 6 months ago

mgorny commented 6 months ago

❓ I'm submitting a ...

🐞 Describe the bug. What is the current behavior? In cheroot 10.0.1, the tests fails against PyPy3.10 7.3.14, whereas they've passed in 10.0.0. The regressing test is:

         - cheroot/test/test_conn.py:886 test_remains_alive_post_unhandled_exception

❓ What is the motivation / use case for changing the behavior? Err, making tests pass again?

πŸ’‘ To Reproduce

Steps to reproduce the behavior:

  1. tox -e pypy3 β†’ it will fail immediately because of a bug in typeguard
  2. . .tox/pypy3/bin/activate
  3. pip uninstall typeguard
  4. python -m pytest

πŸ’‘ Expected behavior No test regressions.

πŸ“‹ Details

Traceback ```pytb ――――――――――――――――――――――――――――――――――――――――――――― test_remains_alive_post_unhandled_exception ――――――――――――――――――――――――――――――――――――――――――――― [gw5] linux -- Python 3.10.13 /tmp/cheroot/.tox/pypy3/bin/python mocker = monkeypatch = <_pytest.monkeypatch.MonkeyPatch object at 0x00007f14ed636790> test_client = testing_server = wsgi_server_thread = @pytest.mark.xfail( IS_CI and IS_PYPY and IS_PY36 and not IS_SLOW_ENV, reason='Fails under PyPy 3.6 under Ubuntu 20.04 in CI for unknown reason', # NOTE: Actually covers any Linux strict=False, ) def test_remains_alive_post_unhandled_exception( mocker, monkeypatch, test_client, testing_server, wsgi_server_thread, ): """Ensure worker threads are resilient to unhandled exceptions.""" class ScaryCrash(BaseException): # noqa: WPS418, WPS431 """A simulated crash during HTTP parsing.""" _orig_read_request_line = ( test_client.server_instance. ConnectionClass.RequestHandlerClass. read_request_line ) def _read_request_line(self): _orig_read_request_line(self) raise ScaryCrash(666) monkeypatch.setattr( test_client.server_instance.ConnectionClass.RequestHandlerClass, 'read_request_line', _read_request_line, ) server_connection_close_spy = mocker.spy( test_client.server_instance.ConnectionClass, 'close', ) # NOTE: The initial worker thread count is 10. assert len(testing_server.requests._threads) == 10 test_client.get_connection().send(b'GET / HTTP/1.1') # NOTE: This spy ensure the log entry gets recorded before we're testing # NOTE: them and before server shutdown, preserving their order and making # NOTE: the log entry presence non-flaky. while not server_connection_close_spy.called: # noqa: WPS328 pass # NOTE: This checks for whether there's any crashed threads while testing_server.requests.idle < 10: # noqa: WPS328 pass assert len(testing_server.requests._threads) == 10 assert all( worker_thread.is_alive() for worker_thread in testing_server.requests._threads ) testing_server.interrupt = SystemExit('test requesting shutdown') assert not testing_server.requests._threads wsgi_server_thread.join() # no extra logs upon server termination actual_log_entries = testing_server.error_log.calls[:] testing_server.error_log.calls.clear() # prevent post-test assertions expected_log_entries = ( ( logging.ERROR, '^Unhandled error while processing an incoming connection ' r'ScaryCrash\(666,?\)$', ), ( logging.INFO, '^SystemExit raised: shutting down$', ), ) > assert len(actual_log_entries) == len(expected_log_entries) E assert 1 == 2 E +1 E -2 ScaryCrash = .ScaryCrash'> _orig_read_request_line = _read_request_line = ._read_request_line at 0x00007f14ed3c0200> actual_log_entries = [ErrorLogCall(msg='SystemExit raised: shutting down', level=20, traceback='')] expected_log_entries = ((40, '^Unhandled error while processing an incoming connection ScaryCrash\\(666,?\\)$'), (20, '^SystemExit raised: shutting down$')) mocker = monkeypatch = <_pytest.monkeypatch.MonkeyPatch object at 0x00007f14ed636790> server_connection_close_spy = test_client = testing_server = wsgi_server_thread = cheroot/test/test_conn.py:963: AssertionError ```

πŸ“‹ Environment

πŸ“‹ Additional context I can reproduce with main (at f156ccd8b13ebc081797c1309d084ae0f3a1b08b) as well.

webknjaz commented 6 months ago

Oh yeah, I didn't know how to make the test stable under PyPy so I just skipped it in CI on that branch. I think it must be related to PyPy's unpredictable garbage collector.. Thanks for filing this, anyway!

webknjaz commented 6 months ago

I can reproduce with main (at f156ccd) as well.

I just realized that with dropping of the Python 3.6/3.7 in CI, the PyPy versions that were present in CI at the time weren't re-added for newer Python versions. So we'll need to do this and rethink the test structure.

The test itself is new, it didn't exist in 10.0.0 Β­β€” it was initially added to cover the bugfixes that landed in 10.0.1.