Skip to content

test: prune redundant and implementation-mirroring tests - #1026

Merged
e54-bot merged 3 commits into
mainfrom
test/prune-redundant-tests
Oct 28, 2026
Merged

e54-bot merged 3 commits into
mainfrom
test/prune-redundant-tests

Conversation

@e54-bot

@e54-bot e54-bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Audit of the Wright test suite under docs/testing-policy.md: remove tests with no distinct durable claim. No production behavior changes.

  • 946 → 809 #[test] functions, 85 → 40 snapshots, about 5,100 lines removed.
  • Commit 1 (CLI focus):
    • CLI tests that repeat coverage held elsewhere: agent_contract_surface_parity (CLI JSON == agent result), plain/terminal snapshots, driver service tests, present.rs/agent.rs/agenttools.rs units.
    • Terminal snapshot matrix: 4 width × color variants per command → one 100-column no-color snapshot; width and color keep dedicated tests.
    • Help-text and removed-flag history locks; hard-coded inventories (MCP tool-name list, athena_version == "0.11.0", emittedBytes=3960).
    • Serve-level copies of driver behavior (probe-vs-capture runs once, in wright-driver/tests/simulate.rs).
    • Code-grammar unit tests written three times → one set in wright-analyzer.
    • Removed the #[cfg(test)] spinner frame counter in present.rs.
  • Commit 2 (driver/analyzer/LSP):
    • Driver service tests already covered by surface parity (session_query_workflows_…, tool_service_routes_…) or that only assert Ok.
    • Seven pass-through provider-failure tests folded into provider_failures_keep_their_code_stage_and_exit_class.
    • Unit tests repeated by an integration test of the same claim (edit.rs, cache.rs, input.rs, wright-lsp capability advertisement).
    • CLI copies of driver lint-package, lookup, and address-snapshot behavior.
  • Kept: LPP protocol robustness, LSP framing, edit atomicity/rollback, self-update safety, freshness/stale-id contract, lint semantics, check --json schema snapshot (CODEOWNERS file unchanged).

Test plan

  • cargo fmt --all -- --check
  • cargo clippy --workspace --all-targets --all-features -- -D warnings
  • cargo test --workspace --all-targets --all-features --no-fail-fast: all binaries pass
  • git diff --check
  • No fixture under tests/fixtures became unreferenced

Remove tests that repeat coverage held at another layer (agent/CLI surface parity, driver service, unit tests), lock help text or removed-flag history, hard-code fixture-derived counts, or need test-only production state. Terminal snapshots keep one 100-column no-color variant per command; width and color behavior stay covered by their dedicated tests. Probe-vs-capture comparisons run once, in the driver suite.
Second pass over the driver, analyzer, and LSP suites. Remove service tests already covered by agent/CLI surface parity, smoke tests that only assert Ok, unit tests repeated by an integration test of the same claim, and CLI copies of driver lint-package and lookup behavior. Fold seven pass-through provider-failure tests into one table-driven test.
@e54-bot

e54-bot commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator Author

Review verdict: CHANGES REQUESTED

Reviewed head f72979bc. Two removals drop the only test of a public CLI contract. A mutation proves each one: the bug passes the PR suite, and the removed test on main catches it.

1. lint package failure exit classes are no longer pinned at any layer

The PR removes install_failures_report_stable_error_codes and search_terms_narrow_the_query_and_search_errors_report_their_code (crates/wright-cli/tests/lint_package.rs). These were the only tests that pinned exit codes on the single-error path (error_envelope → error.exit_code(), crates/wright-cli/src/lint/package.rs:95): lint-package-source → USAGE/2, lint-package-invalid → 1 on install, lint-package-rate-limited → 4.

No surviving test covers these exit codes:

  • Driver tests check only the code (crates/wright-driver/tests/lint_package_pin.rs:512, tests/lint_package.rs:280,345). None calls LintPackageError::exit_class().
  • documented_codes_keep_their_exit_class (crates/wright-driver/src/result.rs:594) takes the class as an input. It cannot detect a wrong classification.
  • The surviving lifecycle tests pin 4 (update missing/pkg) and 1 (the update row path). Both hit the _ => Internal arm or the row path, so they do not cover USAGE.

Mutation M1: crates/wright-driver/src/lint_package.rs:634 Self::Source(_) => ExitClass::Usage → ExitClass::Internal.

  • PR head, cargo test --workspace --all-targets --all-features: all pass. The mutation survives.
  • With main's lint_package.rs restored: install_failures_report_stable_error_codes FAILED at lint_package.rs:304 (USAGE package-install missing).

This is the defect class in #1022 (a failure re-wrapped into a default class at a boundary). Testing policy › Keep failures observable requires public command failure tests to validate the externally visible result kind.

Required: keep one table test that pins code and exit for each LintPackageError variant on the CLI surface: Source → 2, Invalid → 1, NotFound/RateLimited → 4. A driver-level table on exit_class() plus one CLI case for the error_envelope path is also acceptable.

