Skip to content

test: table-drive failure classification on the check, lookup, and LSP surfaces - #1027

Merged
e54-bot merged 2 commits into
mainfrom
fix/1022-table-tests
Oct 29, 2026
Merged

e54-bot merged 2 commits into
mainfrom
fix/1022-table-tests

Conversation

@e54-bot

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

Copy link
Copy Markdown
Collaborator

Summary

Adds one table test per public surface that reports provider or session failures, per the #1022 owner decision:

  • crates/wright-driver/tests/check_failures.rs — the check envelope. Every ProviderError variant (injected through an LppSourceProvider over a scripted LanguageProvider) plus the session failure kinds that reach the surface: the CompilerSession::new construction diagnostic (invalid-selection), session-config-changed, and the source-provider-*/profile-workshop-only load refusals. Each row pins the diagnostic code, the class, and the exit code the class resolves to.
  • crates/wright-driver/tests/lookup_failures.rs — the tool-service Lookup response. Connect-channel failures (lpp/initialize) and request-channel failures (lpp/lookup) driven by a scripted provider process, plus the session-boundary failure kinds. Each row pins ToolErrorInfo.code and class (or the unavailable verdict). Notes in the file record the failure kinds that cannot reach the surface.
  • crates/wright-language/tests/provider_failures.rs — LSP diagnostics on an .opy document. Same connect/request-channel coverage plus the session-construction diagnostics; each row pins the published code and error severity (this surface carries no exit class).

lint-rule-error is deliberately unpinned on the driver surfaces: Diagnostic.class declares SourceError, but the CLI wrappers that can emit the diagnostic — run_configured and run_lookup in crates/wright-cli/src/main.rs and serve::run in crates/wright-cli/src/serve.rs — discard diagnostic.class and hardcode exit USAGE for every session-construction failure. The declared class and the reachable surface behavior disagree; which side is correct is left for a separate owner decision, and this PR makes no production change.

Removed the coverage the tables subsume: the lookup_failure_class unit table, opy_lookup_carries_an_initialize_refusal_with_its_code, opy_lookup_reports_unavailable_when_the_provider_lacks_the_capability, lpp_source_provider_distinguishes_failure_classes, and the refusal/diagnostics halves of two wright-language service tests (the version-negotiation and rename halves remain — they cover mechanisms the tables do not). The only production diff is the deleted mod tests; for the removed unit-test file; no behavior change.

Verification

  • cargo test -p wright-driver -p wright-language — all suites pass (the new tables included).
  • cargo test -p wright-cli — all suites pass.
  • cargo test --workspace --all-targets --all-features — pass.
  • cargo clippy --workspace --all-targets --all-features -- -D warnings — clean.
  • cargo fmt --all -- --check — clean.
  • git diff --check — clean.
  • Mutation checks (issue acceptance criteria), each applied, tested, and reverted:
    • provider_failure_code mapping refusals to unavailable → all three tables fail.
    • lookup_failure_class without the invalid-* arm → lookup table fails.
    • connect_opy_provider relabeling a session diagnostic to provider-refusal-<code> → LSP diagnostics table fails.

Unverified

  • UI gates (cd ui && npm ci && npm run check && npm test && npm run build && npm run check:size) — not applicable; the change is Rust test-only, ui/ untouched.
  • The three scripted-provider surfaces (provider-io request-channel, client session-state errors after a live connect, provider-not-configured for opy on lookup/LSP) have no deterministic trigger — documented in each table's notes and covered at the layer where they are reachable.

…P surfaces

Add one table test per public surface that reports provider or session
failures. `check` and `lookup` live in a new wright-driver integration
test; LSP diagnostics lives in wright-language. Every row drives its
surface end to end and pins the diagnostic `code` plus the exit class
(`check`/`lookup`) or the boundary code (`diagnostics`), so a wrapper
that re-classifies a typed failure — a refusal mapped to `unavailable`,
an `invalid-*` lookup code defaulted to `SourceError`, or a session
diagnostic relabeled `provider-refusal-*` — fails its row.

Removed the tests the tables subsume: the lookup classification unit
test, the two process-level lookup refusal tests, the LPP provider
failure-class test, and the refusal/diagnostics halves of two
language-service tests (the version-negotiation and rename halves stay).

Mutation checks: mapping refusals to `unavailable`, defaulting
`invalid-*` lookup codes to `SourceError`, and relabeling a session
diagnostic `provider-refusal-invalid-selection` each fail a table.

Fixes #1022
Review of the failure-classification tables found two defects:

- The lint-rule-error construction rows pinned `SourceError`, but every
  CLI surface that can emit that diagnostic (wright-cli `run_configured`,
  `run_lookup`, `serve::run`) discards `Diagnostic::class` and exits
  USAGE. Per the owner ruling the rows are removed rather than re-pinned;
  the class/wrapper disagreement is left for a separate owner decision.
  Each table now notes why the kind is deliberately unpinned.
- The test-file headers named a nonexistent `failure_classification`
  file and claimed every row drives `session.check()`/`Lookup` end to
  end, though the session-boundary rows only reach
  `CompilerSession::new`/`ToolService::new`. The headers now name the
  real tables and describe the boundary each row drives.

No production change.
@e54-bot

e54-bot commented Oct 29, 2026

Copy link
Copy Markdown
Collaborator Author

Review findings addressed in 7045f4b (test-only; no production change):

  • Finding 1 — lint-rule-error rows: removed from check_failures.rs and lookup_failures.rs per the owner ruling — removed, not re-pinned. Each table now records why the kind is deliberately unpinned. Surfaced for a separate owner decision: Diagnostic.class declares SourceError for lint-rule-error, but the CLI wrappers that can emit it — run_configured and run_lookup in crates/wright-cli/src/main.rs and serve::run in crates/wright-cli/src/serve.rs — discard diagnostic.class and hardcode exit USAGE for every session-construction failure. The declared class and the reachable surface behavior disagree; which side is correct is left for a separate owner decision.
  • Finding 2 — headers: provider_failures.rs now names the real check_failures.rs/lookup_failures.rs tables; both driver headers and their test docs now state that every row drives the boundary that surfaces the diagnostic — CompilerSession::new/ToolService::new for the session-boundary rows, session.check()/the Lookup response end to end for the rest.

Verification: all three tables pass; cargo test --workspace --all-targets --all-features passes; cargo clippy --workspace --all-targets --all-features -- -D warnings, cargo fmt --all -- --check, and git diff --check are clean.

Ready for follow-up review.

@e54-bot
e54-bot merged commit a2ab75b into main Oct 29, 2026
36 checks passed
@e54-bot
e54-bot deleted the fix/1022-table-tests branch October 29, 2026 02:43
@github-project-automation github-project-automation Bot moved this from Backlog to Done in Make Workshop Great Again Oct 29, 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