diff --git a/.github/workflows/code-quality.yml b/.github/workflows/code-quality.yml index 135bee2..b50684f 100644 --- a/.github/workflows/code-quality.yml +++ b/.github/workflows/code-quality.yml @@ -19,6 +19,9 @@ jobs: security: name: bandit (high severity) runs-on: ubuntu-latest + # ceil(1.5 x slowest of three green main runs) + 3, never under 5 (test.yml): runs 40257217665, + # 40254137380, 40240706583 took 23s / 16s / 23s -> 5 min. + timeout-minutes: 5 steps: - uses: actions/checkout@v7 diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c0f018b..2d032ac 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -19,10 +19,22 @@ concurrency: env: PYTHONFAULTHANDLER: "1" +# Every job has a time limit: ceil(1.5 x the slowest of three recent green runs on main) + 3 +# minutes, never under 5. The 3 minutes leave room for the Python runner's watchdog (120 s, see +# tests/run_suite.py) to print its stack dump before GitHub cancels the job. A limit changes only +# with new measurements written here (TESTING.md "Time limits and hangs"). +# Measured 2026-10-26 on runs 40257217681 (3be0ada), 40240706568 (fb9134f), 40228167328 (f9835aa): +# py matrix, slowest py3.10 on windows-latest: 12m43s / 15m54s / 14m53s -> 27 min +# (ubuntu 5m27s-6m27s, macos 6m14s-8m30s, windows 11m42s-15m54s) +# real ComfyUI compatibility: 13m32s / 8m55s / 11m46s -> 24 min +# coverage gate (85%): 7m26s / 7m22s / 7m54s -> 15 min +# perf budget 30s / 29s / 27s, black and flake8 39s / 37s / 39s, +# frontend modules 23s / 19s / 29s -> 5 min (the floor) jobs: test: name: py${{ matrix.python-version }} on ${{ matrix.os }} runs-on: ${{ matrix.os }} + timeout-minutes: 27 strategy: fail-fast: false matrix: @@ -45,12 +57,12 @@ jobs: pip install -r requirements-dev.txt - name: Run tests - run: python -m unittest discover -s tests -v + run: python -m tests.run_suite -v comfyui: name: real ComfyUI compatibility runs-on: ubuntu-latest - timeout-minutes: 25 + timeout-minutes: 24 steps: - uses: actions/checkout@v7 with: @@ -127,6 +139,7 @@ jobs: coverage: name: coverage gate (85%) runs-on: ubuntu-latest + timeout-minutes: 15 steps: - uses: actions/checkout@v7 @@ -143,7 +156,7 @@ jobs: - name: Measure coverage run: | - python -m coverage run -m unittest discover -s tests + python -m coverage run -m tests.run_suite python -m coverage combine python -m coverage report --fail-under=85 @@ -154,6 +167,7 @@ jobs: name: perf budget if: github.event_name == 'push' || github.event_name == 'workflow_dispatch' runs-on: ubuntu-latest + timeout-minutes: 5 steps: - uses: actions/checkout@v7 @@ -176,6 +190,7 @@ jobs: lint: name: black and flake8 runs-on: ubuntu-latest + timeout-minutes: 5 steps: - uses: actions/checkout@v7 - uses: actions/setup-python@v7 @@ -188,6 +203,7 @@ jobs: js-test: name: frontend modules runs-on: ubuntu-latest + timeout-minutes: 5 steps: - uses: actions/checkout@v7 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index d7efa4e..ca3295d 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -22,7 +22,7 @@ repos: - id: tests name: tests - entry: python -m unittest discover tests/ -v + entry: python -m tests.run_suite -q language: system pass_filenames: false stages: [pre-push] diff --git a/README.md b/README.md index 5ff1860..e59a5e8 100644 --- a/README.md +++ b/README.md @@ -924,12 +924,12 @@ db.model.backup_database("backup_prompts.db") ```bash cd ComfyUI_PromptManager pip install -r requirements-dev.txt -python -m unittest discover -s tests # Python (what CI runs) +python -m tests.run_suite # Python (what CI runs: unittest discovery plus a hang watchdog) node --test tests/js/*.test.js # frontend pure modules -python -m coverage run -m unittest discover -s tests && python -m coverage combine && python -m coverage report +python -m coverage run -m tests.run_suite && python -m coverage combine && python -m coverage report ``` -CI runs the suite on Linux, Windows and macOS for Python 3.10 to 3.12 with an 85% coverage gate. Tests that delete a database file first check that nothing in the process still holds it open (`tests/open_handles.py`), so a connection leak fails on Linux the way it would on Windows. +CI runs the suite on Linux, Windows and macOS for Python 3.10 to 3.13 with an 85% coverage gate. `python -m unittest discover -s tests` still works; see `TESTING.md` ("Time limits and hangs") for the watchdog. Tests that delete a database file first check that nothing in the process still holds it open (`tests/open_handles.py`), so a connection leak fails on Linux the way it would on Windows. ### Code Style diff --git a/TESTING.md b/TESTING.md index da09973..6a0fd43 100644 --- a/TESTING.md +++ b/TESTING.md @@ -31,7 +31,7 @@ its reset endpoint exists only in the test harness. Use a Python environment containing `requirements-dev.txt` and Node.js 24: ```sh -python -m unittest discover -s tests -q +python -m tests.run_suite -q # what CI runs; see "Time limits and hangs" npm ci --prefix tools/vendor --ignore-scripts npx --prefix tools/vendor playwright install --with-deps chromium firefox webkit npm run --prefix tools/vendor test @@ -39,6 +39,9 @@ npm run --prefix tools/vendor test:coverage npm run --prefix tools/vendor test:workflows -- --project firefox --project webkit ``` +`python -m unittest discover -s tests` still works and finds the same tests; +it just has no hang watchdog. + `PM_TEST_PYTHON` selects the Python executable; set it when Python is outside PATH. Every Playwright worker starts its own `tests/browser/server.py --port 0`, which binds a port the OS picks and prints it as `PM_TEST_SERVER_PORT=`, with @@ -204,6 +207,62 @@ not certify every existing feature, arbitrary host versions, GPU/model changes, cross-pack installation or visual design changes. Those still need targeted validation. +## Time limits and hangs + +CI and the pre-push hook run the Python suite as `python -m tests.run_suite` +(`-v`, `-q`, `-s START` and `-p PATTERN` as in `unittest discover`), and the +coverage job as `python -m coverage run -m tests.run_suite`. The runner +(`tests/run_suite.py`) discovers the same tests with the same `sys.path` and +warning filters as `python -m unittest discover -s tests`, exits 0, 1, or 5 when +no tests ran (Python 3.12+ behaviour, on every version), and adds a watchdog: it writes `start: ` to stderr before every test, at every +verbosity, and arms `faulthandler.dump_traceback_later`. If a test, its teardown, +its event-loop shutdown or interpreter exit stops making progress, the stack of +every thread goes to stderr (`Timeout (0:02:00)!` then `Thread 0x...` blocks) and +the process exits non-zero. + +- `PM_TEST_HANG_LIMIT` (default 120 s) bounds each test from its `start:` line to + the next one, including the class and module fixtures (`tearDownClass`, + `setUpModule`, ...) that run in between. `PM_TEST_SHUTDOWN_LIMIT` (default + 120 s) bounds the end of the run: the summary, non-daemon threads, atexit + handlers and coverage's save. Both must be + positive integers; anything else stops the runner with exit 2. 120 s stays below + asyncio's 300 s wait for default-executor threads on Python 3.12 and 3.13 (3.10 + and 3.11 wait without limit), so a hang in an `IsolatedAsyncioTestCase` loop + shutdown is dumped inside the test that hung. +- Reading a dump: the last `start:` line names the test. An `ok` after it means + the test body passed and the hang came after it: in its teardown or event-loop + shutdown (where `IsolatedAsyncioTestCase` joins executor threads), or in a + class or module fixture before the next test; the stack shows which. A dump + after the summary is a hang at interpreter exit. The dump shows thread stacks, not asyncio task + stacks. +- When the watchdog fires under coverage, that run's coverage data is lost; the + job fails anyway and the dump is what matters. + +Every job in `test.yml`, `code-quality.yml` and `viewerjs-check.yml` has +`timeout-minutes` (`tests/vendor/ci-workflow.test.mjs` checks it). The rule is +ceil(1.5 x the slowest of three recent green runs on `main`) + 3 minutes, never +under 5; the three run ids, their times and the resulting limits are written +in a comment above `jobs:`. The 3 minutes leave room for one watchdog firing, so +the dump prints before GitHub cancels the job. A job that hits its limit, or a +watchdog dump, is a failure like any other: read the dump, find the blocking +call, fix it. Never raise a limit or rerun to get green; a limit changes only +with new measurements written next to it. + +### After a push + +Follow the run for the commit you pushed, never an older one: + +```sh +gh run list --commit --workflow test.yml # the run id for that commit +gh run watch --exit-status # returns when the run ends +gh run view --json jobs # shows a job that already failed +gh run view --job --log-failed # that job's failed log, at once +``` + +Start fixing as soon as a failed log shows the cause, but push the fix only after +the run for that commit has finished: pull-request runs use `cancel-in-progress`, +so a push cancels the running jobs and can hide a second failure. + ## Performance budgets A spec performance budget gets two tests: diff --git a/tests/run_suite.py b/tests/run_suite.py new file mode 100644 index 0000000..14a83f8 --- /dev/null +++ b/tests/run_suite.py @@ -0,0 +1,147 @@ +"""Run the Python suite with a hang watchdog: ``python -m tests.run_suite``. + +Why: on PR #349 a macOS job stopped inside the test suite and sat silent until +GitHub cancelled it after six hours, with no stack to read. This runner does +the same discovery as ``python -m unittest discover -s tests`` (same +``sys.path`` when started with ``-m`` from the repo root, same warning +filters, and the exit codes of Python 3.12+ on every version: 0, 1, and 5 +when no tests ran), and adds a watchdog built on +``faulthandler.dump_traceback_later``: when the timer is not re-armed in time, +every thread's stack goes to stderr and the process exits non-zero. It works +the same on Linux, macOS and Windows. + +The timer is armed before discovery, re-armed at the start of every test with +PM_TEST_HANG_LIMIT seconds (default 120), and re-armed when the run ends with +PM_TEST_SHUTDOWN_LIMIT seconds (default 120) to cover interpreter exit +(non-daemon threads, atexit handlers, coverage's save). It is never +cancelled. Class and module fixtures (setUpClass, tearDownModule, ...) run +between two ``start:`` lines, so they count toward the test before them. +120 s stays below asyncio's 300 s wait for default-executor threads on Python +3.12 and 3.13 (3.10 and 3.11 wait without limit), so a hang in an +``IsolatedAsyncioTestCase`` loop shutdown is dumped inside the test that hung. + +Reading a dump: the last ``start: `` line names the test. If its +"ok" line comes after it, the hang is after the test body: its teardown or +event-loop shutdown, or a class/module fixture before the next test (the +stack shows which). A dump after the summary is a hang at interpreter exit +(a non-daemon thread or an atexit handler). + +A limit is a measurement, not a dial: change it only with new measurements +written next to it (TESTING.md "Time limits and hangs"). +""" + +import argparse +import faulthandler +import os +import sys +import unittest +from pathlib import Path + +DEFAULT_PER_TEST_LIMIT = 120 +DEFAULT_SHUTDOWN_LIMIT = 120 +NO_TESTS_EXIT = 5 +USAGE_EXIT = 2 + + +class LimitError(ValueError): + """An environment limit is not a positive integer.""" + + +def read_limit(name, default): + """Return the positive integer in env var ``name``, or ``default``.""" + raw = os.environ.get(name) + if raw is None: + return default + text = raw.strip() + if not (text.isascii() and text.isdigit()) or int(text) <= 0: + raise LimitError(f"run_suite: {name} must be a positive integer, got {raw!r}") + return int(text) + + +def make_result_class(stream, per_test_limit, shutdown_limit): + """Build a TextTestResult that names each test and re-arms the watchdog.""" + + def arm(seconds): + faulthandler.dump_traceback_later(seconds, exit=True, file=stream) + + class WatchdogResult(unittest.TextTestResult): + def startTest(self, test): + # At default verbosity the progress dots share a line; start ours. + prefix = "\n" if self.dots else "" + stream.write(f"{prefix}start: {test.id()}\n") + stream.flush() + super().startTest(test) + arm(per_test_limit) + + def stopTestRun(self): + super().stopTestRun() + arm(shutdown_limit) + + return WatchdogResult + + +def parse_args(argv): + parser = argparse.ArgumentParser( + prog="python -m tests.run_suite", + description="Discover and run the tests with a hang watchdog.", + ) + # As in unittest, -v and -q share one setting and the last one wins. + parser.set_defaults(verbosity=1) + parser.add_argument( + "-v", + "--verbose", + dest="verbosity", + action="store_const", + const=2, + help="verbose output", + ) + parser.add_argument( + "-q", + "--quiet", + dest="verbosity", + action="store_const", + const=0, + help="quiet output", + ) + parser.add_argument( + "-s", + "--start-directory", + default=str(Path(__file__).resolve().parent), + help="directory to start discovery (default: this tests folder)", + ) + parser.add_argument( + "-p", "--pattern", default="test*.py", help="pattern to match test files" + ) + return parser.parse_args(argv) + + +def main(argv=None): + args = parse_args(sys.argv[1:] if argv is None else argv) + # Keep the real stderr: a test may replace sys.stderr, and the watchdog + # needs a stream with a file descriptor. + stream = sys.stderr + try: + per_test_limit = read_limit("PM_TEST_HANG_LIMIT", DEFAULT_PER_TEST_LIMIT) + shutdown_limit = read_limit("PM_TEST_SHUTDOWN_LIMIT", DEFAULT_SHUTDOWN_LIMIT) + except LimitError as error: + stream.write(f"{error}\n") + return USAGE_EXIT + + faulthandler.dump_traceback_later(per_test_limit, exit=True, file=stream) + start = os.path.abspath(args.start_directory) + suite = unittest.TestLoader().discover(start, args.pattern, top_level_dir=start) + runner = unittest.TextTestRunner( + stream=stream, + verbosity=args.verbosity, + # As unittest.main does: show warnings unless -W/PYTHONWARNINGS set them. + warnings=None if sys.warnoptions else "default", + resultclass=make_result_class(stream, per_test_limit, shutdown_limit), + ) + result = runner.run(suite) + if result.testsRun == 0 and not result.skipped: + return NO_TESTS_EXIT + return 0 if result.wasSuccessful() else 1 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/test_execution_history_writer.py b/tests/test_execution_history_writer.py index df1c5b2..ec6327d 100644 --- a/tests/test_execution_history_writer.py +++ b/tests/test_execution_history_writer.py @@ -24,6 +24,13 @@ from utils.execution_history_events import ( ) from utils.execution_history_writer import HistoryWriter +# These calls must return while another thread holds the writer in a step that +# waits up to 3 s (release.wait(3)). 1 s still tells "returned at once" from +# "waited for the release" without failing on a loaded shared runner (#242: +# generous ceilings, never a short wall clock). A 0.1 s ceiling failed at +# 0.19 s on macOS py3.13 (PR #351). +NON_BLOCKING_CEILING = 1.0 + class ManualClock: def __init__(self): @@ -590,10 +597,10 @@ class HistoryWriterTests(unittest.TestCase): before = time.monotonic() self.assertTrue(writer.status().accepting) self.assertTrue(writer.submit(self.finish_command(start))) - self.assertLess(time.monotonic() - before, 0.1) + self.assertLess(time.monotonic() - before, NON_BLOCKING_CEILING) before = time.monotonic() self.assertFalse(writer.close(0)) - self.assertLess(time.monotonic() - before, 0.1) + self.assertLess(time.monotonic() - before, NON_BLOCKING_CEILING) self.assertGreater(writer.context_budget.charged_bytes, 0) self.assertEqual(writer.status().pending, 1) finally: @@ -663,11 +670,11 @@ class HistoryWriterTests(unittest.TestCase): before = time.monotonic() self.assertFalse(writer.submit(self.start_command())) self.assertIsNone(writer.status().epoch) - self.assertLess(time.monotonic() - before, 0.1) + self.assertLess(time.monotonic() - before, NON_BLOCKING_CEILING) self.assertIsNone(self.db.get_execution_run(self.start_command().run_id)) before = time.monotonic() self.assertFalse(writer.close(0.03)) - self.assertLess(time.monotonic() - before, 0.2) + self.assertLess(time.monotonic() - before, NON_BLOCKING_CEILING) release.set() self.wait_for(lambda: not writer.status().alive) self.assertFalse(writer.status().accepting) @@ -958,7 +965,7 @@ class HistoryWriterTests(unittest.TestCase): self.assertTrue(writer.submit(queued)) before = time.monotonic() self.assertFalse(writer.close(0.03)) - self.assertLess(time.monotonic() - before, 0.2) + self.assertLess(time.monotonic() - before, NON_BLOCKING_CEILING) release.set() self.wait_for(lambda: not writer.status().alive) self.assertIsNone(self.db.get_execution_run(queued.run_id)) diff --git a/tests/test_restore_review_fixes.py b/tests/test_restore_review_fixes.py index 4f2f16e..62def53 100644 --- a/tests/test_restore_review_fixes.py +++ b/tests/test_restore_review_fixes.py @@ -291,11 +291,15 @@ class TestCancelledPreview(RestoreTestCase): self.upload(["restored"], "up.db") entered, release = threading.Event(), threading.Event() real = self.api._live_fingerprint + done = threading.Event() def slow(db): entered.set() release.wait(WAIT) - return real(db) + try: + return real(db) + finally: + done.set() with patch.object(self.api, "_live_fingerprint", side_effect=slow): task = asyncio.ensure_future( @@ -310,6 +314,9 @@ class TestCancelledPreview(RestoreTestCase): release.set() with self.assertRaises(asyncio.CancelledError): await task + # The cancel cannot stop the worker thread: it still opens the live + # library. Let it finish, or teardown can find the file open. + self.assertTrue(await loop.run_in_executor(None, done.wait, WAIT)) self.assertEqual(self.stages(), []) self.assertEqual(len(self.api._restore_plans), 0) diff --git a/tests/test_run_suite.py b/tests/test_run_suite.py new file mode 100644 index 0000000..7ce5077 --- /dev/null +++ b/tests/test_run_suite.py @@ -0,0 +1,261 @@ +"""Tests for tests/run_suite.py, the CI test runner and its hang watchdog (#350). + +Each test writes a temporary folder of tiny test modules and runs +``python -m tests.run_suite -s -v`` in a subprocess from the repo +root. Every subprocess call has a timeout, so a broken watchdog fails this +file instead of hanging the suite. Probes that should hang wait a bounded +time, so a watchdog that never fires shows up as a plain assertion failure. +""" + +import os +import subprocess +import sys +import tempfile +import textwrap +import unittest +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent +SUBPROCESS_TIMEOUT = 60 +WATCHDOG_ENV = ("PM_TEST_HANG_LIMIT", "PM_TEST_SHUTDOWN_LIMIT") + + +def run_suite(folder, extra_env=None, *args, verbosity=("-v",)): + """Run the runner on ``folder`` and return the completed process.""" + env = {k: v for k, v in os.environ.items() if k not in WATCHDOG_ENV} + # Windows writes a piped stderr in the ANSI codepage; ask for the UTF-8 + # this helper decodes, so non-ASCII values and paths survive. + env["PYTHONIOENCODING"] = "utf-8" + env.update(extra_env or {}) + command = [sys.executable, "-m", "tests.run_suite", "-s", str(folder)] + return subprocess.run( + [*command, *verbosity, *args], + cwd=REPO_ROOT, + env=env, + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + timeout=SUBPROCESS_TIMEOUT, + ) + + +class RunSuiteTests(unittest.TestCase): + def setUp(self): + self._tmp = tempfile.TemporaryDirectory(prefix="pm-run-suite-") + self.addCleanup(self._tmp.cleanup) + self.folder = Path(self._tmp.name) + + def write(self, name, source): + (self.folder / name).write_text(textwrap.dedent(source), encoding="utf-8") + + def assertDumped(self, result, test_id): + detail = result.stderr[-4000:] + self.assertNotEqual(result.returncode, 0, detail) + self.assertIn(f"start: {test_id}", result.stderr, detail) + self.assertIn("Timeout (", result.stderr, detail) + self.assertIn("Thread 0x", result.stderr, detail) + + def test_passing_test_exits_zero_and_names_each_start(self): + self.write( + "test_probe_pass.py", + """ + import unittest + + class Probe(unittest.TestCase): + def test_ok(self): + pass + """, + ) + result = run_suite(self.folder) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("start: test_probe_pass.Probe.test_ok", result.stderr) + + def test_start_line_is_written_at_quiet_verbosity_too(self): + self.write( + "test_probe_quiet.py", + """ + import unittest + + class Probe(unittest.TestCase): + def test_ok(self): + pass + """, + ) + result = run_suite(self.folder, None, "-q") + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("start: test_probe_quiet.Probe.test_ok", result.stderr) + + def test_start_line_begins_its_own_line_at_default_verbosity(self): + self.write( + "test_probe_dots.py", + """ + import unittest + + class Probe(unittest.TestCase): + def test_a(self): + pass + + def test_b(self): + pass + """, + ) + result = run_suite(self.folder, verbosity=()) + self.assertEqual(result.returncode, 0, result.stderr) + lines = result.stderr.splitlines() + for name in ("test_a", "test_b"): + self.assertIn(f"start: test_probe_dots.Probe.{name}", lines) + + def test_warnings_are_shown_as_with_unittest_discover(self): + self.write( + "test_probe_warn.py", + """ + import unittest + import warnings + + class Probe(unittest.TestCase): + def test_warns(self): + warnings.warn("probe-deprecated", DeprecationWarning) + """, + ) + env = {"PYTHONWARNINGS": ""} + result = run_suite(self.folder, env) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("probe-deprecated", result.stderr) + + def test_failing_test_exits_one(self): + self.write( + "test_probe_fail.py", + """ + import unittest + + class Probe(unittest.TestCase): + def test_fails(self): + self.fail("expected") + """, + ) + result = run_suite(self.folder) + self.assertEqual(result.returncode, 1, result.stderr) + + def test_pattern_option_selects_modules(self): + self.write( + "check_probe.py", + """ + import unittest + + class Probe(unittest.TestCase): + def test_fails(self): + self.fail("selected by -p") + """, + ) + self.assertEqual(run_suite(self.folder).returncode, 5) + result = run_suite(self.folder, None, "-p", "check_*.py") + self.assertEqual(result.returncode, 1, result.stderr) + + def test_empty_folder_exits_five(self): + result = run_suite(self.folder) + self.assertEqual(result.returncode, 5, result.stderr) + + def test_repo_root_modules_import_as_with_unittest_discover(self): + self.write( + "test_probe_import.py", + """ + import unittest + + from utils.hashing import generate_prompt_hash + + class Probe(unittest.TestCase): + def test_import(self): + same = generate_prompt_hash(" A ") == generate_prompt_hash("a") + self.assertTrue(same) + """, + ) + result = run_suite(self.folder) + self.assertEqual(result.returncode, 0, result.stderr) + + def test_hung_test_dumps_every_thread_and_exits(self): + self.write( + "test_probe_hang.py", + """ + import time + import unittest + + class Probe(unittest.TestCase): + def test_hangs(self): + time.sleep(30) + """, + ) + result = run_suite(self.folder, {"PM_TEST_HANG_LIMIT": "2"}) + self.assertDumped(result, "test_probe_hang.Probe.test_hangs") + + def test_hang_in_async_teardown_dumps_and_exits(self): + self.write( + "test_probe_teardown.py", + """ + import threading + import unittest + + class Probe(unittest.IsolatedAsyncioTestCase): + async def asyncTearDown(self): + threading.Event().wait(30) + + async def test_body_passes(self): + pass + """, + ) + result = run_suite(self.folder, {"PM_TEST_HANG_LIMIT": "2"}) + self.assertDumped(result, "test_probe_teardown.Probe.test_body_passes") + + def test_hang_in_event_loop_shutdown_after_ok_dumps_and_exits(self): + # The #349 shape: the test passes ("ok" is printed), then the loop's + # shutdown waits for a default-executor thread that never finishes. + self.write( + "test_probe_loop.py", + """ + import asyncio + import threading + import unittest + + class Probe(unittest.IsolatedAsyncioTestCase): + async def test_leaves_executor_busy(self): + loop = asyncio.get_running_loop() + loop.run_in_executor(None, threading.Event().wait, 30) + """, + ) + result = run_suite(self.folder, {"PM_TEST_HANG_LIMIT": "2"}) + self.assertDumped(result, "test_probe_loop.Probe.test_leaves_executor_busy") + start = result.stderr.index("start: test_probe_loop") + self.assertIn("ok", result.stderr[start:], result.stderr[-4000:]) + + def test_hang_at_interpreter_exit_dumps_and_exits(self): + self.write( + "test_probe_exit.py", + """ + import threading + import unittest + + class Probe(unittest.TestCase): + def test_leaves_thread(self): + threading.Thread(target=threading.Event().wait, args=(30,)).start() + """, + ) + result = run_suite(self.folder, {"PM_TEST_SHUTDOWN_LIMIT": "2"}) + detail = result.stderr[-4000:] + self.assertNotEqual(result.returncode, 0, detail) + self.assertIn("ok", result.stderr, detail) + self.assertIn("Timeout (", result.stderr, detail) + + def test_invalid_limit_is_a_usage_error(self): + for name in WATCHDOG_ENV: + for value in ("abc", "0", "-3", "1.5", "\u00b2"): + with self.subTest(name=name, value=value): + result = run_suite(self.folder, {name: value}) + self.assertEqual(result.returncode, 2, result.stderr) + lines = result.stderr.strip().splitlines() + self.assertEqual(len(lines), 1, result.stderr) + self.assertIn(name, lines[0]) + self.assertIn(repr(value), lines[0]) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/vendor/ci-workflow.test.mjs b/tests/vendor/ci-workflow.test.mjs index b2d8064..f489fd6 100644 --- a/tests/vendor/ci-workflow.test.mjs +++ b/tests/vendor/ci-workflow.test.mjs @@ -1,5 +1,6 @@ -// Structural checks on .github/workflows/viewerjs-check.yml. The tooling has no YAML parser, so these -// read the workflow's fixed two-space layout: jobs at two spaces, their keys at four, steps at six. +// Structural checks on .github/workflows/viewerjs-check.yml, plus the time limits of every CI workflow. +// The tooling has no YAML parser, so these read the workflow's fixed two-space layout: jobs at two +// spaces, their keys at four, steps at six. import test from 'node:test'; import assert from 'node:assert/strict'; import { execFileSync } from 'node:child_process'; @@ -221,3 +222,28 @@ test('browser job timeouts keep a real margin over measured runs', () => { assert.ok(minutes >= 20, `${id} allows at least 20 minutes, since runner speed varies`); } }); + +// Every job is bounded (#350): a hang ends at a measured limit instead of GitHub's 6-hour default. +const BOUNDED_WORKFLOWS = ['test.yml', 'code-quality.yml', 'viewerjs-check.yml']; +const readWorkflow = name => readFileSync(new URL(`../../.github/workflows/${name}`, import.meta.url), 'utf8'); + +test('every job in the CI workflows has a numeric timeout-minutes', () => { + for (const name of BOUNDED_WORKFLOWS) { + const found = jobs(readWorkflow(name)); + assert.ok(found.size > 0, `${name} has jobs`); + for (const [id, body] of found) { + const limit = /^ {4}timeout-minutes: (\d+)\s*$/m.exec(body); + assert.ok(limit, `${name} job ${id} sets a numeric timeout-minutes`); + assert.ok(Number(limit[1]) >= 5, `${name} job ${id} allows at least 5 minutes`); + } + } +}); + +test('the Python matrix and coverage run the suite through the watchdog runner', () => { + const found = jobs(readWorkflow('test.yml')); + assert.match(found.get('test'), /^ {8}run: python -m tests\.run_suite -v$/m); + assert.match(found.get('coverage'), /^ {10}python -m coverage run -m tests\.run_suite$/m); + for (const id of ['test', 'coverage']) { + assert.doesNotMatch(found.get(id), /unittest discover/, `${id} no longer runs the bare unittest discover`); + } +});