From 332543d9d786112d4102621560529e56c61ca8cb Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Fri, 23 Oct 2026 09:48:29 -0400 Subject: [PATCH 1/5] TEST: Stabilize CodeQL scenario runner on busy CI workers Run the Node suite without an extra child process and use the shared correctness watchdog instead of a 60-second latency limit. Verify the TAP summary reports that every collected scenario passed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- doc/contributing/4_running_tests.md | 8 ++++++-- tests/unit/build_scripts/test_codeql_merge_group.py | 10 ++++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/doc/contributing/4_running_tests.md b/doc/contributing/4_running_tests.md index 5ae99d1b25..f394e6e12b 100644 --- a/doc/contributing/4_running_tests.md +++ b/doc/contributing/4_running_tests.md @@ -58,14 +58,18 @@ cancellation when a matrix job has already failed. The CodeQL job needs Fork PRs use native CodeQL publishing and GitHub's read-only fork token policy. Run the deterministic, offline regression coverage with Node.js installed. -The pytest entry point runs the Node scenario tests and checks workflow wiring: +The pytest entry point runs the Node scenario tests and checks workflow wiring. +It runs `node:test` in a single Node process, with the shared 300-second correctness +watchdog to allow for startup and filesystem delays on busy Windows runners. +The watchdog is a hang backstop, not a performance requirement; API calls and +polling delays remain mocked. ```bash uv run --frozen pytest tests/unit/build_scripts/test_codeql_merge_group.py -q ``` The helper's scenarios can also run directly with -`node --test tests/unit/build_scripts/test_codeql_merge_group.cjs`. +`node --test-reporter=tap tests/unit/build_scripts/test_codeql_merge_group.cjs`. ## Running a subset while iterating diff --git a/tests/unit/build_scripts/test_codeql_merge_group.py b/tests/unit/build_scripts/test_codeql_merge_group.py index f6be8d6054..45f203f7b4 100644 --- a/tests/unit/build_scripts/test_codeql_merge_group.py +++ b/tests/unit/build_scripts/test_codeql_merge_group.py @@ -4,6 +4,7 @@ from __future__ import annotations +import re import subprocess from pathlib import Path from typing import Any @@ -11,6 +12,7 @@ import pytest import yaml +from unit.async_utils import TEST_WATCHDOG_SECONDS REPO_ROOT = Path(__file__).resolve().parents[3] CODEQL_PIN = "24c54180a607b1449ed407dd24f251e4e9147c8d" @@ -42,15 +44,19 @@ def steps() -> dict[str, Any]: def test_codeql_merge_group_scenarios(steps: dict[str, Any]) -> None: script = steps["Publish merge-group CodeQL results"]["with"]["script"] with patch.dict("os.environ", {"CODEQL_TEST_SCRIPT": script}): + # Run node:test in-process, without --test's extra child process on busy Windows runners. result = subprocess.run( - ["node", "--test", "--test-reporter=tap", str(Path(__file__).with_suffix(".cjs"))], + ["node", "--test-reporter=tap", str(Path(__file__).with_suffix(".cjs"))], cwd=REPO_ROOT, capture_output=True, text=True, check=False, - timeout=60, + timeout=TEST_WATCHDOG_SECONDS, ) assert result.returncode == 0, result.stdout + result.stderr + summary = dict(re.findall(r"^# (tests|pass) (\d+)$", result.stdout, re.MULTILINE)) + assert int(summary.get("tests", "0")) > 0, result.stdout + assert summary.get("pass") == summary["tests"], result.stdout assert "# fail 0" in result.stdout and "# skipped 0" in result.stdout From f7f5a2fb817f487b3873da1c89e0af3d2ff3aa03 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Fri, 23 Oct 2026 10:06:01 -0400 Subject: [PATCH 2/5] TEST: Apply shared watchdog to documentation Node suites Document when correctness subprocesses should use the shared watchdog while preserving intentional performance and timeout assertions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/instructions/test.instructions.md | 7 +++++++ doc/contributing/4_running_tests.md | 4 ++++ tests/unit/build_scripts/test_docs_javascript.py | 7 ++++--- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/.github/instructions/test.instructions.md b/.github/instructions/test.instructions.md index e83b7bb452..8d94c9e7da 100644 --- a/.github/instructions/test.instructions.md +++ b/.github/instructions/test.instructions.md @@ -35,6 +35,13 @@ Most tests should be unit tests. Integration and end-to-end tests are for testin - No network calls should ever happen in unit tests, but file access is okay - Unit tests should be fast and can be run in parallel. +## Correctness Test Timeouts + +- For unit-test subprocesses whose timeout only prevents a hang, use `TEST_WATCHDOG_SECONDS` from `unit.async_utils` rather than a short wall-clock deadline. The shared watchdog allows for startup, filesystem, and scheduling delays on busy CI workers; it is not an expected runtime. +- Preserve intentional performance limits, operation deadlines, timeout/cancellation assertions, and JavaScript VM execution limits. Do not increase them merely to make a test pass. +- For a single `node:test` suite that does not need child-process isolation, run `node --test-reporter=tap ` without `--test` to avoid an extra Node process. Preserve fresh subprocesses where isolation is part of the test contract, such as import guards. +- Still require a successful process exit and, for Node suites, a positive collected-test count with every collected test passing. A longer watchdog must not hide failures, skipped tests, or an empty suite. + ## Integration Test Rules - Mark with `@pytest.mark.run_only_if_all_tests` (skipped unless `RUN_ALL_TESTS=true`) diff --git a/doc/contributing/4_running_tests.md b/doc/contributing/4_running_tests.md index f394e6e12b..4bfa941f29 100644 --- a/doc/contributing/4_running_tests.md +++ b/doc/contributing/4_running_tests.md @@ -71,6 +71,10 @@ uv run --frozen pytest tests/unit/build_scripts/test_codeql_merge_group.py -q The helper's scenarios can also run directly with `node --test-reporter=tap tests/unit/build_scripts/test_codeql_merge_group.cjs`. +The documentation JavaScript suites in `test_docs_javascript.py` also use one +Node process per suite and the shared correctness watchdog. Their per-VM +execution limits remain unchanged. + ## Running a subset while iterating For a narrower run, invoke `pytest` directly. You can invoke pytest if it's in your path or via python; either `pytest` or `python -m pytest`. For the following examples, we will use `pytest`. diff --git a/tests/unit/build_scripts/test_docs_javascript.py b/tests/unit/build_scripts/test_docs_javascript.py index 01ce6a3445..ed9ed1c96e 100644 --- a/tests/unit/build_scripts/test_docs_javascript.py +++ b/tests/unit/build_scripts/test_docs_javascript.py @@ -2,8 +2,8 @@ # Licensed under the MIT license. """Run documentation JavaScript tests with one Node startup per suite. -The Node tests use fresh, time-limited VMs for behavior checks. The process -deadline allows for slow CI startup, not slow JavaScript execution. +The Node tests use fresh, time-limited VMs for behavior checks. The shared +correctness watchdog allows for slow CI startup, not slow JavaScript execution. Local environments without Node skip these tests. """ @@ -13,6 +13,7 @@ from pathlib import Path import pytest +from unit.async_utils import TEST_WATCHDOG_SECONDS @pytest.mark.skipif(shutil.which("node") is None, reason="node not installed") @@ -22,7 +23,7 @@ def test_docs_javascript(suite: str) -> None: ["node", "--test-reporter=tap", str(Path(__file__).with_name(suite))], capture_output=True, text=True, - timeout=60, + timeout=TEST_WATCHDOG_SECONDS, ) assert result.returncode == 0, result.stdout + result.stderr summary = dict(re.findall(r"^# (tests|pass) (\d+)$", result.stdout, re.MULTILINE)) From b107485a2f77c13f1ee50370b852719ef4d12804 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Fri, 23 Oct 2026 11:21:33 -0400 Subject: [PATCH 3/5] TEST: Use shared watchdog for deployment script checks Allow busy Windows runners to finish Bash correctness checks without relaxing simulated readiness deadlines or rollback signal and safety assertions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- doc/contributing/4_running_tests.md | 4 ++++ tests/unit/infra/test_code_deployment.py | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/doc/contributing/4_running_tests.md b/doc/contributing/4_running_tests.md index 4bfa941f29..68fe9ad06e 100644 --- a/doc/contributing/4_running_tests.md +++ b/doc/contributing/4_running_tests.md @@ -75,6 +75,10 @@ The documentation JavaScript suites in `test_docs_javascript.py` also use one Node process per suite and the shared correctness watchdog. Their per-VM execution limits remain unchanged. +The Bash deployment helper in `test_code_deployment.py` uses the same shared +watchdog for subprocess completion. Readiness and rollback tests retain their +simulated operation deadlines, polling assertions, and signal exit-code checks. + ## Running a subset while iterating For a narrower run, invoke `pytest` directly. You can invoke pytest if it's in your path or via python; either `pytest` or `python -m pytest`. For the following examples, we will use `pytest`. diff --git a/tests/unit/infra/test_code_deployment.py b/tests/unit/infra/test_code_deployment.py index 8c7ba492c2..ba1276ccf4 100644 --- a/tests/unit/infra/test_code_deployment.py +++ b/tests/unit/infra/test_code_deployment.py @@ -12,6 +12,8 @@ import unittest from pathlib import Path +from unit.async_utils import TEST_WATCHDOG_SECONDS + REPO_ROOT = Path(__file__).resolve().parents[3] PIPELINES = REPO_ROOT / "infra" / "pipelines" SUBSCRIPTION = "11111111-1111-1111-1111-111111111111" @@ -246,7 +248,7 @@ def _run( text=True, check=False, env=environment, - timeout=30, + timeout=TEST_WATCHDOG_SECONDS, ) assert "Unexpected " not in result.stderr, result.stderr return result From f321b2c87cee3c7948d6d9df1513fc088ae7c650 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Fri, 23 Oct 2026 11:57:32 -0400 Subject: [PATCH 4/5] TEST: Stabilize analytics quick-lane request coordination Use shared readiness and non-cancelling completion watchdogs instead of ten/thirty-second wall-clock limits. Release the reader gate and drain owned requests before restoring mocks. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- doc/contributing/4_running_tests.md | 4 ++++ tests/unit/backend/test_analytics_routes.py | 8 ++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/doc/contributing/4_running_tests.md b/doc/contributing/4_running_tests.md index 68fe9ad06e..62df67eea3 100644 --- a/doc/contributing/4_running_tests.md +++ b/doc/contributing/4_running_tests.md @@ -96,6 +96,10 @@ release any test-controlled waits and join their tasks before removing mocks or disposing fixtures, including when a watchdog expires. When waiting for a task to signal readiness, also watch for that task finishing so an early exception is not hidden by the readiness timeout. +The analytics quick-lane capacity test observes request admission and completion +with these shared watchdogs, then releases its reader gate and drains all requests +before restoring mocks. Its 503/Retry-After and independent-execution assertions +remain unchanged. Use `wait_for_event_async` to observe readiness and `wait_for_completion_async` to observe the outcome, including failures after readiness. Observation timeouts do not cancel owned work. Release gates before calling `settle_tasks_async` in `finally`. diff --git a/tests/unit/backend/test_analytics_routes.py b/tests/unit/backend/test_analytics_routes.py index d46b94f4ed..608082c501 100644 --- a/tests/unit/backend/test_analytics_routes.py +++ b/tests/unit/backend/test_analytics_routes.py @@ -43,7 +43,7 @@ AttackAnalyticsResultsQuery, AttackOutcome, ) -from unit.async_utils import wait_for_completion_async, wait_for_event_async +from unit.async_utils import settle_tasks_async, wait_for_completion_async, wait_for_event_async from unit.memory.test_attack_analytics import make_result @@ -439,18 +439,18 @@ async def blocked_async(**kwargs: object) -> Any: asyncio.create_task(analytics_client.post("/api/analytics/attacks/results", json={})) for _ in range(12) ] try: - await asyncio.wait_for(admitted.wait(), 10) + await wait_for_event_async(event=admitted, tasks=requests) rejected = await analytics_client.post("/api/analytics/attacks/results", json={}) assert rejected.status_code == 503 assert rejected.headers["Retry-After"] == "1" assert analytics_service.has_active_work release.set() - responses = await asyncio.wait_for(asyncio.gather(*requests), 30) + responses = await wait_for_completion_async(future=asyncio.gather(*requests)) assert all(response.status_code == 200 for response in responses) assert read.await_count == 12 finally: release.set() - await asyncio.gather(*requests, return_exceptions=True) + await settle_tasks_async(*requests) @pytest.mark.parametrize( From 850b18147a20f5d16d6bdeed32b5d30c6f11d779 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Fri, 23 Oct 2026 12:19:07 -0400 Subject: [PATCH 5/5] TEST: Use correctness watchdog for seeded converter thread barrier Preserve the two-worker rendezvous and RNG isolation assertions without requiring workers to start within five seconds on loaded CI runners. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/instructions/test.instructions.md | 1 + doc/contributing/4_running_tests.md | 4 ++++ tests/unit/converter/test_seeded_converter_determinism.py | 3 ++- 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/.github/instructions/test.instructions.md b/.github/instructions/test.instructions.md index 8d94c9e7da..26bd1650a4 100644 --- a/.github/instructions/test.instructions.md +++ b/.github/instructions/test.instructions.md @@ -38,6 +38,7 @@ Most tests should be unit tests. Integration and end-to-end tests are for testin ## Correctness Test Timeouts - For unit-test subprocesses whose timeout only prevents a hang, use `TEST_WATCHDOG_SECONDS` from `unit.async_utils` rather than a short wall-clock deadline. The shared watchdog allows for startup, filesystem, and scheduling delays on busy CI workers; it is not an expected runtime. +- Use the same watchdog for thread-barrier rendezvous that test concurrency correctness, not worker startup speed. Keep the barrier and assertions that require distinct workers. - Preserve intentional performance limits, operation deadlines, timeout/cancellation assertions, and JavaScript VM execution limits. Do not increase them merely to make a test pass. - For a single `node:test` suite that does not need child-process isolation, run `node --test-reporter=tap ` without `--test` to avoid an extra Node process. Preserve fresh subprocesses where isolation is part of the test contract, such as import guards. - Still require a successful process exit and, for Node suites, a positive collected-test count with every collected test passing. A longer watchdog must not hide failures, skipped tests, or an empty suite. diff --git a/doc/contributing/4_running_tests.md b/doc/contributing/4_running_tests.md index 62df67eea3..ccf7866f1e 100644 --- a/doc/contributing/4_running_tests.md +++ b/doc/contributing/4_running_tests.md @@ -79,6 +79,10 @@ The Bash deployment helper in `test_code_deployment.py` uses the same shared watchdog for subprocess completion. Readiness and rollback tests retain their simulated operation deadlines, polling assertions, and signal exit-code checks. +The seeded-converter worker-thread test also uses the shared watchdog for its +two-worker barrier. It still requires distinct random generators and identical +seeded draws; worker startup speed is not part of that contract. + ## Running a subset while iterating For a narrower run, invoke `pytest` directly. You can invoke pytest if it's in your path or via python; either `pytest` or `python -m pytest`. For the following examples, we will use `pytest`. diff --git a/tests/unit/converter/test_seeded_converter_determinism.py b/tests/unit/converter/test_seeded_converter_determinism.py index 80952b3e9a..e07309ced0 100644 --- a/tests/unit/converter/test_seeded_converter_determinism.py +++ b/tests/unit/converter/test_seeded_converter_determinism.py @@ -7,6 +7,7 @@ from collections.abc import Callable import pytest +from unit.async_utils import TEST_WATCHDOG_SECONDS from pyrit.common.random_context import configure_random_seed from pyrit.converter import ( @@ -134,7 +135,7 @@ async def convert_async(self, *, prompt: str, input_type: PromptDataType = "text barrier = threading.Barrier(2) def draw() -> tuple[random.Random, float]: - barrier.wait(timeout=5) + barrier.wait(timeout=TEST_WATCHDOG_SECONDS) rng = self._get_random_generator(stream="values") return (rng, rng.random())