test: split MEDIUM-effort server test files into 26 per-concern files - #4658
Merged
Trecek merged 22 commits intoAug 17, 2026
Merged
Conversation
Split the 883-line test_pipeline_tracker.py into per-concern files mirroring
the existing test_record_pipeline_step_complete.py split pattern:
- test_record_pipeline_step.py (~290 LoC): TestRecordPipelineStepInit,
TestRecordPipelineStepGateClosed, TestRecordPipelineStepStatus
- test_complete_run_skill_result.py (~145 LoC): TestCompleteRunSkillResult,
TestGetPipelineReportIncludesTrackerGaps
- test_open_kitchen_auto_init_tracker.py (~228 LoC): module helper
_configure_open_kitchen_mock + TestOpenKitchenAutoInitTracker
- test_pipeline_tracker_authority.py (~217 LoC): TestCheckPipelineDepsImmutableTarget,
TestSelectTrackerAuthority, TestRestoreReservedTrackerAuthority,
TestSelectTrackerTarget
Mechanical lift: no production code changes, no behavior changes, no
test assertion changes. Each file carries pytestmark = [layer("server"),
small]. Class-level _setup autouse fixtures preserved verbatim where the
class-level setup exceeds what tests/server/conftest.py does.
Refs: #4610
Split the 872-line test_tools_kitchen_visibility.py by concern
(Group C integration / Group H sous-chef / Group I redisable unit /
T-VIS-003/004/005 feature gates) into:
- test_tools_kitchen_visibility.py (kept, trimmed ~411 LoC): T-VISIBILITY-1a/1b/2/2b/3a/3
Group C integration tests + 3 Group I _redisable_subsets unit tests
- test_tools_kitchen_recipe_admission.py (~244 LoC): 7 functions covering the
open_kitchen(name=...) admission control flow
- test_tools_kitchen_feature_gates.py (~109 LoC): T-VIS-003/004/005 subset
re-disable integration tests
- test_tools_kitchen_sous_chef.py (~158 LoC): 4 Group H sous-chef discipline
injection tests
Mechanical lift: no production code changes, no behavior changes, no test
assertion changes. Each file carries pytestmark = [layer("server"), small].
Group markers (Group C / H / I) redistributed into per-concern files.
Refs: #4610
Split the 810-line test_tools_execution_input_gates.py (12 test classes plus
9 module-level helpers) into per-concern files plus a shared helper module:
- _input_contract_test_helpers.py (new, ~50 LoC): centralized helpers consumed
by 3 of the 6 split files:
_FixedUUID, _DETERMINISTIC_HEX, _DETERMINISTIC_MARKER, _patch_uuid4,
_make_input_contract_resolver, _collect_all_path_input_specs,
_collect_file_path_list_specs, _ALL_PATH_INPUT_SPECS, _FILE_PATH_LIST_SPECS
- test_tools_execution_input_gates_dry_walkthrough.py (~190 LoC):
TestCheckDryWalkthrough + TestDryWalkthroughGateWithPrefix
- test_tools_execution_input_gates_command.py (~135 LoC):
TestRunSkillPrefix + TestValidateSkillCommand
- test_tools_execution_input_gates_contracts.py (~140 LoC):
TestInputContractValidation + TestInputContractResolver +
TestInputContractIntegration (uses shared helpers)
- test_tools_execution_input_gates_real_contracts.py (~145 LoC):
TestInputContractRealContracts + TestFilePathListRealResolver (uses shared
_ALL_PATH_INPUT_SPECS / _FILE_PATH_LIST_SPECS)
- test_tools_execution_input_gates_cross_binding.py (~95 LoC):
TestPreparePrCrossBinding + TestOpenIntegrationPrExactShape
- test_tools_execution_input_gates_cwd.py (~50 LoC):
TestRunSkillCwdValidation (uses shared _patch_uuid4 / _DETERMINISTIC_MARKER)
Mechanical lift: no production code changes, no behavior changes, no test
assertion changes. Each file carries pytestmark = [layer("server"), small].
The shared helper module centralizes the YAML-loaded contract-spec tables
(loaded once at import time rather than per test class) and the deterministic
UUID fixtures used by the CWD and integration tests.
Refs: #4610
Split the 799-line test_tools_load_recipe.py (11 classes + 1 module-level fn
plus 1 module-level helper and 1 module-level constant) into:
- test_tools_load_recipe.py (kept, trimmed ~270 LoC): core load_recipe tests
(skill-name rejection, kitchen-gated loading, diagram response, ingredients_only,
no-context handling) + module-level _MINIMAL_RECIPE_FOR_DIAGRAM constant
- test_load_recipe_contract_validation.py (~95 LoC):
TestContractMigrationAdapterValidate, TestLoadRecipeSurfacesValidationFailure
- test_load_recipe_exception_handling.py (~190 LoC):
TestLoadRecipeExceptionHandling, TestLoadRecipeFailClosed
- test_load_recipe_migration.py (~150 LoC):
TestMigrationSuppression, TestApplyTriageGate
- test_load_recipe_authority.py (~125 LoC):
TestLoadRecipeReadOnly, TestLoadRecipeAuthorityClobber
The 4-line autouse fixture is copied verbatim into each per-concern file
(per CLAUDE.md Do Not Over-Engineer — extracting one 2-line assignment body
to a helper module plus an import-in-each-file wrapper would add indirection
without simplification).
Mechanical lift: no production code changes, no behavior changes, no test
assertion changes. Each file carries pytestmark = [layer("server"), small]
and the autouse fixture _default_recipe_names_do_not_resolve_as_skills.
Refs: #4610
Split the 776-line test_tools_ci.py (33 module-level test functions + 2
classes: TestWaitForCiTiming, TestWaitForMergeQueueTiming) into:
- test_tools_ci_gate.py (~80 LoC): 6 functions covering GATED_TOOLS / UNGATED_TOOLS
membership and gate-closure behavior for wait_for_ci / get_ci_status /
wait_for_merge_queue
- test_tools_ci_wait_for_ci.py (~360 LoC): 15 functions + class TestWaitForCiTiming
covering the wait_for_ci tool surface (success/failure, head_sha inference,
event/lookback forwarding, remote_url, exception handling, timing)
- test_tools_ci_get_ci_status.py (~50 LoC): 3 functions covering the get_ci_status
tool surface (missing branch/run_id, no watcher, workflow propagation)
- test_tools_ci_wait_for_merge_queue.py (~280 LoC): 9 functions + class
TestWaitForMergeQueueTiming covering the wait_for_merge_queue tool surface
(delegation, repo inference, remote_url, exception handling,
PRState round-trip exhaustiveness, timing)
Per-tool naming follows the established test_tools_<tool>.py convention.
Coexistence with test_tools_ci_enqueue.py and test_tools_ci_merge_state.py is
unaffected — those files cover different production tools (enqueue_pr,
check_repo_merge_state).
The actual split is disjoint and complete (33 functions total). The plan
estimate of 26 was a count discrepancy; the plan said ~15 for wait_for_ci
which matches the actual count.
Mechanical lift: no production code changes, no behavior changes, no test
assertion changes. Each file carries pytestmark = [layer("server"), small].
Refs: #4610
Split the 994-line test_session_type_visibility.py (4 classes:
TestSessionTypeVisibility, TestFeatureGateVisibility,
TestExplorerBindingVisibility, TestEvidenceReaderBindingVisibility) into
per-concern files plus an arch-test registration update:
- test_session_type_visibility_fleet.py: 11 functions covering fleet /
FLEET_DISPATCH integration + 5 regression guards (orchestrator/skill/no-type
hides_fleet_tools). Carries pytest.mark.feature("fleet") in pytestmark.
- test_session_type_visibility_orchestrator.py: 15 functions covering
orchestrator / skill / food-truck / cook / transitional-bridge session
types. No feature marker (none of its tests are fleet-specific).
- test_session_type_feature_gates.py: 3 functions covering the
_apply_session_type_visibility sole-calling-convention contract and fleet
tag activation. Carries pytest.mark.feature("fleet") in pytestmark. The
redundant class-level _reset_mcp_visibility fixture is dropped (the
directory conftest _reset_mcp_tags already covers it).
- test_session_type_explorer_binding.py: 4 functions covering terminal
explorer (allowlist) visibility. The redundant class-level
_reset_mcp_visibility fixture is dropped.
- test_session_type_evidence_reader.py: TestEvidenceReaderBindingVisibility
class with 6 methods + staticmethod _set_complete_identity. No class-level
autouse fixture (mirrors the source).
Cross-cutting updates:
- tests/arch/test_feature_markers.py: removed the obsolete
server/test_session_type_visibility.py entry from _FLEET_CLASS_MARKERS
(the file no longer exists), and added the two new fleet files
(test_session_type_visibility_fleet.py, test_session_type_feature_gates.py)
to _FLEET_CROSS_DIR_FILES — they have pytest.mark.feature("fleet") in
pytestmark, which _pytestmark_has_feature() validates.
Mechanical lift: no production code changes, no behavior changes, no test
assertion changes. Each file carries
pytestmark = [layer("server"), medium] (with feature("fleet") for the two
fleet files).
Refs: #4610
Replace references to the four deleted source files (test_tools_ci.py, test_tools_execution_input_gates.py, test_pipeline_tracker.py, test_session_type_visibility.py) with their new per-concern filenames in tests/_test_filter.py. The two kept files (test_tools_kitchen_visibility.py, test_tools_load_recipe.py) remain referenced as-is since they were only trimmed, not renamed. Refs: #4610
Two cascade-map guards flagged stale/missing entries caused by the file splits in commit 8034585 and earlier: TestFileLevelCascadeDriftGuard::test_no_importing_test_file_missing_from_file_level_entries flagged 5 files that DO import autoskillit.recipe (or workspace) but were not registered: - test_load_recipe_contract_validation.py (recipe._api_cache) - test_load_recipe_exception_handling.py (recipe.schema) - test_load_recipe_migration.py (recipe.staleness_cache) - test_tools_kitchen_sous_chef.py (recipe._recipe_ingredients, workspace.SkillsDirectoryProvider) TestFileLevelCascadeImportGuard::test_file_level_entries_import_their_cascade_package flagged 15 stale entries for files that do NOT actually import autoskillit.recipe or workspace at AST level (per-concern files that share import proximity but no direct import, plus the kept trimmed test_tools_kitchen_visibility.py which drops every recipe/workspace import in favor of inline imports in test methods). Update: - recipe cascade: drop the 13 stale per-concern + test_tools_kitchen_visibility entries; add the 4 files that DO import recipe - workspace cascade: drop test_tools_kitchen_visibility entry; add test_tools_kitchen_sous_chef (the only per-concern file that imports workspace.SkillsDirectoryProvider) Refs: #4610
Cascade-map guards round 2:
- Drift guard: 3 more files DO import autoskillit.recipe (or transitively
through _input_contract_test_helpers) but were not registered. Add:
- test_open_kitchen_auto_init_tracker.py
(recipe.schema.RecipeStep in method bodies)
- test_tools_execution_input_gates_cross_binding.py
(recipe._contracts_manifest.resolve_input_specs)
- test_tools_execution_input_gates_real_contracts.py
(transitively via _input_contract_test_helpers,
which loads recipe._contracts_manifest at module import)
- Import guard: 2 stale entries to remove:
- test_recipe_namespace_admission.py (imports workspace, NOT recipe)
- test_tools_load_recipe.py (kept version imports
autoskillit.server.tools.tools_recipe, NOT autoskillit.recipe)
Refs: #4610
The MEDIUM-effort test file split removed test_tools_kitchen_visibility.py from the LAYER_CASCADE_CONSERVATIVE workspace entry without re-adding it. Two split successors (test_load_recipe_authority.py, test_tools_execution_input_gates_contracts.py) were also missing from the recipe cascade. Both reference autoskillit.recipe only via patch strings or helper-deferred imports, so the import-guard AST scan needs an exemption (same rationale as the existing server/test_tools_load_recipe.py entry). Addresses review-pr findings #1, #2, #3.
Follow-up to the _input_contract_test_helpers.py fix — ruff prefers over .
These three load_recipe split files carry an autouse fixture that only takes a tool_ctx_kitchen_open parameter and has an empty body — the leftover of splitting the original test_tools_load_recipe.py. The parameter already flows through the existing class-level autouse fixture (via test fixtures), so the wrapper does nothing. Delete each fixture body and keep the surrounding test class definition. Addresses review-pr finding #9.
Two authority tests select a tracker lease via _select_tracker_authority and only release it after _check_pipeline_deps and intervening asserts. If either raises, the on-disk .lease.lock leaks and contaminates the shared tool_ctx_kitchen_open fixture for downstream tests on the same xdist worker. Wrap acquire/release in try/finally so cleanup is unconditional. Addresses review-pr findings #10 (test_open_kitchen_auto_init_tracker.py), #11 (test_pipeline_tracker_authority.py).
The split-file inherited a misplaced module-level import with a noqa-E402
silencer. The asserted rationale ("to satisfy in-method reference
inside the class above") is incorrect — run_skill is resolved at call
time as a module global, so the import belongs in the top import block
alongside _check_dry_walkthrough and _get_config. Drop the noqa and
delete the misplaced block.
Addresses review-pr findings #22 (bugs), #23 (defense).
assert "invalid" in error and assert "content" in parsed["error"] are
loose enough to match traceback fragments or unrelated substrings.
Anchor each on the distinguishing phrase emitted by the source:
- "Result content is empty" for empty content
- "Missing required key: content" for missing content
- "is invalid:" for the JSON-parse error path in
_read_tracker_unlocked
Addresses review-pr findings #29, #30 (info/defense).
The previous _discard_background stub fired asyncio.create_task(asyncio.sleep(0)) and returned the unawaited task. If the lifespan exited before the loop scheduled it, asyncio would warn the task was destroyed pending and any exception it raised would be swallowed. Return an already-completed future instead — caller contract is satisfied with no resource to clean up. Addresses review-pr finding #37 (info/defense).
The session_type_visibility split docstrings narrated which conftest fixtures were dropped in the split and why — split-era scaffolding that will rot as files move. Replace with brief test-surface descriptions that remain stable as the test files migrate. Addresses review-pr findings #13, #14, #16, #17 (warning/slop).
…rcise the suppression path Both *_empty_step_name_skips_timing tests requested the bare tool_ctx fixture, which constructs a ToolContext with gate.enabled=False. wait_for_ci and wait_for_merge_queue short-circuit at the very first call to _require_enabled() and return a gate-error envelope without ever reaching the empty-step_name timing-suppression branch. The assertions therefore ran vacuously: removing the suppression entirely would not change test outcome. Switch to tool_ctx_kitchen_open (gate enabled) so the watcher runs, the empty step_name skips the timing_log.record() call inside the function, and the post-condition assert is actually meaningful. Addresses review-pr finding #40 (info/defense, paired with the shape at test_tools_ci_wait_for_merge_queue.py:301).
test_wait_for_merge_queue_invalid_remote_url_falls_through_to_inference shells out to a real 'git remote get-url upstream' subprocess without patching it, even though every other test in the file does. Project defaults place pytest tmp_path under /tmp (not a git repo), so the test passes today — but a developer running with TMPDIR inside their git-tracked home, or CI with TMPDIR set to a repo subdir, would break the 'repo is None' assertion for environmental reasons unrelated to the behavior under test. Match the file's established pattern: patch create_subprocess_exec to return empty stdout + returncode 1, mirroring the 'no git repo' sibling tests (lines 201-208). Addresses review-pr finding #41 (info/defense).
…entry The '`_ensure_ctx` is dead' finding (#9) was incorrect — the empty bodies still served as autouse injection points so test methods without '`tool_ctx_kitchen_open`' in their signature (and that depend on `_get_ctx()` returning the open-kitchen context inside load_recipe's control flow) still get a configured ToolContext. Removing the fixture silently regressed 5 tests: - test_load_recipe_surfaces_validation_failure - test_yaml_error_surfaces_as_suggestion - test_value_error_surfaces_as_suggestion - test_file_not_found_surfaces_as_suggestion - test_unexpected_exception_returns_structured_error Re-add the empty-body autouse fixtures to keep those tests wired. Also revert the workspace cascade L853 visibility↔sous_chef swap from the previous commit: the new test_tools_kitchen_visibility.py no longer imports workspace at module level, which fails the import- package cascade guard, while sous_chef (which DOES import workspace) is the entry that satisfies the guard.
The test asserts on plain substrings of mypy stdout, but built the subprocess env from os.environ, so mypy honoured an inherited FORCE_COLOR/COLORTERM and interleaved ANSI escapes inside the quoted type names, breaking the match. Pass --no-color-output so the assertion holds regardless of the caller's terminal environment.
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
1 participant
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.
Split MEDIUM-effort server test files (Refs #4610)
Six oversized
tests/server/test files (~4,900 LoC collectively) split into 26 focused per-concern files. Mechanical refactor — no production-code changes, no behavior changes, no test assertion changes. Every existing test continues to execute unchanged in its new home.Per-source-file breakdown
test_pipeline_tracker.pytest_tools_kitchen_visibility.pytest_tools_execution_input_gates.pytest_tools_load_recipe.pytest_tools_ci.pytest_session_type_visibility.pyNaming convention
<production_function>.py(when a single function is the unit) or<parent>_<child>.py(when a behavioural concern is the unit), mirroring the existing precedent set bytest_record_pipeline_step_complete.py,test_tools_ci_enqueue.py,test_tools_ci_merge_state.py,test_tools_git_branch.py, andtest_tools_execution_input_gates_file_path_list.py.Cross-cutting updates
tests/_test_filter.py— refreshed path-string references for the split source files; rebuiltLAYER_CASCADE_CONSERVATIVE["recipe"]to include the four new files that genuinely importautoskillit.recipe(the contract-validation / exception-handling / migration / sous-chef splits), removed stale per-concern entries that never imported it at AST level, and added a newrecipekey to_IMPORT_GUARD_TRANSITIVE_OVERRIDESfor the kepttest_tools_load_recipe.pywhose recipe touchpoints exist only as patch-string literals.tests/arch/test_feature_markers.py— replaced the deleted-source entry in_FLEET_CLASS_MARKERSwith the two new fleet files (test_session_type_visibility_fleet.py,test_session_type_feature_gates.py) registered in_FLEET_CROSS_DIR_FILES.Marker contract preserved
pytestmark = [pytest.mark.layer("server"), pytest.mark.<small\|medium>]pertests/AGENTS.md.pytest.mark.feature("fleet")in pytestmark (the redundancy with the deleted class-level decorator is resolved by pytestmark-level coverage instead of class-level coverage)._reset_mcp_visibilityautouse fixtures fromTestFeatureGateVisibilityandTestExplorerBindingVisibilityare dropped because the directory conftest's_reset_mcp_tagsautouse already truncatesmcp._transformsand disables every gated tag.Verification
task test-all(full test suite): PASS —37600 passed, 603 skipped, 27 xfailed, 5224 warnings(~8 min)pre-commit run --all-files: clean (ruff format, ruff check, mypy, uv lock, gitleaks, all other hooks)tests/arch/test_layer_markers.pyandtests/arch/test_size_markers.pycontinue to pass — every new file carries the required layer+size markers.tests/arch/test_cascade_map_guard.pycascade-narrowing tests continue to pass after the_test_filter.pycascade repair.Notes
tests/server/_input_contract_test_helpers.pycentralizes YAML-loaded contract-spec tables (loaded once at import time rather than per test class) and deterministic-UUID fixtures consumed by the CWD and integration tests. Leading underscore marks it as test-internal, matching the existing_pipeline_test_helpers.pyand_recipe_segment_test_helpers.pyconventions.Refs: #4610