Repository navigation
ci(tests): cut idle waits from the python suite and skip OS-independent tests on windows - #9859
Merged
joshistoast merged 4 commits intoOct 15, 2026
Merged
Conversation
…nt tests on windows Tests hash at bcrypt cost 4, shorten service queue polls, scope the move backoff to scripted failures, and freeze the heap before repeated gc passes. Legacy-module symbol checks share one subprocess; a new platform_independent marker is deselected on windows only. tests/AGENTS.md documents test cost and when the marker applies.
Pfannkuchensack
requested review from
blessedcoolant,
joshistoast and
lstein
as code owners
October 15, 2026 00:39
Reads the source's key frames and trims around each open-GOP I-frame instead of from all 40 starts. Asserts read_h264_layout offers no open-GOP I-frame as a cut point, which the old loop did not catch.
joshistoast
approved these changes
Oct 15, 2026
lstein
approved these changes
Oct 15, 2026
lstein
left a comment
Collaborator
There was a problem hiding this comment.
Approving at d213bf5. I tried to break each mechanism and found no blockers.
Verified
- Pinned to 2 cores (
taskset -c 0,1,-n 2,OMP_NUM_THREADS=1) to approximate the windows runner, the image_index, model_install, download, auth, routers and RAM-budget cache tests passed twice: 1931 passed, 2 skipped each run.test_module_compatibility.pyandtest_video_frame_extract_range.py: 32 passed.--collect-only -m platform_independentcollects 47. - Marker override: the command-line
-mcomes afteraddopts, so linux and macOS still select exactlynot slow. - Password context: nothing imports
pwd_contextby name; the only users arehash_password/verify_password. Nothing callsneeds_update, so no hash is rehashed. Cost-12 hashes from module-scoped fixtures still verify, and the cost-factor guard still reads the real production context. - Move backoff:
_move_with_retriesreads_MOVE_RETRY_DELAY_SECONDSwhen it is called, and no caller passeddelay=. Every failing-movepatch goes through_fake_move; the remaining one (move_then_scan) doesn't fail. platform_independent: torch/bitsandbytes markers match for win32 and linux x86_64, and none of invokeai'ssys.platform/os.namebranches run at import time.gc.freeze(): it freezes uncollected garbage from earlier tests, including caches whose deferred workers are still alive.test_dropped_cache_is_collectable_and_its_worker_exitscompares thread identity, not count, so it is unaffected. Budgets are per-test objects.- Open-GOP test: once
sync_samples == (0,)holds, every start the old loop also covered takes the same full re-encode path, so narrowing the starts loses nothing.
Non-blocking
_fast_service_pollingdocstring: it says the fixture "only removes the idle wait".image_index_default._POLL_SECONDSis also the base of_backoff_seconds()(it starts at 0.02 s instead of 1 s and reaches the 60 s cap after about 13 failures). It is also the generation-idle poll in_wait_for_idle_generation, which now checks the session queue 50 times a second. The outage tests use>=assertions and pass; the docstring should say this.- Workflow
-mfallback:matrix.pytest-markers || 'not slow'duplicates theaddoptsdefault. Ifaddopts -mis ever extended, linux and macOS CI would silently drop the change. Passing-monly when the matrix sets it avoids that. - Folded symbol checks: inside the shared subprocess, the representative-symbol check is mostly implied by
old is new; it still catches a missing symbol. What the old parametrized tests also gave was importing each legacy module first in a fresh interpreter. That gap is small but worth noting in the PR. - Possible follow-up: the four
_fake_movetests whose move fails permanently still take about 2.2 s each. A profile puts that in_safe_rmtree's unconditionalgc.collect()(about 0.37 s per call, 7 calls), not the backoff. That collect only matters on Windows, so restricting it to win32 would remove most of the cost. This is out of scope here.
A woken worker runs a full gc.collect() before taking its item, which late in an xdist session can exceed the 2 s join. Polls every 60 s with a 10 s deadline, so a pass still requires the release listener.
This was referenced Oct 16, 2026
lstein
added a commit
that referenced
this pull request
Oct 17, 2026
…atabase layer (#9893) ## Summary This follows #9859. On current `main`, the files with the most windows CI time were video splice tests, the font service tests and a few duplicate or vacuous checks. Font and video tests now spend most of their time starting worker processes rather than doing work. This PR removes the parts that cost time without adding coverage, plus one import cost that also affects the running app. - **The font worker no longer loads the database layer.** `fonts_worker` is spawned once per font parse and once per variable-font instance. Its docstring says it stays light, but it imported its helpers from `fonts_default`, which pulled in SQLAlchemy and config. - The byte-level helpers now live in `fonts_parsing`, and the error hierarchy and supported extensions in `fonts_common`. - The worker's import time drops from 1.1–1.3 s to ~0.25 s per spawn. This also speeds up font uploads in the app. - `fonts_default` no longer imports fontTools. - A fresh-interpreter guard test asserts the worker loads neither the database layer nor config. - **The SIGINT subprocess test is skipped on windows.** There `os.kill(pid, SIGINT)` is `TerminateProcess`, so the test passed even with `run_node` swallowing `BaseException`. The in-process `test_run_node_propagates_keyboard_interrupt` still catches that on windows. - **The duplicate UMAP smoke test is removed.** `test_projection.py` already runs real seeded UMAP and DBSCAN fits on every platform. `test_numba_jit_is_active` stays. - **Three shape-only VAE tiling sweeps are marked `platform_independent`**, like the sweeps already marked in `test_vae_tiling_scope.py`. - **The event-loop heartbeat test is deterministic.** It used to assert a 50 ms wall-clock threshold against a 100 ms fake delay. It now holds the fake threadpool on an `asyncio.Event` and asserts that the request is still pending. - **Video concat tests do less repeated work.** - The short-remainder test takes its first-clip lengths from the actual keyframe position, as 65–67 instead of 65–69. It asserts that precondition, and it asserts the splice point in the log. - The two repeated-extension tests ran the same three-round chain and are merged into one. It keeps every assertion of both. Measured locally (CPU only, 8 xdist workers), worker time on the files present in both runs: | | before | after | | --- | --- | --- | | whole suite | 1987 s | 1751 s (−12 %) | | windows selection (`not slow and not platform_independent`) | 1460 s | 1281 s | | `test_fonts_service.py` | 73 s | 20 s | | `test_video_concat_splice.py` (alone) | 129 s | 107 s | | `test_umap_import.py` | 27 s | ~0 s | | SIGINT subprocess test (windows) | 24 s | skipped | ## Related Issues / Discussions Follows #9859. ## QA Instructions - The full suite passes with `uv run --no-sync pytest -n 8`: 13755 passed. - The font service and router tests, the video upload limit tests and `routers/test_utilities.py` passed twice in a row (109 passed). - Import measurements: - A fresh-interpreter `-X importtime` of `fonts_worker` drops from 1.1–1.3 s to ~0.25 s. - `invokeai.app.services.shared.database` and fontTools are no longer imported by the worker and by `fonts_default`, respectively. - Mutation checks: - SIGINT test on windows: with `run_node` swallowing `BaseException`, the subprocess test passed and only the in-process test failed. - Heartbeat test: it fails when authentication runs on the loop, and when the fake threadpool returns without waiting. - Short-remainder test: it fails under the combined mutation that also fails the old version (`MIN_ENCODED_FRAMES=1` with the post-join timeline check disabled). With `MIN_ENCODED_FRAMES=1` alone it now fails too, which the old version missed. - Merged repeated-extension test: it fails with `_plan_splice` returning None. Of the old pair, the soundtrack test passed in that case. - `ruff check` and `ruff format --check` are clean on all changed files. - Not run here: the macOS and linux CI runners. The win32 skip condition was checked by reading only. ## Review - An earlier draft left `SUPPORTED_FONT_EXTENSIONS` in `fonts_parsing`, so the API process imported fontTools just for that constant. It now lives in `fonts_common`. - Accepted coverage change: the merged repeated-extension test runs with audio, so a silent concat-into-concat chain is no longer exercised. A single silent join and a concat-produced first clip with audio both still are. - No material findings remain. The video node's remaining cost is its own per-call decode and probe worker spawns, up to five per concat. Reducing that would be a production change and is outside this PR. ## Checklist - [x] _The PR has a short but descriptive title, suitable for a changelog_ - [x] _Meaningful regression coverage added / updated where needed; obsolete tests/code removed_ - [ ] _Persisted-state and API changes include required migrations / compatibility validation_ - [x] _Relevant performance/efficiency opportunities considered; material claims have evidence_ - [x] _Material review findings resolved and relevant checks rerun_ - [ ] _Documentation added / updated (if applicable)_ - [ ] _Updated `What's New` copy (if doing a release after this PR)_ <!-- codesmith:footer --> --- <a href="https://app.blacksmith.sh/invoke-ai/codesmith/InvokeAI/pr/9893?autoLogin=true&ref=codesmith_pr_footer"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v3.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-light-v3.svg"><img alt="View with [code]smith" src="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v3.svg"></picture></a> <a href="https://backend.blacksmith.sh/track/enable-autofix?expires=1794781371&installation_model_id=441795&pr_number=9893&ref=codesmith_pr_footer&repository=invoke-ai%2FInvokeAI&return_to=https%3A%2F%2Fgithub.com%2Finvoke-ai%2FInvokeAI%2Fpull%2F9893&signature=8bd8ae6d68cdb7160e6e4bb7604d0ce8eb2b94ea73039c8910efab026b16d30d"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-light.svg"><img alt="Autofix with [code]smith" src="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark.svg"></picture></a> <sup>Need help on this PR? Tag <code>@codesmith-bot</code> with what you need.</sup> <!-- codesmith:autofix:disabled --> <!-- /codesmith:footer -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The windows python-tests job takes 27–28 minutes and sets the CI wall clock. Most of that time is not test work. Fixtures wait out idle intervals, bcrypt hashes at cost 12, a gc pass walks the whole heap, and a fresh interpreter is started for each of 16 parametrized cases. Tests that cannot behave differently by OS also run three times.
password_utils.pwd_contextfor a cost-4 copy. The cost-factor test and theslowauth benchmarks requestproduction_password_hashing, which restores the production context. Cost-12 hashes stored elsewhere still verify._QUEUE_POLL_SECONDS,_INSTALL_QUEUE_POLL_SECONDS). The root conftest shortens them and the image index's existing_POLL_SECONDS, sostop()in a fixture teardown returns at once instead of after up to 1 s per service. Production values are unchanged._move_with_retriesreads_MOVE_RETRY_DELAY_SECONDS. Only the installer tests that fake a failingmovezero it, through_fake_move. A real Windows lock keeps its full recovery window.gc.freeze()in the RAM-budget cache tests, so their 33gc.collect()calls scan only the test's own objects.platform_independenton 47 tests: import-graph boundaries in a fresh interpreter, static source scans, scheduler visit counts and VAE tile-shape sweeps. Windows installs the linux dependency set and deselects them. macOS runs a different torch and keeps them. Numerics, process-spawning behavior, paths and locking, and dependency smoke tests (umap/numba, PiD chunking) stay unmarked.test_open_gop_source_trim_keeps_every_frameno longer trims from all 40 starts in the last two GOPs. It reads the source's key frames, trims only around each open-GOP I-frame, and asserts thatread_h264_layoutoffers none of them as a cut point. That assertion is new and matters: with the IDR filter removed, the old loop still passed, because the node's join validation re-encoded instead.test_worker_leaves_items_alone_while_its_gpu_is_lent_and_resumes_on_releasealso failed onmain. A woken worker runs a fullgc.collect()before taking its item, and late in an xdist session that can outlast the test's 2 s join. Reproduced locally by inflating the heap: a 2.1 s collection fails it. The test now polls every 60 s with a 10 s deadline, so a pass still requires the release listener.tests/AGENTS.mdnow covers test cost and when the marker applies.Measured locally (CPU only, 8 xdist workers,
OMP_NUM_THREADS=1), worker time on the files present in both runs:not slow and not platform_independent)test_model_install.pytest_image_index_service.pytest_module_compatibility.pytest_multiuser_authorization.pytest_model_cache_ram_budget.pytest_open_gop_source_trim_keeps_every_frame(alone)Related Issues / Discussions
None.
QA Instructions
uv run --no-sync pytest -n 8on the full suite passes. The localHF_ENDPOINTwas pointed at huggingface.co, and the LFS fixtures were pulled.pytest --collect-only -m platform_independentcollects 47 tests.04, thatproduction_password_hashingyields12, and that the cost-4 context verifies a cost-12 hash.test_every_legacy_module_imports_and_aliases_canonical.read_h264_layoutfails the new open-GOP test; the old version passed it.test_video_frame_extract_range.py: 27 passed.ruff checkandruff format --checkare clean on the changed files.mainruns took 27–28 min.gc.collect()it passes. With the release listener disabled it still fails, after the 10 s deadline.Review
Checklist
What's Newcopy (if doing a release after this PR)