2. wright lookup --limit is no longer wired to any test

The PR removes lookup_rejects_an_out_of_range_limit (crates/wright-cli/tests/cli.rs). It was the only test of the --limit flag → ToolRequest::Lookup.limit mapping (crates/wright-cli/src/main.rs:496). crates/wright-driver/tests/lookup.rs:234 tests the range at the service layer, so it cannot detect a CLI that drops the flag. serve.rs's lookup_rejects_an_out_of_range_limit_on_every_transport was also removed.

Mutation M2: main.rs:496 limit: args.limit → limit: None.

  • PR head: all tests pass. The mutation survives.
  • With main's cli.rs restored: lookup_rejects_an_out_of_range_limit FAILED at cli.rs:4469 (exit Some(0), expected Some(1)).

Testing policy › Organize tests by feature and behavior ranks public contract tests above lower-layer copies. The flag is CLI-owned, so the driver test is not the same claim.

Required: restore one CLI case. For example, lookup wait --limit 0 -f json → exit 1, error.code == "malformed-request".

Evidence for the rest of the PR

  • Production diffs (present.rs, completion.rs, analyzer/src/service.rs, driver/src/{cache,diag,edit,input,service/refusal}.rs, language/src/service.rs, lsp/src/main.rs) change only #[cfg(test)] code. That includes the spinner frame counter.
  • provider_failures_keep_their_code_stage_and_exit_class keeps all seven code/stage/exit triples. The removed transport test used session.compile(). Mutation M3 (session/provider.rs Compile arm → Diagnostic::error("provider-failed", Stage::Internal, …)) is caught by check_only_provider_inspect_reports_the_provider_compile_decline and service_check_and_compile_run_without_a_prewarmed_snapshot.
  • Mutation M4 (drop the analyze selection wiring, main.rs:189) is caught by an_unknown_rule_id_in_a_selection_is_a_usage_error and unmatched_files_refuse_on_cli_and_serve.
  • Serve/simulate claims hold:
    • Probes, stale identity, and provider-loaded OPY simulate are in crates/wright-driver/tests/simulate.rs.
    • Disk-change freshness and stale-id are in serve_mcp.rs::mcp_transport_observes_disk_changes_and_enforces_fresh_ids and the driver/tests/service.rs reload tests.
  • Terminal width and color keep dedicated tests: width_is_capped_at_100_columns and no_color_and_color_flag_both_apply. The terminal__convert.snap rename from _w100_always is byte-identical to _w100_never.
  • agent_contract_surface_parity.rs is unchanged.
  • Gates (local): cargo fmt --all -- --check OK. cargo clippy --workspace --all-targets --all-features -- -D warnings OK. cargo test --workspace --all-targets --all-features --no-fail-fast OK: 70 binaries, 0 failures.
  • gh pr checks 1026: all required checks pass.
  • Mutations ran on the local working tree only. They were reverted, and nothing was pushed.

…-limit

Pin code and exit for each LintPackageError variant on the CLI surface in one table test, and restore one wright lookup --limit range case next to the lookup --only test.
@e54-bot

e54-bot commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator Author

Review round 1 fixes are pushed in f357161b. Please do a follow-up review.

1. lint package failure exit classes

crates/wright-cli/tests/lint_package.rs::package_failures_report_their_code_and_exit_class is a single table test on the CLI surface. It pins the header, the code, and the exit for each LintPackageError variant:

Case Code Exit
install gitlab.com/community/rules lint-package-source (USAGE package-install) 2
install with a malformed package lint-package-invalid 1
install against a 404 server lint-package-not-found 4
search against a rate-limited server lint-package-rate-limited 4

Mutation M1 (crates/wright-driver/src/lint_package.rs:634 Self::Source(_) => ExitClass::Usage → ExitClass::Internal): the test FAILS with lint-package-source: exit 4.

2. wright lookup --limit

crates/wright-cli/tests/selector_vocabulary.rs::lookup_rejects_an_out_of_range_limit runs lookup wait --limit 0 -f json and asserts exit 1 with error.code == "malformed-request". The test sits next to lookup_only_filters_entries_by_kind. The size-guard hook rejects any growth of cli.rs because the file is over its limit, so the test does not go there.

Mutation M2 (crates/wright-cli/src/main.rs:496 limit: args.limit → limit: None): the test FAILS with exit Some(0), expected Some(1).

Gates (local)

cargo fmt --all -- --check, cargo clippy --workspace --all-targets --all-features -- -D warnings, cargo test --workspace --all-targets --all-features, and git diff --check all pass. I reverted both mutations, and the commit does not contain them.

@e54-bot
e54-bot merged commit b931f92 into main Oct 28, 2026
36 checks passed
@e54-bot
e54-bot deleted the test/prune-redundant-tests branch October 28, 2026 07:16
@github-project-automation github-project-automation Bot moved this from Backlog to Done in Make Workshop Great Again Oct 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

2 participants