Skip to content

fix(conformance): fail on a CONFORMANCE_REPO override that points nowhere - #142

Merged
chrisuthe merged 3 commits into
Sendspin:mainfrom
chrisuthe:chrisuthe/task/fail-loudly-on-a-conformance-repo-override-that
Oct 7, 2026
Merged

chrisuthe merged 3 commits into
Sendspin:mainfrom
chrisuthe:chrisuthe/task/fail-loudly-on-a-conformance-repo-override-that

Conversation

@chrisuthe

Copy link
Copy Markdown
Member

What

A CONFORMANCE_REPO_<NAME> override that points at a path which does not exist now stops the run with an error naming the variable and the path. Before, it was skipped and resolution fell through to repos/<name>, so the run audited a different checkout than the one asked for and nothing in the output said so.

This is the half #125 left alone: that PR fixed the test that tripped over the fallthrough and deliberately did not change resolution.

Behaviour

repos/spec exists in all three cases.

CONFORMANCE_REPO_SPEC Before (935b5ae) After
set, exists resolves to the override resolves to the override
set, does not exist (/nonexistent/spec) resolves to repos/spec, silently RepoOverrideError: CONFORMANCE_REPO_SPEC=/nonexistent/spec points at /nonexistent/spec, which does not exist
unset resolves to repos/spec resolves to repos/spec

Observed before, with the override set to a nonexistent path:

CONFORMANCE_REPO_SPEC=/nonexistent/spec
  resolve_repo_path('spec'): <worktree>/repos/spec
  resolve_required_repo_path('spec'): <worktree>/repos/spec

After, at each entry point:

$ CONFORMANCE_REPO_SPEC=/nonexistent/spec CONFORMANCE_REPO_SENDSPIN_RS=~/nope python scripts/run_all.py
Repository override does not resolve to a checkout: CONFORMANCE_REPO_SENDSPIN_RS=/Users/chris/nope points at /Users/chris/nope, which does not exist; CONFORMANCE_REPO_SPEC=/nonexistent/spec points at /nonexistent/spec, which does not exist
exit 1, no results directory created

$ CONFORMANCE_REPO_SPEC=/nonexistent/spec python -m conformance.cli run
conformance: error: Repository override does not resolve to a checkout: CONFORMANCE_REPO_SPEC=/nonexistent/spec points at /nonexistent/spec, which does not exist
exit 2

conformance build and scripts/setup_workspace.py stop the same way.

Where the error is raised, and why

Two places, on purpose.

In the resolver (paths.repo_override_path, used by candidate_repo_paths). A set override is now the only candidate for its repository, so no caller can fall through. The error is a new RepoOverrideError that is not a FileNotFoundError: the build steps catch FileNotFoundError from ensure_repo_checkout and turn it into an ordinary failed build row, after which the matrix carries on. A bad override reported that way would be one more red row in a 20-minute run rather than a stop.

At startup (validate_repo_overrides()), as the first thing scripts/run_all.py, scripts/setup_workspace.py and conformance build / conformance run do. Without this the error would first surface wherever a repository happens to be resolved: partway through the build, or once per case inside an adapter subprocess. It checks every known repository and reports all bad overrides in one message, before anything is built and before run_matrix clears the results directory.

conformance report does not check, because it resolves no checkout. scripts/setup_repositories.py is unchanged: it never reads the override and clones into --repos-dir as before.

One deliberate tightening: overrides must be absolute

A relative override is now rejected (CONFORMANCE_REPO_AIOSENDSPIN=./aiosendspin is not an absolute path). Adapters run with the repository root as their working directory and re-resolve the inherited variable, so a relative path that passes the startup check from another directory names a different directory inside every adapter. Today that is the same silent fallthrough; with only the nonexistent-path check it would have become a per-case failure mid-run. ~ is still expanded.

This is the one case that worked before and no longer does: a relative override used from the repository root. Say if you would rather keep it.

Unchanged

  • No override set: candidates are still repos/<name> then the sibling directory, first existing wins. Unpinned clones are untouched.
  • No path guessing or text matching. scripts/detect_regressions.py is not touched. No ScenarioSpec change, so no scenario_revision bump.

Verification

  • python -m unittest discover -s tests: 309 pass. New tests/test_repo_override.py covers unset, empty, valid, nonexistent, relative, several bad at once, a build step not downgrading the error, and the CLI stopping before build or run.
  • conformance run --from aiosendspin --to aiosendspin --jobs 1, no override, on 935b5ae and on this branch against the same fresh clones (aiosendspin 90cecf2, sendspin-cli 2a1dbbe, spec 671a34d): 11 passed, 2 failed on both, identical per case. Both failures are the client's declared lack of OPUS and legacy-unencrypted support.
  • The same run with CONFORMANCE_REPO_AIOSENDSPIN set to an absolute copy of the checkout: 11 passed, 2 failed, same cases.
  • conformance report renders, and repositories.json records the spec and aiosendspin revisions.

Earlier runs of the comparison showed one to three extra address already in use failures on either side. Another checkout on the same machine was running a matrix on the same fixed ports at the time; the runs quoted above had none.

Not in this PR

Found while tracing the consumers, and left out to keep this to one change:

  1. A valid override can still be ignored by the native builds. ensure_repo_checkout() returns repos/<name> whenever it exists, even when a valid override resolved somewhere else, and the Rust, Swift, JVM, C++ and Go adapters reference repos/<name> by relative path. With CONFORMANCE_REPO_SENDSPIN_RS set to an existing directory and a real repos/sendspin-rs present, resolve_repo_path returns the override and ensure_repo_checkout returns repos/sendspin-rs, so the build uses one checkout while repositories.json reports the other's revision. The fix needs a decision (reject the combination, or make those adapters honour the override).
  2. A misspelt variable name such as CONFORMANCE_REPO_AIOSENSPIN matches no repository and has no effect.
  3. Recording in the report that an override was used. It needs a field in repositories.json plus site normalisation and rendering, and until (1) is fixed the flag would be wrong for the native adapters.

…here

A CONFORMANCE_REPO_<NAME> override naming a path that does not exist was
skipped, and resolution fell through to repos/<name>. The run then audited a
different checkout than the one asked for, with nothing in the output saying so.

A set override is now the only candidate for its repository, and resolving one
that does not exist raises RepoOverrideError naming the variable and the path.
run_all.py, setup_workspace.py and the build and run commands check every
override before doing any work, so the error arrives at startup rather than
partway through a matrix.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unknown-user tilde paths bypass the new error handling and produce a traceback.

2 open findings
What changed in this PR

Makes repository overrides authoritative and fails early when they reference invalid paths.

Changes:

  • Adds override validation and aggregated errors.
  • Validates overrides at build, run, and workspace startup.
  • Documents behavior and adds tests.
File Description
README.md Documents repository overrides.
src/​conformance/​paths.py Validates and resolves override paths.
src/​conformance/​implementations.py Aggregates invalid overrides.
src/​conformance/​cli.py Validates before build or run.
scripts/​setup_workspace.py Validates before workspace setup.
scripts/​run_all.py Validates before full execution.
tests/​test_repo_override.py Tests override resolution and failure behavior.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/conformance/paths.py Outdated
Comment thread README.md Outdated
@chrisuthe
chrisuthe marked this pull request as ready for review October 7, 2026 16:28
@chrisuthe
chrisuthe merged commit 55c4d0b into Sendspin:main Oct 7, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants