Skip to content

Commit

Permalink
Avoid Gunicorn worker test subprocess re-spawning
Browse files Browse the repository at this point in the history
Running subprocesses in tests helps match real-world code usage but also
adds complexity. One of the limitations is that the subprocesses must
exit properly. If they do not, tests may "flake" (fail intermittently)
or there may be more complex problems. One of the problems with the
Gunicorn worker tests is that subprocesses seem to be re-spawning. After
the `coverage run` command completes with a successful exit code and the
test run is concluded, coverage.py keeps generating `.coverage` files.
It's possible these files are coming from unterminated subprocesses,
but this is unlikely because the tests check that the processes have
terminated. Coverage.py offers a configuration setting for handling
`SIGTERM` signals (`sigterm = true` on `coverage.run`), and the tests
were previously exiting with `SIGTERM` (via `process.terminate()`), but
enabling the `sigterm = true` setting has no effect.

To check for these unexpected `.coverage` files, a test will be added to
the GitHub Actions job that runs the tests. The test is simple - count
coverage files in the directory (`find . -name '.coverage*' | wc -l`),
wait for ten seconds (`sleep 10`), then count again. If files continue
to be generated, the GitHub Actions job will error and exit.

To avoid re-spawning subprocesses, this commit will update the Gunicorn
process fixture to exit with `SIGQUIT` instead of `SIGTERM`. While this
seems to avoid the re-spawning subprocesses, it also leads coverage.py
to report incorrect coverage. Coverage.py reports that the ASGI app in
the Gunicorn worker test module `test_gunicorn_workers.py` is uncovered,
when it obviously is covered (otherwise the tests couldn't pass).

To maintain test coverage, this commit will add a pytest fixture with a
simplified worker configuration that exits with `SIGTERM` instead of
`SIGQUIT`. The new fixture will be used in a test that makes a `GET`
request to the worker. This sounds redundant with what was already in
the tests, but adding a redundant fixture and test seems to help
coverage.py report the correct test coverage.

https://coverage.readthedocs.io/en/latest/subprocess.html
  • Loading branch information
br3ndonland committed Jan 10, 2025
1 parent e8019a2 commit c52fd63
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 0 deletions.
15 changes: 15 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,21 @@ jobs:
run: |
export COVERAGE_PROCESS_START="$PWD/pyproject.toml"
hatch run ${{ env.HATCH_ENV }}:coverage run
coverage_files_after_run=$(find . -name '.coverage*' | wc -l)
sleep_time=10
sleep "$sleep_time"
coverage_files_after_sleep=$(find . -name '.coverage*' | wc -l)
echo "[INFO] Verifying that coverage.py has stopped generating .coverage files.
[INFO] Number of coverage files after coverage run: $coverage_files_after_run
[INFO] Number of coverage files after $sleep_time second sleep: $coverage_files_after_sleep
"
if [ "$coverage_files_after_sleep" -gt "$coverage_files_after_run" ]; then
echo "[ERROR] Unexpected .coverage files detected."
exit 1
fi
timeout-minutes: 5
- name: Enforce test coverage
run: |
Expand Down
65 changes: 65 additions & 0 deletions tests/test_gunicorn_workers.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,59 @@ def gunicorn_process(
process.client = client
process.output = output
yield process
process.send_signal(signal.SIGQUIT)
process.wait(timeout=2)


@pytest.fixture
def gunicorn_process_with_sigterm(
unused_tcp_port: int,
) -> Generator[Process, None, None]:
"""Yield a subprocess running a Gunicorn arbiter with a Uvicorn worker.
An instance of `httpx.Client` is available on the `client` attribute.
Output is saved to a temporary file and accessed with `read_output()`.
This pytest fixture provides a simplified worker configuration that exits
with `SIGTERM` instead of `SIGQUIT`. Exiting with `SIGTERM` seems to help
coverage.py report the correct code coverage, even without the configuration
setting `sigterm = true` on `coverage.run`, but test processes also seem to
re-spawn unexpectedly. Coverage.py continues generating `.coverage.*` files,
even after the test run has concluded. This is why `SIGQUIT` is used instead
of `SIGTERM` in the more complex `gunicorn_process` fixture. The idea is to
use `SIGTERM` as little as possible to avoid these re-spawning subprocesses.
"""
worker_class = (
f"{gunicorn_workers.UvicornWorker.__module__}."
f"{gunicorn_workers.UvicornWorker.__name__}"
)
app_module = f"{__name__}:{app.__name__}"
args = [
"gunicorn",
"--bind",
f"127.0.0.1:{unused_tcp_port}",
"--graceful-timeout",
"1",
"--log-level",
"debug",
"--worker-class",
worker_class,
"--workers",
"1",
app_module,
]
bind = f"127.0.0.1:{unused_tcp_port}"
base_url = f"http://{bind}"
verify = False
with (
httpx.Client(base_url=base_url, verify=verify) as client,
tempfile.TemporaryFile() as output,
):
with Process(args, stdout=output, stderr=output) as process:
time.sleep(2)
process.client = client
process.output = output
yield process
process.terminate()
process.wait(timeout=2)

Expand Down Expand Up @@ -312,3 +365,15 @@ def test_uvicorn_worker_get_request(gunicorn_process: Process) -> None:
output_text = gunicorn_process.read_output()
assert response.status_code == 204
assert "inboard.gunicorn_workers", "startup complete" in output_text


def test_uvicorn_worker_get_request_with_sigterm(
gunicorn_process_with_sigterm: Process,
) -> None:
"""Test a GET request to the Gunicorn Uvicorn worker's ASGI app
when `SIGTERM` is used to stop the process instead of `SIGQUIT`.
"""
response = gunicorn_process_with_sigterm.client.get("/")
output_text = gunicorn_process_with_sigterm.read_output()
assert response.status_code == 204
assert "inboard.gunicorn_workers", "startup complete" in output_text

0 comments on commit c52fd63

Please sign in to comment.