Skip to content

Commit fa5639b

Browse files
CopiloteleanorjboydCopilot
authored
Fix pytest CI matrix compatibility and prevent hangs (#26027)
## Summary Fix the Python test matrix so every job runs its requested pytest version, cover supported Python versions against recent and upcoming pytest releases, remove the obsolete global pytest pin, and prevent subprocess failures from hanging CI indefinitely. ## Root cause `pytest-black==0.6.0` uses the deprecated `pytest_collect_file(path, ...)` hook removed by pytest 8.1. The resulting `pytest<8.1` workaround lived in shared test requirements, which were installed after the matrix-selected pytest version and silently downgraded stable and pre-release jobs to pytest 8.0.2. The downgrade also made the manual pytest-cov test reject `--disable-plugin-autoload`. Its subprocess exited, but the named-pipe listener waited indefinitely, causing the multi-hour CI hangs. ## Test matrix On Ubuntu and Windows: - Python 3.10, 3.11, 3.12, 3.13, and 3.14 - Latest pytest 8.4 patch (`pytest==8.4.*`) for approximately one year of compatibility coverage - Pytest pre-release channel to catch upcoming breaking changes This produces 20 jobs. The 8.4 wildcard currently resolves to pytest 8.4.2. ## Changes - Install shared test requirements before matrix-selected pytest. - Remove `pytest<8.1` and `pytest-black` from shared requirements. - Skip the obsolete pytest-black-specific discovery case; pytest-ruff continues to cover plugin-generated test items. - Explicitly install stable pytest in the build workflow. - Use `PYTEST_DISABLE_PLUGIN_AUTOLOAD` for cross-version manual plugin loading. - Bound subprocess and pipe waits and propagate failures promptly. - Add regression tests for subprocess timeout, failure propagation, and pipe timeout. ## Validation - Python 3.10 / pytest 8.4.2: **219 passed, 1 skipped** - Python 3.13 / pytest 9.1.1: **219 passed, 1 skipped** - Ruff checks and `git diff --check` pass. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 08d2506 commit fa5639b

6 files changed

Lines changed: 112 additions & 44 deletions

File tree

.github/workflows/build.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,9 @@ jobs:
197197
- name: Install test requirements
198198
run: python -m pip install --upgrade -r build/test-requirements.txt
199199

200+
- name: Install pytest
201+
run: python -m pip install --upgrade pytest
202+
200203
- name: Run Python unit tests
201204
run: python python_files/tests/run_all.py
202205

.github/workflows/pr-check.yml

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -156,9 +156,9 @@ jobs:
156156
# We're not running CI on macOS for now because it's one less matrix entry to lower the number of runners used,
157157
# macOS runners are expensive, and we assume that Ubuntu is enough to cover the Unix case.
158158
os: [ubuntu-latest, windows-latest]
159-
# Run the tests on the oldest and most recent versions of Python.
160-
python: ['3.10', '3.x', '3.13'] # run for 3 pytest versions, most recent stable, oldest version supported and pre-release
161-
pytest-version: ['pytest', 'pytest@pre-release', 'pytest==6.2.0']
159+
python: ['3.10', '3.11', '3.12', '3.13', '3.14']
160+
# Test approximately one year of pytest releases and upcoming pytest changes.
161+
pytest-version: ['pytest==8.4.*', 'pytest@pre-release']
162162

163163
steps:
164164
- name: Checkout
@@ -172,18 +172,6 @@ jobs:
172172
with:
173173
python-version: ${{ matrix.python }}
174174

175-
- name: Install specific pytest version
176-
if: matrix.pytest-version == 'pytest@pre-release'
177-
run: |
178-
python -m pip install --pre pytest
179-
180-
- name: Install specific pytest version
181-
if: matrix.pytest-version != 'pytest@pre-release'
182-
run: |
183-
python -m pip install "${{ matrix.pytest-version }}"
184-
185-
- name: Install specific pytest version
186-
run: python -m pytest --version
187175
- name: Install base Python requirements
188176
uses: brettcannon/pip-secure-install@92f400e3191171c1858cc0e0d9ac6320173fdb0c # v1.0.0
189177
with:
@@ -193,6 +181,17 @@ jobs:
193181
- name: Install test requirements
194182
run: python -m pip install --upgrade -r build/test-requirements.txt
195183

184+
- name: Install specific pytest version (pre-release)
185+
if: matrix.pytest-version == 'pytest@pre-release'
186+
run: python -m pip install --upgrade --pre pytest
187+
188+
- name: Install specific pytest version
189+
if: matrix.pytest-version != 'pytest@pre-release'
190+
run: python -m pip install --upgrade "${{ matrix.pytest-version }}"
191+
192+
- name: Print pytest version
193+
run: python -m pytest --version
194+
196195
- name: Run Python unit tests
197196
run: python python_files/tests/run_all.py
198197

build/test-requirements.txt

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,6 @@ pylint
77
pycodestyle
88
pydocstyle
99
prospector
10-
# pytest-black 0.6.0 uses the deprecated `path` arg in pytest_collect_file,
11-
# which was removed in pytest 8.1. Pin to <8.1 to maintain compatibility.
12-
pytest<8.1
1310
flask
1411
fastapi
1512
uvicorn
@@ -41,4 +38,3 @@ pytest-describe
4138

4239
# for pytest-ruff related tests
4340
pytest-ruff
44-
pytest-black

python_files/tests/pytestadapter/helpers.py

Lines changed: 39 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
import tempfile
1313
import threading
1414
import uuid
15-
from typing import Any, Dict, List, Optional, Tuple
15+
from typing import Any, Dict, List, Optional, Tuple, Union
1616

1717
if sys.platform == "win32":
1818
from namedpipe import NPopen # pylint: disable=import-error # cspell: disable-line
@@ -28,6 +28,8 @@
2828
TEST_DATA_PATH = pathlib.Path(__file__).parent / ".data"
2929
CONTENT_LENGTH: str = "Content-Length:"
3030
CONTENT_TYPE: str = "Content-Type:"
31+
PIPE_RESULT_TIMEOUT_SECONDS = 10
32+
TEST_SUBPROCESS_TIMEOUT_SECONDS = 300
3133

3234

3335
@contextlib.contextmanager
@@ -242,10 +244,36 @@ def _listen_on_pipe_new(listener, result: List[str], completed: threading.Event)
242244
result.append("".join(all_data))
243245

244246

245-
def _run_test_code(proc_args: List[str], proc_env, proc_cwd: str, completed: threading.Event):
246-
result = subprocess.run(proc_args, env=proc_env, cwd=proc_cwd, check=False)
247-
completed.set()
248-
return result
247+
def _run_test_code(
248+
proc_args: List[str],
249+
proc_env,
250+
proc_cwd: Union[str, os.PathLike[str]],
251+
completed: threading.Event,
252+
) -> subprocess.CompletedProcess:
253+
try:
254+
return subprocess.run(
255+
proc_args,
256+
env=proc_env,
257+
cwd=proc_cwd,
258+
check=False,
259+
timeout=TEST_SUBPROCESS_TIMEOUT_SECONDS,
260+
)
261+
finally:
262+
completed.set()
263+
264+
265+
def _wait_for_pipe_result(
266+
listener_thread: threading.Thread,
267+
process_result: subprocess.CompletedProcess,
268+
result: List[str],
269+
) -> None:
270+
listener_thread.join(timeout=PIPE_RESULT_TIMEOUT_SECONDS)
271+
if listener_thread.is_alive():
272+
if process_result.returncode:
273+
raise subprocess.CalledProcessError(process_result.returncode, process_result.args)
274+
raise TimeoutError("Timed out waiting for the test subprocess pipe result")
275+
if process_result.returncode and not result:
276+
raise subprocess.CalledProcessError(process_result.returncode, process_result.args)
249277

250278

251279
def runner(args: List[str]) -> Optional[List[Dict[str, Any]]]:
@@ -359,18 +387,12 @@ def runner_with_cwd_env(
359387

360388
result = [] # result is a string array to store the data during threading
361389
t1: threading.Thread = threading.Thread(
362-
target=_listen_on_pipe_new, args=(pipe, result, completed)
390+
target=_listen_on_pipe_new, args=(pipe, result, completed), daemon=True
363391
)
364392
t1.start()
365393

366-
t2 = threading.Thread(
367-
target=_run_test_code,
368-
args=(process_args, env, path, completed),
369-
)
370-
t2.start()
371-
372-
t1.join()
373-
t2.join()
394+
process_result = _run_test_code(process_args, env, path, completed)
395+
_wait_for_pipe_result(t1, process_result, result)
374396

375397
return process_data_received(result[0]) if result else None
376398
else: # Unix design
@@ -397,19 +419,12 @@ def runner_with_cwd_env(
397419

398420
result = [] # result is a string array to store the data during threading
399421
t1: threading.Thread = threading.Thread(
400-
target=_listen_on_fifo, args=(pipe_name, result, completed)
422+
target=_listen_on_fifo, args=(pipe_name, result, completed), daemon=True
401423
)
402424
t1.start()
403425

404-
t2: threading.Thread = threading.Thread(
405-
target=_run_test_code,
406-
args=(process_args, env, path, completed),
407-
)
408-
409-
t2.start()
410-
411-
t1.join()
412-
t2.join()
426+
process_result = _run_test_code(process_args, env, path, completed)
427+
_wait_for_pipe_result(t1, process_result, result)
413428

414429
return process_data_received(result[0]) if result else None
415430

python_files/tests/pytestadapter/test_discovery.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -476,10 +476,11 @@ def test_config_sub_folder():
476476
expected_discovery_test_output.ruff_test_expected_output,
477477
"--ruff",
478478
),
479-
(
479+
pytest.param(
480480
"2496-black-formatter",
481481
expected_discovery_test_output.black_formatter_expected_output,
482482
"--black",
483+
marks=pytest.mark.skip(reason="pytest-black does not support pytest 8.1 or newer"),
483484
),
484485
],
485486
)
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
# Copyright (c) Microsoft Corporation. All rights reserved.
2+
# Licensed under the MIT License.
3+
4+
import os
5+
import subprocess
6+
import sys
7+
import threading
8+
9+
import pytest
10+
11+
from . import helpers
12+
13+
14+
def test_run_test_code_times_out(tmp_path, monkeypatch):
15+
monkeypatch.setattr(helpers, "TEST_SUBPROCESS_TIMEOUT_SECONDS", 0.01)
16+
completed = threading.Event()
17+
18+
with pytest.raises(subprocess.TimeoutExpired):
19+
helpers._run_test_code( # noqa: SLF001
20+
[sys.executable, "-c", "import time; time.sleep(1)"],
21+
os.environ.copy(),
22+
str(tmp_path),
23+
completed,
24+
)
25+
26+
assert completed.is_set()
27+
28+
29+
def test_wait_for_pipe_result_surfaces_subprocess_failure():
30+
listener_thread = threading.Thread(target=lambda: None)
31+
listener_thread.start()
32+
process_result = subprocess.CompletedProcess(["pytest"], returncode=2)
33+
34+
with pytest.raises(subprocess.CalledProcessError):
35+
helpers._wait_for_pipe_result( # noqa: SLF001
36+
listener_thread, process_result, []
37+
)
38+
39+
40+
def test_wait_for_pipe_result_times_out(monkeypatch):
41+
monkeypatch.setattr(helpers, "PIPE_RESULT_TIMEOUT_SECONDS", 0.01)
42+
release_listener = threading.Event()
43+
listener_thread = threading.Thread(target=release_listener.wait, daemon=True)
44+
listener_thread.start()
45+
process_result = subprocess.CompletedProcess(["pytest"], returncode=0)
46+
47+
try:
48+
with pytest.raises(TimeoutError, match="Timed out waiting"):
49+
helpers._wait_for_pipe_result( # noqa: SLF001
50+
listener_thread, process_result, []
51+
)
52+
finally:
53+
release_listener.set()
54+
listener_thread.join()

0 commit comments

Comments
 (0)