test: delete dead and trivial test files (#4598) - #4616
Merged
Conversation
…reate_worktree.sh) C1.1 from issue #4598 audit-tests review. The 3 tests in this file exercised shutil.copy2 (stdlib) directly — never invoking src/autoskillit/recipes/scripts/create_worktree.sh. They verified stdlib behavior, not production behavior. No sibling coverage loss: tests/recipe/test_create_worktree_script.py and tests/recipe/test_create_worktree_functional.py already exist and are independent of this module. Issue: #4598 Finding: C1.1 (HIGH)
C1.3 from issue #4598 audit-tests review. The 3 tests in this file only asserted callable() or is not None after import — they verify Python's import machinery, not production behavior. Sibling coverage is robust: - tests/workspace/test_clone_detect.py: TestClassifyRemoteUrl (11+ tests) - tests/workspace/test_clone_remote.py: TestCloneRemoteUrlResolution, TestProbeSingleRemote, TestProbeCloneSourceUrl - tests/workspace/test_clone_core.py: 20+ behavioral tests calling clone_repo() directly Issue: #4598 Finding: C1.3 (HIGH)
…ctory) C6.1 from issue #4598 audit-tests review. The 4 test classes (TestResearchDesignSubRecipe, TestResearchImplementSubRecipe, TestResearchReviewSubRecipe, TestResearchArchiveSubRecipe) all use class-scoped fixtures that look in builtin_sub_recipes_dir() ('recipes/sub-recipes/') — but research-{design,implement,review, archive}.yaml live at builtin_recipes_dir() ('recipes/'). src/autoskillit/recipes/sub-recipes/ contains only research.json and research.yaml. Every test in this file exits via pytest.skip() before reaching any assertion. Deletion removes zero coverage. Coverage is provided by sibling files (existed before this change): - tests/recipe/test_bundled_recipes_research_design.py - tests/recipe/test_research_implement_recipe.py - tests/recipe/test_research_review_recipe.py - tests/recipe/test_research_archive_recipe.py Issue: #4598 Finding: C6.1 (HIGH, upgraded from MEDIUM)
…tive tests) C1.8 from issue #4598 audit-tests review. Removed 8 tests that only verified Python import machinery, string-source substring matches, or hasattr() checks — none asserted real production behavior: Deleted (lines 23-30, 39-82 of the pre-trim file): - test_marketplace_module_exists (line 23) -- pass with comment - test_install_importable_from_marketplace (line 28) -- bare import - test_upgrade_importable_from_marketplace (line 40) -- bare import - test_ensure_marketplace_importable_from_marketplace (line 45) - test_install_defined_in_app_module (line 50) -- inspect.getsource substring - test_install_registered_as_cli_command (line 61) -- hasattr() - test_upgrade_is_registered_as_cli_command (line 69) -- substring - test_marketplace_module_still_importable (line 80) -- bare import Preserved (the 3 real tests): - test_install_requires_typed_request (real signature contract: install() raises TypeError matching 'request' without request kwarg) - TestInstallPluginInstallCapableGuard.test_rejects_when_plugin_install_not_capable - TestInstallPluginInstallCapableGuard.test_passes_guard_when_plugin_install_capable Module-level pytestmark preserved (layer + small markers intact). Imports and _direct_request() helper preserved. Issue: #4598 Finding: C1.8 (MEDIUM)
Auto-generated by scripts/compare-coverage-ast.py --mode build-test-source-map via 'task coverage-audit'. This derived artifact dropped the deleted file paths (test_audit_trail_artifacts, test_clone_split, test_research_sub_recipes) and retained the surviving test_cli_marketplace.py reference (3 entries). 591 source files mapped.
Reviewer finding (warning/bugs) flagged that deletion of MK-DEP-2 removed
the only test verifying 'def upgrade(' is exposed in cli/app.py. While
tests/cli/test_install.py already covers the install CLI surface via
app(['install']) and test_app_install_* family tests, no test covered
the upgrade command registration — only test_upgrade_uses_atomic_write
imports upgrade from autoskillit.cli._marketplace, not cli.app.
Add a regression guard that exercises app(['upgrade']) and asserts the
command dispatches to the registered @app.command wrapper. Tests the
actual dispatch boundary, not source-text grep.
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.
Summary
Remove four test files / sections that the
audit-testsreview validated as dead or trivial — they exerciseshutil.copy2semantics, Python's import machinery, or are unconditionally skipped — and add no production-behavior coverage. The substantive tests intests/cli/test_cli_marketplace.py(theTestInstallPluginInstallCapableGuardclass) are preserved.tests/recipe/test_audit_trail_artifacts.pytests/workspace/test_clone_split.pytests/cli/test_cli_marketplace.py(8 useless tests)test_install_requires_typed_request+TestInstallPluginInstallCapableGuardtests/recipe/test_research_sub_recipes.pyChanges
tests/recipe/test_audit_trail_artifacts.py— 63 lines / 3 tests deleted. Tests exercisedshutil.copy2(stdlib) directly; never invokedcreate_worktree.sh.tests/workspace/test_clone_split.py— 27 lines / 3 tests deleted. Tests only assertedcallable()/is not Noneafter import — Python import machinery, not production behavior. Siblingtest_clone_*.pyfiles provide robust behavioral coverage.tests/recipe/test_research_sub_recipes.py— 111 lines / 4 classes × 4 tests deleted. Class-level fixtures looked inrecipes/sub-recipes/(onlyresearch.json/research.yaml); the recipes live inrecipes/. All 16 tests unconditionallypytest.skip()-ed at fixture level. Coverage is provided by siblingtest_research_{implement,review,archive}_recipe.pyandtest_bundled_recipes_research_design.py.tests/cli/test_cli_marketplace.py— 56 lines / 8 useless tests removed. Retainedtest_install_requires_typed_request(real signature contract:install()raisesTypeErrormatching "request") +TestInstallPluginInstallCapableGuard(2 methods driving the realinstall()guard with monkeypatch-stubbed config/backend). Module-levelpytestmark = [pytest.mark.layer("cli"), pytest.mark.small]preserved..autoskillit/test-source-map.json— regenerated viatask coverage-audit; 591 source files mapped, 0 references to deleted paths, 3 surviving references totest_cli_marketplace.py.Verification
pre-commit run --all-filespasses on the committed code.tests/cli/test_cli_marketplace.pycollects exactly 3 tests:test_install_requires_typed_request,TestInstallPluginInstallCapableGuard::test_rejects_when_plugin_install_not_capable,TestInstallPluginInstallCapableGuard::test_passes_guard_when_plugin_install_capable.*.py/*.yaml/*.json/*.md(the single matchtest_research_sub_recipes_declare_food_truck_kindlives in the survivingtest_research_sub_recipe_rules.pyand is independent of the deleted file).Closes #4598
Implementation Plan
Plan file:
.autoskillit/temp/make-plan/delete_dead_test_files_4598_plan_2026-08-15_120000.md🤖 Generated with Claude Code via AutoSkillit