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 workers #1995

Closed
wants to merge 20 commits into from
Closed

Test workers #1995

wants to merge 20 commits into from

Conversation

br3ndonland
Copy link
Contributor

Description

The uvicorn.workers module provides worker classes intended for use as Gunicorn workers. The Uvicorn docs say, "For production deployments we recommend using gunicorn with the uvicorn worker class." Other projects suggest similar use cases. For example, Sanic suggests running Gunicorn with the Uvicorn worker (by exposing the Sanic app instance as an ASGI app to Uvicorn).

Although we recommend running workers in production, we don't have tests for this approach. It would be helpful to test the workers if we're recommending them for production deployments. PR #631 added some tests for the Gunicorn worker, but these tests were deemed "flaky" (unclear why) and removed in #650 and #658. Gunicorn doesn't really test its own workers either (benoitc/gunicorn#2742), which gives us even more reason to run tests in this project.

Changes

Worker tests

This PR offers tests for 100% of the Gunicorn worker code in a new module tests/test_workers.py. Test fixtures are also stored in tests/test_workers.py, rather than in tests/conftest.py, to cleanly separate code depending on Gunicorn from the rest of the tests.

A test fixture starts a subprocess running Gunicorn with a Uvicorn worker and an ASGI app. The subprocess includes an instance of httpx.Client for HTTP requests to the Uvicorn worker's ASGI app, and saves its output to a temporary file for assertions on stdout/stderr. Tests can send signals to the process.

Coverage configuration

Previously, uvicorn.workers was completely omitted from coverage measurement due to use of the include setting to specify source files. Uvicorn recently switched the coverage.py config from the include setting to the source_pkgs setting, which shows the expected coverage (#1990 (comment)).

This PR will add tests to cover 100% of the worker code by configuring coverage.py for subprocess test coverage measurement. There is some longstanding awkwardness involved (nedbat/coveragepy#367). Changes in this PR include:

  • Enable the required parallel mode
  • Set the required COVERAGE_PROCESS_START environment variable
  • Add the coverage_enable_subprocess package to invoke coverage.process_startup
  • Combine coverage reports before reporting coverage
  • Update .gitignore to ignore files named .coverage* (many coverage files are generated when subprocesses are measured in parallel mode)

Checklist

  • I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.

Related

Uvicorn

Other projects

- Enable the required parallel mode
- Set the required `COVERAGE_PROCESS_START` environment variable
- Add `coverage_enable_subprocess` for `coverage.process_startup`
- Combine coverage reports before reporting coverage
- Update `.gitignore` to ignore files named `.coverage*`

https://coverage.readthedocs.io/en/latest/subprocess.html
Occasional flakes are seen with some less-common signals.
This commit will add some simple conditionals to avoid `AssertionError`
exceptions when those signals are not handled as expected.
`WatchFilesReload` flakes when used with coverage.py parallel mode.
It may have to do with the threading in the `touch_soon` fixture.

Other WatchGod and Watchfiles flake on Windows specifically. Errors:

```text
================================== FAILURES ===================================
___________ TestBaseReload.test_override_defaults[WatchFilesReload] ___________

self = <test_reload.TestBaseReload object at 0x0000014D4AB94160>
touch_soon = <function touch_soon.<locals>.start at 0x0000014D4C3192D0>

    @pytest.mark.parametrize("reloader_class", [WatchFilesReload, WatchGodReload])
    def test_override_defaults(self, touch_soon) -> None:
        dotted_file = self.reload_path / ".dotted"
        dotted_dir_file = self.reload_path / ".dotted_dir" / "file.txt"
        python_file = self.reload_path / "main.py"

        with as_cwd(self.reload_path):
            config = Config(
                app="tests.test_config:asgi_app",
                reload=True,
                # We need to add *.txt otherwise no regular files will match
                reload_includes=[".*", "*.txt"],
                reload_excludes=["*.py"],
            )
            reloader = self._setup_reloader(config)

            assert self._reload_tester(touch_soon, reloader, dotted_file)
            assert self._reload_tester(touch_soon, reloader, dotted_dir_file)
>           assert not self._reload_tester(touch_soon, reloader, python_file)
E           AssertionError: assert not [WindowsPath('C:/Users/runneradmin/AppData/Local/Temp/pytest-of-runneradmin/pytest-0/reload_directory1/.coverage.fv-az811-309.2664.795750')]
E            +  where [WindowsPath('C:/Users/runneradmin/AppData/Local/Temp/pytest-of-runneradmin/pytest-0/reload_directory1/.coverage.fv-az811-309.2664.795750')] = <bound method TestBaseReload._reload_tester of <test_reload.TestBaseReload object at 0x0000014D4AB94160>>(<function touch_soon.<locals>.start at 0x0000014D4C3192D0>, <uvicorn.supervisors.watchfilesreload.WatchFilesReload object at 0x0000014D4C39DEA0>, WindowsPath('C:/Users/runneradmin/AppData/Local/Temp/pytest-of-runneradmin/pytest-0/reload_directory1/main.py'))
E            +    where <bound method TestBaseReload._reload_tester of <test_reload.TestBaseReload object at 0x0000014D4AB94160>> = <test_reload.TestBaseReload object at 0x0000014D4AB94160>._reload_tester

tests\supervisors\test_reload.py:253: AssertionError
---------------------------- Captured stderr call -----------------------------
INFO:     Will watch for changes in these directories: ['C:\\Users\\runneradmin\\AppData\\Local\\Temp\\pytest-of-runneradmin\\pytest-0\\reload_directory1']
INFO:     Started reloader process [6280] using WatchFiles
------------------------------ Captured log call ------------------------------
INFO     uvicorn.error:config.py:345 Will watch for changes in these directories: ['C:\\Users\\runneradmin\\AppData\\Local\\Temp\\pytest-of-runneradmin\\pytest-0\\reload_directory1']
INFO     uvicorn.error:basereload.py:77 Started reloader process [6280] using WatchFiles
____________ TestBaseReload.test_override_defaults[WatchGodReload] ____________

self = <test_reload.TestBaseReload object at 0x0000014D4AB959F0>
touch_soon = <function touch_soon.<locals>.start at 0x0000014D4C3E77F0>

    @pytest.mark.parametrize("reloader_class", [WatchFilesReload, WatchGodReload])
    def test_override_defaults(self, touch_soon) -> None:
        dotted_file = self.reload_path / ".dotted"
        dotted_dir_file = self.reload_path / ".dotted_dir" / "file.txt"
        python_file = self.reload_path / "main.py"

        with as_cwd(self.reload_path):
            config = Config(
                app="tests.test_config:asgi_app",
                reload=True,
                # We need to add *.txt otherwise no regular files will match
                reload_includes=[".*", "*.txt"],
                reload_excludes=["*.py"],
            )
            reloader = self._setup_reloader(config)

            assert self._reload_tester(touch_soon, reloader, dotted_file)
>           assert self._reload_tester(touch_soon, reloader, dotted_dir_file)

tests\supervisors\test_reload.py:252:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <test_reload.TestBaseReload object at 0x0000014D4AB959F0>
touch_soon = <function touch_soon.<locals>.start at 0x0000014D4C3E77F0>
reloader = <uvicorn.supervisors.watchgodreload.WatchGodReload object at 0x0000014D4C39DAB0>
files = (WindowsPath('C:/Users/runneradmin/AppData/Local/Temp/pytest-of-runneradmin/pytest-0/reload_directory1/.dotted_dir/file.txt'),)
@py_assert2 = [WindowsPath('C:/Users/runneradmin/AppData/Local/Temp/pytest-of-runneradmin/pytest-0/reload_directory1/.coverage.fv-az...s/runneradmin/AppData/Local/Temp/pytest-of-runneradmin/pytest-0/reload_directory1/.coverage.fv-az811-309.4856.060689')]
@py_assert4 = False
@py_format5 = "assert not [WindowsPath('C:/Users/runneradmin/AppData/Local/Temp/pytest-of-runneradmin/pytest-0/reload_directory1/.co...v-az811-309.4856.060689')] = next(<uvicorn.supervisors.watchgodreload.WatchGodReload object at 0x0000014D4C39DAB0>)\n}"

    def _reload_tester(
        self, touch_soon, reloader: BaseReload, *files: Path
    ) -> Optional[List[Path]]:
        reloader.restart()
        if WatchFilesReload is not None and isinstance(reloader, WatchFilesReload):
            touch_soon(*files)
        else:
>           assert not next(reloader)
E           AssertionError: assert not [WindowsPath('C:/Users/runneradmin/AppData/Local/Temp/pytest-of-runneradmin/pytest-0/reload_directory1/.coverage.fv-az...s/runneradmin/AppData/Local/Temp/pytest-of-runneradmin/pytest-0/reload_directory1/.coverage.fv-az811-309.4856.060689')]
E            +  where [WindowsPath('C:/Users/runneradmin/AppData/Local/Temp/pytest-of-runneradmin/pytest-0/reload_directory1/.coverage.fv-az...s/runneradmin/AppData/Local/Temp/pytest-of-runneradmin/pytest-0/reload_directory1/.coverage.fv-az811-309.4856.060689')] = next(<uvicorn.supervisors.watchgodreload.WatchGodReload object at 0x0000014D4C39DAB0>)

tests\supervisors\test_reload.py:63: AssertionError
---------------------------- Captured stderr call -----------------------------
INFO:     Will watch for changes in these directories: ['C:\\Users\\runneradmin\\AppData\\Local\\Temp\\pytest-of-runneradmin\\pytest-0\\reload_directory1']
INFO:     Started reloader process [6280] using WatchGod
------------------------------ Captured log call ------------------------------
INFO     uvicorn.error:config.py:345 Will watch for changes in these directories: ['C:\\Users\\runneradmin\\AppData\\Local\\Temp\\pytest-of-runneradmin\\pytest-0\\reload_directory1']
INFO     uvicorn.error:basereload.py:77 Started reloader process [6280] using WatchGod
```
The GitHub Actions `jobs.<job_id>.strategy.fail-fast` setting controls
how failures are handled. The default of `true` will fail the entire job
when any job in the matrix fails.

With the introduction of platform-specific worker testing, subprocesses,
and operating system signal handling, there is potential for specific
workflow jobs to occasionally fail. In these cases, it can help to set
`fail-fast: false` to ensure that those specific failed jobs can be
identified.

https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions
With the introduction of platform-specific worker testing, subprocesses,
and operating system signal handling, there is potential for specific
workflow jobs to occasionally hang. In these cases, it can help to set
a timeout on the test step to ensure the workflow jobs don't hang until
the default 360 minute (6 hour) timeout.

https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions
- Explain how to skip subprocess tests by using the custom pytest marker
- Explain how to re-run failed GitHub Actions jobs
@br3ndonland
Copy link
Contributor Author

Help wanted

I'm seeing some occasional test flakes with tests/protocols/test_websocket.py::test_connection_lost_before_handshake_complete. This PR is not modifying any of the websocket code, so I'm not sure why these flakes are occurring.

The tests sometimes fail on this line:

assert response is not None

The errors typically look like this (expand).
Run scripts/test
+ [ -z true ]
+ export COVERAGE_PROCESS_START=/home/runner/work/uvicorn/uvicorn/pyproject.toml
+ coverage run --debug config -m pytest
-- config ----------------------------------------------------
         attempted_config_files: .coveragerc
                                 setup.cfg
                                 tox.ini
                                 pyproject.toml
                         branch: False
                   command_line: None
                    concurrency: -none-
                    config_file: /home/runner/work/uvicorn/uvicorn/pyproject.toml
              config_files_read: /home/runner/work/uvicorn/uvicorn/pyproject.toml
                        context: None
                    cover_pylib: False
                      data_file: .coverage
                          debug: config
                     debug_file: None
               disable_warnings: -none-
                dynamic_context: None
                   exclude_also: -none-
                   exclude_list: pragma: no cover
                                 pragma: nocover
                                 if TYPE_CHECKING:
                                 if typing.TYPE_CHECKING:
                                 raise NotImplementedError
                                 py-not-win32
                                 py-linux
                                 py-gte-38
                      extra_css: None
                     fail_under: 98.05
                         format: None
                       html_dir: htmlcov
              html_skip_covered: None
                html_skip_empty: None
                     html_title: Coverage report
                  ignore_errors: False
     include_namespace_packages: False
                    json_output: coverage.json
              json_pretty_print: False
             json_show_contexts: False
                    lcov_output: coverage.lcov
                       parallel: True
            partial_always_list: while (True|1|False|0):
                                 if (True|1|False|0):
                   partial_list: #\s*(pragma|PRAGMA)[:\s]?\s*(no|NO)\s*(branch|BRANCH)
                          paths: {}
                 plugin_options: {'coverage_conditional_plugin': {'omit': {"sys_platform == 'win32'": ['tests/test_workers.py', 'uvicorn/workers.py']}, 'rules': {'py-win32': "sys_platform == 'win32'", 'py-not-win32': "sys_platform != 'win32'", 'py-linux': "sys_platform == 'linux'", 'py-darwin': "sys_platform == 'darwin'", 'py-gte-38': 'sys_version_info >= (3, 8)', 'py-lt-38': 'sys_version_info < (3, 8)', 'py-gte-39': 'sys_version_info < (3, 9)', 'py-lt-39': 'sys_version_info < (3, 9)'}}}
                        plugins: coverage_conditional_plugin
                      precision: 2
                 relative_files: False
                report_contexts: None
                 report_include: None
                    report_omit: None
                    run_include: -none-
                       run_omit: -none-
                  show_contexts: False
                   show_missing: True
                        sigterm: False
                   skip_covered: True
                     skip_empty: False
                           sort: None
                         source: None
                    source_pkgs: uvicorn
                                 tests
                          timid: False
                     xml_output: coverage.xml
              xml_package_depth: 99
-- end -------------------------------------------------------
============================= test session starts ==============================
platform linux -- Python 3.10.11, pytest-7.2.1, pluggy-1.0.0
rootdir: /home/runner/work/uvicorn/uvicorn, configfile: pyproject.toml
plugins: mock-3.10.0, anyio-3.7.0
collected 565 items

tests/test_auto_detection.py ...                                         [  0%]
tests/test_cli.py ............                                           [  2%]
tests/test_config.py ................................................... [ 11%]
.......................................................                  [ 21%]
tests/test_default_headers.py ......                                     [ 22%]
tests/test_lifespan.py ................                                  [ 25%]
tests/test_main.py .........                                             [ 26%]
tests/test_ssl.py ....                                                   [ 27%]
tests/test_subprocess.py ..                                              [ 27%]
tests/test_workers.py ..........................................         [ 35%]
tests/importer/test_importer.py ......                                   [ 36%]
tests/middleware/test_logging.py ..............                          [ 38%]
tests/middleware/test_message_logger.py ..                               [ 39%]
tests/middleware/test_proxy_headers.py ...........                       [ 41%]
tests/middleware/test_wsgi.py ...........                                [ 43%]
tests/protocols/test_http.py ........................................... [ 50%]
...................................................................      [ 62%]
tests/protocols/test_utils.py ......                                     [ 63%]
tests/protocols/test_websocket.py ...................................... [ 70%]
........................................................................ [ 83%]
......FFFF..............................................                 [ 93%]
tests/supervisors/test_multiprocess.py .                                 [ 93%]
tests/supervisors/test_reload.py ...................................     [ 99%]
tests/supervisors/test_signal.py ...                                     [100%]

=================================== FAILURES ===================================
____ test_connection_lost_before_handshake_complete[H11Protocol-WSProtocol] ____

ws_protocol_cls = <class 'uvicorn.protocols.websockets.wsproto_impl.WSProtocol'>
http_protocol_cls = <class 'uvicorn.protocols.http.h11_impl.H11Protocol'>
unused_tcp_port = 39059

    @pytest.mark.anyio
    @pytest.mark.parametrize("ws_protocol_cls", WS_PROTOCOLS)
    @pytest.mark.parametrize("http_protocol_cls", HTTP_PROTOCOLS)
    async def test_connection_lost_before_handshake_complete(
        ws_protocol_cls, http_protocol_cls, unused_tcp_port: int
    ):
        send_accept_task = asyncio.Event()
        disconnect_message = {}

        async def app(scope, receive, send):
            nonlocal disconnect_message
            message = await receive()
            if message["type"] == "websocket.connect":
                await send_accept_task.wait()
            disconnect_message = await receive()

        response: typing.Optional[httpx.Response] = None

        async def websocket_session(uri):
            nonlocal response
            async with httpx.AsyncClient() as client:
                response = await client.get(
                    f"http://127.0.0.1:{unused_tcp_port}",
                    headers={
                        "upgrade": "websocket",
                        "connection": "upgrade",
                        "sec-websocket-version": "13",
                        "sec-websocket-key": "dGhlIHNhbXBsZSBub25jZQ==",
                    },
                )

        config = Config(
            app=app,
            ws=ws_protocol_cls,
            http=http_protocol_cls,
            lifespan="off",
            port=unused_tcp_port,
        )
        async with run_server(config):
            task = asyncio.create_task(
                websocket_session(f"ws://127.0.0.1:{unused_tcp_port}")
            )
            await asyncio.sleep(0.1)
            send_accept_task.set()

        task.cancel()
>       assert response is not None
E       assert None is not None

tests/protocols/test_websocket.py:754: AssertionError
----------------------------- Captured stderr call -----------------------------
INFO:     Started server process [2037]
INFO:     Uvicorn running on http://127.0.0.1:39059 (Press CTRL+C to quit)
INFO:     Shutting down
------------------------------ Captured log call -------------------------------
INFO     uvicorn.error:server.py:76 Started server process [2037]
INFO     uvicorn.error:server.py:219 Uvicorn running on http://127.0.0.1:39059 (Press CTRL+C to quit)
INFO     uvicorn.error:server.py:265 Shutting down
_ test_connection_lost_before_handshake_complete[H11Protocol-WebSocketProtocol] _

ws_protocol_cls = <class 'uvicorn.protocols.websockets.websockets_impl.WebSocketProtocol'>
http_protocol_cls = <class 'uvicorn.protocols.http.h11_impl.H11Protocol'>
unused_tcp_port = 35627

    @pytest.mark.anyio
    @pytest.mark.parametrize("ws_protocol_cls", WS_PROTOCOLS)
    @pytest.mark.parametrize("http_protocol_cls", HTTP_PROTOCOLS)
    async def test_connection_lost_before_handshake_complete(
        ws_protocol_cls, http_protocol_cls, unused_tcp_port: int
    ):
        send_accept_task = asyncio.Event()
        disconnect_message = {}

        async def app(scope, receive, send):
            nonlocal disconnect_message
            message = await receive()
            if message["type"] == "websocket.connect":
                await send_accept_task.wait()
            disconnect_message = await receive()

        response: typing.Optional[httpx.Response] = None

        async def websocket_session(uri):
            nonlocal response
            async with httpx.AsyncClient() as client:
                response = await client.get(
                    f"http://127.0.0.1:{unused_tcp_port}",
                    headers={
                        "upgrade": "websocket",
                        "connection": "upgrade",
                        "sec-websocket-version": "13",
                        "sec-websocket-key": "dGhlIHNhbXBsZSBub25jZQ==",
                    },
                )

        config = Config(
            app=app,
            ws=ws_protocol_cls,
            http=http_protocol_cls,
            lifespan="off",
            port=unused_tcp_port,
        )
        async with run_server(config):
            task = asyncio.create_task(
                websocket_session(f"ws://127.0.0.1:{unused_tcp_port}")
            )
            await asyncio.sleep(0.1)
            send_accept_task.set()

        task.cancel()
>       assert response is not None
E       assert None is not None

tests/protocols/test_websocket.py:754: AssertionError
----------------------------- Captured stderr call -----------------------------
INFO:     Started server process [2037]
INFO:     Uvicorn running on http://127.0.0.1:35627 (Press CTRL+C to quit)
INFO:     Shutting down
------------------------------ Captured log call -------------------------------
INFO     uvicorn.error:server.py:76 Started server process [2037]
INFO     uvicorn.error:server.py:219 Uvicorn running on http://127.0.0.1:35627 (Press CTRL+C to quit)
INFO     uvicorn.error:server.py:265 Shutting down
_ test_connection_lost_before_handshake_complete[HttpToolsProtocol-WSProtocol] _

ws_protocol_cls = <class 'uvicorn.protocols.websockets.wsproto_impl.WSProtocol'>
http_protocol_cls = <class 'uvicorn.protocols.http.httptools_impl.HttpToolsProtocol'>
unused_tcp_port = 56383

    @pytest.mark.anyio
    @pytest.mark.parametrize("ws_protocol_cls", WS_PROTOCOLS)
    @pytest.mark.parametrize("http_protocol_cls", HTTP_PROTOCOLS)
    async def test_connection_lost_before_handshake_complete(
        ws_protocol_cls, http_protocol_cls, unused_tcp_port: int
    ):
        send_accept_task = asyncio.Event()
        disconnect_message = {}

        async def app(scope, receive, send):
            nonlocal disconnect_message
            message = await receive()
            if message["type"] == "websocket.connect":
                await send_accept_task.wait()
            disconnect_message = await receive()

        response: typing.Optional[httpx.Response] = None

        async def websocket_session(uri):
            nonlocal response
            async with httpx.AsyncClient() as client:
                response = await client.get(
                    f"http://127.0.0.1:{unused_tcp_port}",
                    headers={
                        "upgrade": "websocket",
                        "connection": "upgrade",
                        "sec-websocket-version": "13",
                        "sec-websocket-key": "dGhlIHNhbXBsZSBub25jZQ==",
                    },
                )

        config = Config(
            app=app,
            ws=ws_protocol_cls,
            http=http_protocol_cls,
            lifespan="off",
            port=unused_tcp_port,
        )
        async with run_server(config):
            task = asyncio.create_task(
                websocket_session(f"ws://127.0.0.1:{unused_tcp_port}")
            )
            await asyncio.sleep(0.1)
            send_accept_task.set()

        task.cancel()
>       assert response is not None
E       assert None is not None

tests/protocols/test_websocket.py:754: AssertionError
----------------------------- Captured stderr call -----------------------------
INFO:     Started server process [2037]
INFO:     Uvicorn running on http://127.0.0.1:56383 (Press CTRL+C to quit)
INFO:     Shutting down
------------------------------ Captured log call -------------------------------
INFO     uvicorn.error:server.py:76 Started server process [2037]
INFO     uvicorn.error:server.py:219 Uvicorn running on http://127.0.0.1:56383 (Press CTRL+C to quit)
INFO     uvicorn.error:server.py:265 Shutting down
_ test_connection_lost_before_handshake_complete[HttpToolsProtocol-WebSocketProtocol] _

ws_protocol_cls = <class 'uvicorn.protocols.websockets.websockets_impl.WebSocketProtocol'>
http_protocol_cls = <class 'uvicorn.protocols.http.httptools_impl.HttpToolsProtocol'>
unused_tcp_port = 33179

    @pytest.mark.anyio
    @pytest.mark.parametrize("ws_protocol_cls", WS_PROTOCOLS)
    @pytest.mark.parametrize("http_protocol_cls", HTTP_PROTOCOLS)
    async def test_connection_lost_before_handshake_complete(
        ws_protocol_cls, http_protocol_cls, unused_tcp_port: int
    ):
        send_accept_task = asyncio.Event()
        disconnect_message = {}

        async def app(scope, receive, send):
            nonlocal disconnect_message
            message = await receive()
            if message["type"] == "websocket.connect":
                await send_accept_task.wait()
            disconnect_message = await receive()

        response: typing.Optional[httpx.Response] = None

        async def websocket_session(uri):
            nonlocal response
            async with httpx.AsyncClient() as client:
                response = await client.get(
                    f"http://127.0.0.1:{unused_tcp_port}",
                    headers={
                        "upgrade": "websocket",
                        "connection": "upgrade",
                        "sec-websocket-version": "13",
                        "sec-websocket-key": "dGhlIHNhbXBsZSBub25jZQ==",
                    },
                )

        config = Config(
            app=app,
            ws=ws_protocol_cls,
            http=http_protocol_cls,
            lifespan="off",
            port=unused_tcp_port,
        )
        async with run_server(config):
            task = asyncio.create_task(
                websocket_session(f"ws://127.0.0.1:{unused_tcp_port}")
            )
            await asyncio.sleep(0.1)
            send_accept_task.set()

        task.cancel()
>       assert response is not None
E       assert None is not None

tests/protocols/test_websocket.py:754: AssertionError
----------------------------- Captured stderr call -----------------------------
INFO:     Started server process [2037]
INFO:     Uvicorn running on http://127.0.0.1:33179 (Press CTRL+C to quit)
INFO:     Shutting down
------------------------------ Captured log call -------------------------------
INFO     uvicorn.error:server.py:76 Started server process [2037]
INFO     uvicorn.error:server.py:219 Uvicorn running on http://127.0.0.1:33179 (Press CTRL+C to quit)
INFO     uvicorn.error:server.py:265 Shutting down
================== 4 failed, 561 passed in 148.51s (0:02:28) ===================

Could we modify or remove this test?

I could use some help figuring this out.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Help wanted

Some of the reloader tests in tests/test_reload.py have been flaking, sometimes only when using watchfiles, sometimes only on Windows. Not sure why these flakes are occurring, but it could relate to using coverage.py parallel mode with the threading done in the test fixture.

The errors typically look like this (expand).
Run scripts/test
+ [ -z true ]
+ export COVERAGE_PROCESS_START=/home/runner/work/uvicorn/uvicorn/setup.cfg
+ coverage run --debug config -m pytest
-- config ----------------------------------------------------
         attempted_config_files: .coveragerc
                                 setup.cfg
                         branch: False
                   command_line: None
                    concurrency: -none-
                    config_file: /home/runner/work/uvicorn/uvicorn/setup.cfg
              config_files_read: /home/runner/work/uvicorn/uvicorn/setup.cfg
                        context: None
                    cover_pylib: False
                      data_file: .coverage
                          debug: config
                     debug_file: None
               disable_warnings: -none-
                dynamic_context: None
                   exclude_also: -none-
                   exclude_list: pragma: no cover
                                 pragma: nocover
                                 if TYPE_CHECKING:
                                 if typing.TYPE_CHECKING:
                                 raise NotImplementedError
                                 py-not-win32
                                 py-linux
                                 py-gte-38
                      extra_css: None
                     fail_under: 98.8
                         format: None
                       html_dir: htmlcov
              html_skip_covered: None
                html_skip_empty: None
                     html_title: Coverage report
                  ignore_errors: False
     include_namespace_packages: False
                    json_output: coverage.json
              json_pretty_print: False
             json_show_contexts: False
                    lcov_output: coverage.lcov
                       parallel: True
            partial_always_list: while (True|1|False|0):
                                 if (True|1|False|0):
                   partial_list: #\s*(pragma|PRAGMA)[:\s]?\s*(no|NO)\s*(branch|BRANCH)
                          paths: {}
                 plugin_options: {'coverage_conditional_plugin': {'rules': '\n"sys_platform == \'win32\'": py-win32\n"sys_platform != \'win32\'": py-not-win32\n"sys_platform == \'linux\'": py-linux\n"sys_platform == \'darwin\'": py-darwin\n"sys_version_info >= (3, 8)": py-gte-38\n"sys_version_info < (3, 8)": py-lt-38'}}
                        plugins: coverage_conditional_plugin
                      precision: 2
                 relative_files: False
                report_contexts: None
                 report_include: None
                    report_omit: None
                    run_include: -none-
                       run_omit: venv/*
                  show_contexts: False
                   show_missing: True
                        sigterm: True
                   skip_covered: True
                     skip_empty: False
                           sort: None
                         source: None
                    source_pkgs: uvicorn
                                 tests
                          timid: False
                     xml_output: coverage.xml
              xml_package_depth: 99
-- end -------------------------------------------------------
============================= test session starts ==============================
platform linux -- Python 3.10.11, pytest-7.2.1, pluggy-1.0.0
rootdir: /home/runner/work/uvicorn/uvicorn, configfile: setup.cfg
plugins: anyio-3.6.2, mock-3.10.0
collected 484 items

tests/test_auto_detection.py ...                                         [  0%]
tests/test_cli.py ............                                           [  3%]
tests/test_config.py ..................................................  [ 13%]
tests/test_default_headers.py ......                                     [ 14%]
tests/test_lifespan.py ................                                  [ 17%]
tests/test_main.py .........                                             [ 19%]
tests/test_ssl.py ....                                                   [ 20%]
tests/test_subprocess.py ..                                              [ 21%]
tests/test_workers.py ....................                               [ 25%]
tests/importer/test_importer.py ......                                   [ 26%]
tests/middleware/test_logging.py ..............                          [ 29%]
tests/middleware/test_message_logger.py ..                               [ 29%]
tests/middleware/test_proxy_headers.py ...........                       [ 32%]
tests/middleware/test_wsgi.py ...........                                [ 34%]
tests/protocols/test_http.py ........................................... [ 43%]
...................................................................      [ 57%]
tests/protocols/test_utils.py ......                                     [ 58%]
tests/protocols/test_websocket.py ...................................... [ 66%]
........................................................................ [ 80%]
........................................................                 [ 92%]
tests/supervisors/test_multiprocess.py .                                 [ 92%]
tests/supervisors/test_reload.py .............F......F...F..........     [100%]

=================================== FAILURES ===================================
_ TestBaseReload.test_should_not_reload_when_exclude_pattern_match_file_is_changed[WatchFilesReload] _

self = <test_reload.TestBaseReload object at 0x7f7f17c7a110>
touch_soon = <function touch_soon.<locals>.start at 0x7f7f1324e710>

    @pytest.mark.parametrize("reloader_class", [WatchFilesReload, WatchGodReload])
    def test_should_not_reload_when_exclude_pattern_match_file_is_changed(
        self, touch_soon
    ) -> None:
        python_file = self.reload_path / "app" / "src" / "main.py"
        css_file = self.reload_path / "app" / "css" / "main.css"
        js_file = self.reload_path / "app" / "js" / "main.js"

        with as_cwd(self.reload_path):
            config = Config(
                app="tests.test_config:asgi_app",
                reload=True,
                reload_includes=["*"],
                reload_excludes=["*.coverage*", "*.js"],
            )
            reloader = self._setup_reloader(config)

            assert self._reload_tester(touch_soon, reloader, python_file)
>           assert self._reload_tester(touch_soon, reloader, css_file)
E           AssertionError: assert []
E            +  where [] = <bound method TestBaseReload._reload_tester of <test_reload.TestBaseReload object at 0x7f7f17c7a110>>(<function touch_soon.<locals>.start at 0x7f7f1324e710>, <uvicorn.supervisors.watchfilesreload.WatchFilesReload object at 0x7f7f17d410c0>, PosixPath('/tmp/pytest-of-runner/pytest-0/reload_directory1/app/css/main.css'))
E            +    where <bound method TestBaseReload._reload_tester of <test_reload.TestBaseReload object at 0x7f7f17c7a110>> = <test_reload.TestBaseReload object at 0x7f7f17c7a110>._reload_tester

tests/supervisors/test_reload.py:168: AssertionError
----------------------------- Captured stderr call -----------------------------
INFO:     Will watch for changes in these directories: ['/tmp/pytest-of-runner/pytest-0/reload_directory1/.dotted_dir', '/tmp/pytest-of-runner/pytest-0/reload_directory1/app', '/tmp/pytest-of-runner/pytest-0/reload_directory1/app_first', '/tmp/pytest-of-runner/pytest-0/reload_directory1/app_second', '/tmp/pytest-of-runner/pytest-0/reload_directory1/app_third', '/tmp/pytest-of-runner/pytest-0/reload_directory1/ext']
INFO:     Started reloader process [1950] using WatchFiles
------------------------------ Captured log call -------------------------------
INFO     uvicorn.error:config.py:343 Will watch for changes in these directories: ['/tmp/pytest-of-runner/pytest-0/reload_directory1/.dotted_dir', '/tmp/pytest-of-runner/pytest-0/reload_directory1/app', '/tmp/pytest-of-runner/pytest-0/reload_directory1/app_first', '/tmp/pytest-of-runner/pytest-0/reload_directory1/app_second', '/tmp/pytest-of-runner/pytest-0/reload_directory1/app_third', '/tmp/pytest-of-runner/pytest-0/reload_directory1/ext']
INFO     uvicorn.error:basereload.py:77 Started reloader process [1950] using WatchFiles
_ TestBaseReload.test_should_reload_when_directories_have_same_prefix[WatchFilesReload] _

self = <test_reload.TestBaseReload object at 0x7f7f17c7aa10>
touch_soon = <function touch_soon.<locals>.start at 0x7f7f132c11b0>

    @pytest.mark.parametrize(
        "reloader_class", [StatReload, WatchGodReload, WatchFilesReload]
    )
    def test_should_reload_when_directories_have_same_prefix(self, touch_soon) -> None:
        app_dir = self.reload_path / "app"
        app_file = app_dir / "src" / "main.py"
        app_first_dir = self.reload_path / "app_first"
        app_first_file = app_first_dir / "src" / "main.py"

        with as_cwd(self.reload_path):
            config = Config(
                app="tests.test_config:asgi_app",
                reload=True,
                reload_dirs=[str(app_dir), str(app_first_dir)],
            )
            reloader = self._setup_reloader(config)

            assert self._reload_tester(touch_soon, reloader, app_file)
>           assert self._reload_tester(touch_soon, reloader, app_first_file)
E           AssertionError: assert []
E            +  where [] = <bound method TestBaseReload._reload_tester of <test_reload.TestBaseReload object at 0x7f7f17c7aa10>>(<function touch_soon.<locals>.start at 0x7f7f132c11b0>, <uvicorn.supervisors.watchfilesreload.WatchFilesReload object at 0x7f7f17ff9390>, PosixPath('/tmp/pytest-of-runner/pytest-0/reload_directory1/app_first/src/main.py'))
E            +    where <bound method TestBaseReload._reload_tester of <test_reload.TestBaseReload object at 0x7f7f17c7aa10>> = <test_reload.TestBaseReload object at 0x7f7f17c7aa10>._reload_tester

tests/supervisors/test_reload.py:205: AssertionError
----------------------------- Captured stderr call -----------------------------
INFO:     Will watch for changes in these directories: ['/tmp/pytest-of-runner/pytest-0/reload_directory1/app', '/tmp/pytest-of-runner/pytest-0/reload_directory1/app_first']
INFO:     Started reloader process [1950] using WatchFiles
------------------------------ Captured log call -------------------------------
INFO     uvicorn.error:config.py:343 Will watch for changes in these directories: ['/tmp/pytest-of-runner/pytest-0/reload_directory1/app', '/tmp/pytest-of-runner/pytest-0/reload_directory1/app_first']
INFO     uvicorn.error:basereload.py:77 Started reloader process [1950] using WatchFiles
___________ TestBaseReload.test_override_defaults[WatchFilesReload] ____________

self = <test_reload.TestBaseReload object at 0x7f7f17c7b880>
touch_soon = <function touch_soon.<locals>.start at 0x7f7f132c16c0>

    @pytest.mark.parametrize("reloader_class", [WatchFilesReload, WatchGodReload])
    def test_override_defaults(self, touch_soon) -> None:
        dotted_file = self.reload_path / ".dotted"
        dotted_dir_file = self.reload_path / ".dotted_dir" / "file.txt"
        python_file = self.reload_path / "main.py"

        with as_cwd(self.reload_path):
            config = Config(
                app="tests.test_config:asgi_app",
                reload=True,
                # We need to add *.txt otherwise no regular files will match
                reload_includes=[".*", "*.txt"],
                reload_excludes=["*.coverage*", "*.py"],
            )
            reloader = self._setup_reloader(config)

            assert self._reload_tester(touch_soon, reloader, dotted_file)
>           assert self._reload_tester(touch_soon, reloader, dotted_dir_file)
E           AssertionError: assert []
E            +  where [] = <bound method TestBaseReload._reload_tester of <test_reload.TestBaseReload object at 0x7f7f17c7b880>>(<function touch_soon.<locals>.start at 0x7f7f132c16c0>, <uvicorn.supervisors.watchfilesreload.WatchFilesReload object at 0x7f7f17a64b20>, PosixPath('/tmp/pytest-of-runner/pytest-0/reload_directory1/.dotted_dir/file.txt'))
E            +    where <bound method TestBaseReload._reload_tester of <test_reload.TestBaseReload object at 0x7f7f17c7b880>> = <test_reload.TestBaseReload object at 0x7f7f17c7b880>._reload_tester

tests/supervisors/test_reload.py:250: AssertionError
----------------------------- Captured stderr call -----------------------------
INFO:     Will watch for changes in these directories: ['/tmp/pytest-of-runner/pytest-0/reload_directory1']
INFO:     Started reloader process [1950] using WatchFiles
------------------------------ Captured log call -------------------------------
INFO     uvicorn.error:config.py:343 Will watch for changes in these directories: ['/tmp/pytest-of-runner/pytest-0/reload_directory1']
INFO     uvicorn.error:basereload.py:77 Started reloader process [1950] using WatchFiles
=================== 3 failed, 481 passed in 93.14s (0:01:33) ===================

@Kludex
Copy link
Sponsor Member

Kludex commented Jun 4, 2023

This PR caused the flaky behavior: #1804

Revert PR welcome, or solution to the problem... 😅

tests/test_workers.py Outdated Show resolved Hide resolved
tests/test_workers.py Outdated Show resolved Hide resolved
@br3ndonland
Copy link
Contributor Author

This PR caused the flaky behavior: #1804

Revert PR welcome, or solution to the problem... 😅

Thank you for pointing that out. I will take a look into it.

@br3ndonland br3ndonland requested a review from a team June 4, 2023 18:06
.github/workflows/test-suite.yml Outdated Show resolved Hide resolved
docs/contributing.md Outdated Show resolved Hide resolved
To run the test suite without subprocess tests:

```shell
$ scripts/test -m "not subprocess"
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to not test by default, and do the opposite: scripts/test -m "subprocess"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can. The way to disable subprocess tests by default is to add the -m "not subprocess" to addopts in pyproject.toml.

The downsides to this approach are:

  • When we do want to run the subprocess tests, test commands and invocation become more complicated. We would have to add another marker called unmarked and add it to all the other tests (Apply a default marker to all unmarked tests pytest-dev/pytest#8573), then do -m "subprocess or unmarked" to run all the tests. I tried it but pytest is still skipping a lot of tests and I'm having trouble with it.
  • Coverage reports will fail by default, which may confuse people.
  • It doesn't match up as well with the pytest docs on custom markers (the docs recommend the "not subprocess" approach for skipping tests with custom markers).

I prefer the "not subprocess" approach overall.

Copy link
Sponsor Member

@Kludex Kludex Jun 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm inclined about removing the workers.py module from uvicorn.

tests/test_workers.py Show resolved Hide resolved
tests/test_workers.py Show resolved Hide resolved
tests/test_workers.py Show resolved Hide resolved
tests/test_workers.py Outdated Show resolved Hide resolved
tests/test_workers.py Outdated Show resolved Hide resolved
@mbbyn
Copy link

mbbyn commented Jul 27, 2023

If possible, please consider covering the ssl_version parameter as well. Trying to update gunicorn to 21.x we started to see this error:

ai-api-1       | [2023-07-27 11:00:32 +0300] [766] [INFO] Booting worker with pid: 766
ai-api-1       | [2023-07-27 11:00:32 +0300] [760] [ERROR] Exception in worker process
ai-api-1       | Traceback (most recent call last):
ai-api-1       |   File "/opt/venv/lib/python3.10/site-packages/gunicorn/arbiter.py", line 609, in spawn_worker
ai-api-1       |     worker.init_process()
ai-api-1       |   File "/opt/venv/lib/python3.10/site-packages/uvicorn/workers.py", line 66, in init_process
ai-api-1       |     super(UvicornWorker, self).init_process()
ai-api-1       |   File "/opt/venv/lib/python3.10/site-packages/gunicorn/workers/base.py", line 142, in init_process
ai-api-1       |     self.run()
ai-api-1       |   File "/opt/venv/lib/python3.10/site-packages/uvicorn/workers.py", line 98, in run
ai-api-1       |     return asyncio.run(self._serve())
ai-api-1       |   File "/usr/local/lib/python3.10/asyncio/runners.py", line 44, in run
ai-api-1       |     return loop.run_until_complete(main)
ai-api-1       |   File "uvloop/loop.pyx", line 1517, in uvloop.loop.Loop.run_until_complete
ai-api-1       |   File "/opt/venv/lib/python3.10/site-packages/uvicorn/workers.py", line 93, in _serve
ai-api-1       |     await server.serve(sockets=self.sockets)
ai-api-1       |   File "/opt/venv/lib/python3.10/site-packages/uvicorn/server.py", line 68, in serve
ai-api-1       |     config.load()
ai-api-1       |   File "/opt/venv/lib/python3.10/site-packages/uvicorn/config.py", line 430, in load
ai-api-1       |     self.ssl: Optional[ssl.SSLContext] = create_ssl_context(
ai-api-1       |   File "/opt/venv/lib/python3.10/site-packages/uvicorn/config.py", line 119, in create_ssl_context
ai-api-1       |     ctx = ssl.SSLContext(ssl_version)
ai-api-1       |   File "/usr/local/lib/python3.10/ssl.py", line 496, in __new__
ai-api-1       |     self = _SSLContext.__new__(cls, protocol)
ai-api-1       | TypeError: 'str' object cannot be interpreted as an integer

Relevant code:

# prestart.sh

export GUNICORN_CMD_ARGS=" \
    --ssl-version=5 \
    --cert-reqs=2 \
    --ca-certs=/app/cert/[...] \
    --keyfile=/app/cert/[...] \
    --certfile=/app/cert/[...]"
# start.sh

# ... random stuff

# Start Gunicorn
exec python -m gunicorn -k "$WORKER_CLASS" -c "$GUNICORN_CONF" "$APP_MODULE"

@Kludex
Copy link
Sponsor Member

Kludex commented Mar 2, 2024

I'm going to push this code to https://pypi.org/project/uvicorn-worker/ when I have a bit more time, and deprecate the uvicorn worker on uvicorn code source.

@Kludex Kludex mentioned this pull request Mar 21, 2024
4 tasks
@Kludex
Copy link
Sponsor Member

Kludex commented Mar 30, 2024

@br3ndonland I've invited you as a collaborator. I'll be closing this, and deprecating the worker from uvicorn.

@Kludex Kludex closed this Mar 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test Gunicorn Worker
3 participants