Clean up the test suite: one test file per source, named fixtures, Go E2E runner by nodeselector · Pull Request #192 · github/gh-actions-lock · GitHub
Skip to content

Clean up the test suite: one test file per source, named fixtures, Go E2E runner - #192

Closed
nodeselector wants to merge 71 commits into
nodeselector-transitive-lock-graphsfrom
nodeselector-test-suite-cleanup
Closed

nodeselector wants to merge 71 commits into
nodeselector-transitive-lock-graphsfrom
nodeselector-test-suite-cleanup

Conversation

@nodeselector

@nodeselector nodeselector commented Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator

Planned layer 7/7 in the approved order: #197 → #198 → #190 → #195 → #196 → #193 → #192. Parent: #193; rebase and base retarget are pending. Native stack 191 retains its existing membership; the source-bound evidence below is historical, not qualification of the planned restack.

Stacked on #190 in the stack #186 → #190 → #192. This PR changes only tests, CI and docs; product behavior is unchanged.

What changed

  • Fixture guard: internal/testfixture gives every real upstream commit the tests use a name and its source release. TestPinsAreInRecordedRefHistory (make test-fixtures, run by a new blocking fixture-guard job and at the start of integration-live) proves each commit is in its recorded ref's history and that repository and owner IDs match. It caught the actions/setup-go@v5 fixture, which pinned a v6.4.0 commit with checkout's repository ID.
  • Test file names: every test file is named after the source file it tests (foo.go → foo_test.go); the 27 files that weren't are merged into the matching file, and TestEveryTestFileHasSource enforces the rule.
  • Shared fakes: httpmock now owns the fakes command and API tests kept re-declaring (StubRepo, ParityIntact/ParityQuery, RoundTripFunc, CountingTransport), and testfixture.Metadata replaces three copies of the same repository-ID fake.
  • Command tests: they use workflowUsing and runCheckJSON instead of repeating workflow bodies and JSON decoding, and tests that differed only in one stubbed response are now tables (TestCheck_RelockAncestry, TestShouldRetry).
  • Go E2E runner: test/e2e (about 750 lines) replaces the 3,200-line Ruby harness, driving the compiled binary over test/scenarios/catalog.yml with a TLS fake GitHub selected by stub: profiles. Stub scenarios now run in go test ./....
  • Catalog: three scenarios that only restated Go command tests are removed, and TestEveryScenarioRunsInCI requires every scenario to be tagged stub or need a token.
  • Docs: AGENTS.md documents the naming rule, named pins and shared fakes, and CI no longer installs Ruby.

Reviewer guide

Reading order:

  1. internal/testfixture/fixtures.go and its test: pins, the guard and the naming rule.
  2. internal/ghapi/httpmock/githubfixtures.go: the shared fakes.
  3. cmd/gh-actions-lock/root_test.go: the command harness. In run_test.go, the diff is mostly moved tests; the tables are the part to read.
  4. test/e2e/e2e.go, then the catalog.yml diff.
  5. .github/workflows/test.yml, Makefile and AGENTS.md.

Evidence against #190's head:

  • make test passes.
  • Per-package coverage is the same or higher everywhere (httpmock goes from 35% to 68%).
  • 7 of 7 mutations of key rules are caught: severity decides validity, exact ref names, owner/repo case-folding, stale-workflow pruning, opacity of ./ actions, rejection of nested ./ actions, and checking against the recorded entry ref.
  • E2E_LIVE=1 go test ./test/e2e passes. Before the Ruby was deleted, the Go runner matched it on all 103 scenarios.
  • make test-fixtures passes.
  • The test list matches Validate and rebuild complete transitive lock graphs #190's apart from renames (tenant-host test names) and the table merges above.

Expect

  • go test ./... takes about 20s longer, because the server-error scenario waits through the binary's retry backoff.
  • Catalog exit codes are now matched exactly. Two flag-conflict scenarios exit 2.
  • make test-live now runs the live scenarios; before, it selected none.
  • github.com/creack/pty is now a direct dependency, for the one interactive-prompt scenario.

nodeselector and others added 15 commits October 19, 2026 19:11
Recover local fix fcdd43df96c77d5a376a3c27de8d03f07f6317c5 and retain both community normalization safeguards.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Recorded tags now re-peel before repair (#174) and branch normalization
needs a head proof (#165), so same-run conflicting repairs can no longer
be planned. Fixtures ported from candidate commit 3eb0100.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Parity now resolves recorded branch heads and fails closed when the
recorded ref is inconclusive (#174). Stub fixtures ported from candidate
commits 3eb0100 and 22e894c.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CHECKOUT_SHA was the actions/checkout v6.0.2 commit while the fixtures
record it under v4 and main. Recorded-ref tracing (#174) correctly rejects
that; use the real v4.2.2 commit the stub tag list already names.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Restores the #167 same-run scenario with a recorded tag instead of
swapping it for a branch, which only exercised the missing-head exit.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep the original --verify bodies, adjusted for ref-qualified lookups and
fail-closed inconclusive checks; replace the mode cross-products with one
default/--no-fix test, and drop full-SHA and entry-ref cases that
TestParityFinding_FullSHAKey and TestValidEntryRef already cover.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The backend resolver, not the runner, looks up and rejects lock entry
refs; say so in comments, docs and the missing-ref finding detail.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Tags move, and the Actions backend accepts a pin whose commit is still in
its tag's history. Every lock entry ref is now checked by reachability:
a tag that lost the commit still blocks and an inconclusive check still
fails closed, but a tag that moved ahead only warns under --verify.

Dropping the separate tag-equality lookup also saves the extra REST
commit call when a ref still points at its locked commit.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
An error-severity finding blocks the run, so it never renders as a
warning. Under --verify the terminal now shows an exact tag that moved
ahead of its locked commit, matching the JSON warning, and the
inconclusive-check remediation names the Actions backend.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
A ref like main~1 or v1@{1} is not a tag or branch name, but a commit-ish
lookup evaluates it as a revision. Validate names with git
check-ref-format rules, matching actions-lockfile's RefName: an entry ref
that fails makes the lockfile unreadable, and parity never builds a
lookup expression from one.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
--relock drops findings for recorded pins because it replaces them, but
it also dropped them for pins it had just resolved, so it could write a
lock that the next --verify rejects.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
A full-SHA entry whose recorded tag or branch no longer vouches for it
recommended --accept-moved, which then refused it. --accept-moved now
accepts it as a warning and re-records a tag or branch that contains the
commit, chosen by reverse lookup; a commit no branch contains still fails.
Fresh full-SHA pins are no longer checked against a pre-planning ref.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The Actions backend resolver looks a recorded ref up as a tag before a
branch, so reverse lookup now skips a containing branch whose name is
also a tag.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
A ref counts only if a tag or branch exists by exactly that name: GraphQL
also asks ref(qualifiedName:), and REST reads git/ref/{tags|heads}/<name>.
Like the Actions backend, names ending in -g plus four or more hex
digits are not accepted as recorded or uses: refs, so an entry with one
makes the lockfile unreadable and a uses: ref with one is never locked.

Adapted from 2134d14 and 93b2f4a.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@nodeselector
nodeselector force-pushed the nodeselector-test-suite-cleanup branch from d0d11a3 to 8b02826 Compare October 20, 2026 03:51
@nodeselector
nodeselector changed the base branch from nodeselector-174-trace-recorded-ref to nodeselector-transitive-lock-graphs October 20, 2026 03:51
@nodeselector
nodeselector added this pull request to stack #191 October 20, 2026 03:51
--accept-moved now prints the old and new recorded ref when it keeps a
full-SHA pin's commit but records a new tag or branch for it. A ref this
run resolved that isn't a tag or branch is titled "Not a tag or branch";
"Ref no longer exists" is kept for a ref the lockfile recorded.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@nodeselector
nodeselector force-pushed the nodeselector-transitive-lock-graphs branch from b1a85f2 to e3315a6 Compare October 20, 2026 03:58
nodeselector and others added 3 commits October 19, 2026 21:00
A tool failure now prints before the run log path instead of after it.
Inconclusive checks and the Dependabot proxy abort show a short cause on
the terminal, such as "could not check v4 history: GitHub returned 403
Forbidden"; --json and the run log keep the full error.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
A full-SHA pin whose recorded tag or branch no longer exists was titled
"Pinned commit not in upstream history". It is now "Ref no longer
exists", like any recorded ref that is gone.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@nodeselector
nodeselector force-pushed the nodeselector-test-suite-cleanup branch from 8b02826 to 2cca9d3 Compare October 20, 2026 04:06
@nodeselector
nodeselector force-pushed the nodeselector-transitive-lock-graphs branch from e3315a6 to 342b1a3 Compare October 20, 2026 04:09
@nodeselector
nodeselector force-pushed the nodeselector-test-suite-cleanup branch from 2cca9d3 to a0d9f34 Compare October 20, 2026 04:14
@nodeselector
nodeselector requested a max review from Copilot October 20, 2026 04:18
Terminal warnings were grouped by dependency alone, so a full-SHA pin's
sha-as-ref notice hid the --verify warning that its recorded exact tag
moved ahead. Group by dependency and category.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

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

Unresolved fixture-validation, E2E cache-isolation, reference-matching, and coverage-path defects require fixes.

1 open issue
What changed in this PR

Reorganizes tests into source-aligned files, centralizes named fixtures, and replaces the Ruby integration harness with Go E2E tests.

Changes:

  • Consolidates tests and reusable HTTP fakes.
  • Adds named pins, fixture validation, and test-layout guards.
  • Updates scenario handling, Make targets, CI, and documentation.
File Description
test/​scenarios/​catalog.go Extends scenario definitions and adds strict decoding.
test/​scenarios/​catalog_test.go Adds CI-lane and fixture-reference checks.
test/​e2e/​e2e_test.go Builds the binary and runs selected scenarios.
Makefile Switches integration targets to Go; adds fixture checks.
internal/​testfixture/​fixtures.go Defines named pins, synthetic commits, and lockfile fixtures.
internal/​testfixture/​fixtures_test.go Adds fixture provenance and test-layout guards.
internal/​tag/​tags_test.go Consolidates lister and cache-key tests.
internal/​tag/​tags_keys_test.go Removes relocated cache-key tests.
internal/​tag/​tagging_test.go Moves lister-specific tests out.
internal/​pipeline/​run_test.go Uses a named checkout fixture.
internal/​pipeline/​diagnose_test.go Consolidates diagnosis helpers and tests.
internal/​pipeline/​diagnose_helpers_test.go Removes relocated helper tests.
internal/​pipeline/​checks/​run_test.go Adopts a checkout fixture in resolver tests.
internal/​pin/​retain_impostor_test.go Moves retention tests into commit tests.
internal/​pin/​relock_test.go Moves relock tests into plan tests.
internal/​pin/​normalization_test.go Moves normalization tests into plan tests.
internal/​pin/​imprecise_test.go Moves precision tests into plan tests.
internal/​pin/​commit_test.go Consolidates retention tests and metadata fixtures.
internal/​lockfile/​prune_test.go Moves pruning tests into state tests.
internal/​lockfile/​hosts_test.go Moves host tests into state tests.
internal/​lockfile/​entryref_test.go Redistributes entry-reference tests by source responsibility.
internal/​lockfile/​direct_tracker_test.go Uses a named checkout fixture.
internal/​lockfile/​convertor_test.go Houses entry-reference validation tests.
internal/​ghapi/​rest_fallback_test.go Uses the shared transport adapter.
internal/​ghapi/​repos_test.go Consolidates request-coalescing tests.
internal/​ghapi/​repos_dedup_test.go Removes relocated coalescing tests.
internal/​ghapi/​httpmock/​httpmock.go Adds repeated stubs and HTTP serving.
internal/​ghapi/​httpmock/​httpmock_test.go Tests server-backed and repeated responses.
internal/​ghapi/​httpmock/​githubfixtures.go Adds shared GitHub responses and transport helpers.
internal/​ghapi/​httpmock/​githubfixtures_test.go Tests shared HTTP fixture helpers.
internal/​ghapi/​hosts_test.go Renames tenant-host tests.
internal/​ghapi/​graphql_action_files_test.go Uses the shared transport adapter.
internal/​ghapi/​client_test.go Consolidates retry cases and transport helpers.
internal/​dep/​dependency_test.go Uses named commit fixtures.
internal/​config/​file_test.go Removes relocated configuration tests.
internal/​config/​config_test.go Consolidates configuration-file tests.
go.sum Records E2E dependency checksums.
go.mod Adds E2E runner dependencies.
CONTRIBUTING.md Documents Go-based integration testing.
cmd/​gh-actions-lock/​verify_test.go Reuses workflow and JSON helpers.
cmd/​gh-actions-lock/​selfrepository_test.go Removes relocated self-reference tests.
cmd/​gh-actions-lock/​runlog_test.go Uses shared HTTP responses and named pins.
cmd/​gh-actions-lock/​root_test.go Consolidates root tests and command-test helpers.
cmd/​gh-actions-lock/​reporoot_test.go Removes relocated repository-root tests.
cmd/​gh-actions-lock/​prune_workflow_test.go Removes relocated pruning tests.
cmd/​gh-actions-lock/​proxima_test.go Removes relocated tenant-host tests.
cmd/​gh-actions-lock/​pin_summary_test.go Partially adopts named checkout fixtures.
cmd/​gh-actions-lock/​parallel_steps_test.go Removes relocated parallel-step tests.
cmd/​gh-actions-lock/​onboard_gate_test.go Uses named pins and shared HTTP fixtures.
cmd/​gh-actions-lock/​missing_ref_test.go Removes relocated missing-reference tests.
cmd/​gh-actions-lock/​migrate_test.go Removes relocated migration tests.
cmd/​gh-actions-lock/​freshtag_test.go Removes relocated freshness tests.
cmd/​gh-actions-lock/​fresh_tag_test.go Consolidates freshness tests.
cmd/​gh-actions-lock/​format/​url_test.go Uses a named checkout fixture.
cmd/​gh-actions-lock/​format/​json_test.go Renames a tenant-host JSON test.
cmd/​gh-actions-lock/​check_json_golden_test.go Removes relocated JSON-contract tests.
cmd/​gh-actions-lock/​atomic_test.go Removes relocated atomic-write tests.
AGENTS.md Documents test layout and fixture conventions.
.github/​workflows/​test.yml Updates Go integration jobs and fixture validation.

Review effort: Max

Comment thread internal/testfixture/fixtures_test.go Outdated
nodeselector and others added 17 commits October 19, 2026 23:08
Each internal test file is now named after the source file it exercises
(foo.go -> foo_test.go). Pure moves: the same 367 test functions run
before and after.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Command tests now live in the test file of the source that implements the
behavior: runCheck wiring in run_test.go, root command flags, paths and
the shared command harness in root_test.go, cooldown resolution in
fresh_tag_test.go, the no-onboard gate in onboard_gate_test.go. Pure
moves: the same 154 test functions run before and after.

With no orphans left, TestEveryTestFileHasSource drops its allowlist.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
httpmock now owns the shapes command and API tests kept re-declaring:
StubRepo for the batched resolve query, ParityIntact/ParityQuery,
RoundTripFunc and CountingTransport. Command tests use workflowUsing
for the standard one-job workflow and runCheckJSON/checkResult for the
--json=valid,findings payload instead of hand-decoding it.

The four reachability tests that differed only in the compare response
are one table, TestCheck_RelockAncestry. Tenant-host tests get neutral
names.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Go tests that spelled out actions/checkout and actions/setup-go release
commits now reference testfixture pins, so each literal carries the
release it came from and the fixture guard checks it. Adds the
checkout v4.2.1 pin those tests use.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
lockfile, pin and pipeline tests each declared their own RepoIDs fake
returning fixed IDs; testfixture.Metadata replaces all three.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
These files already asserted with testify everywhere else.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
ServeHTTP and RegisterRepeated let a subprocess under test talk to the
same stub registry the command tests use.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
test/e2e builds the binary once and runs every catalog scenario in a
fresh git repo. Stub scenarios name a fake-GitHub profile (stub:) served
by httpmock over TLS and run in go test ./...; live scenarios need
E2E_LIVE=1. Custom assertions become declarative output_count and
then_run, exit codes are matched exactly, and every no-token scenario is
tagged stub so it guards merge.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
make test-integration, test-live, test-smoke, test-stub, and test-real
now run go test ./test/e2e with E2E_LIVE / E2E_TAGS. CI no longer
installs Ruby. The interactive shell and review/report tooling go with
the harness.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
relock_bumps_stale_branch_ref restated TestCheck_Relock_BumpsMovedBranchRef;
verify_local_conflict_relock restated TestValidateOutputFlags_VerifyConflicts
(verify_conflict_verify_local still proves the real binary's usage exit
code); no_workflows_summary folds into no_workflows.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The impostor-pin guard accepted any lookup error, so a 401, rate limit
or outage passed it. It now requires a 404 or 422 and fails on
anything inconclusive.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Three gaps in the runner:

- Scenarios ran under the developer's HOME, so every run wrote its run
  log to the real user cache dir (where its retention GC could delete
  real logs), and live scenarios read the real user config file. Each
  scenario now gets its own HOME, XDG and gh config dirs; the unused
  GH_ACTIONS_LOCK_CACHE_DIR is gone.
- The lockfile coherence check matched `uses:` by prefix, so a stale
  `actions/checkout@v4` entry passed against `@v4.2.2`. usesRef now
  requires the exact ref, with owner/repo folding case.
- A relative E2E_COVERDIR resolved against each scenario's repository,
  silently dropping all coverage. It is now made absolute and created.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
TestCatalogSHAsAreNamedPins accepted any pin with the same SHA, so a real
commit attached to another repository or recorded under the wrong ref
passed. Bare `owner/repo@sha` uses must match the pin's repository, and
expected-JSON nwo/ref/sha triples must match a pin exactly.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
- Scenarios no longer inherit GH_HOST or enterprise tokens, so a
  developer's configured host can't redirect live runs.
- The package builds on Windows: the binary gets its .exe suffix,
  USERPROFILE and AppData move into the scenario home, and the PTY
  runner is limited to platforms that have one.
- lockfile_deps_cover_direct now also fails when a workflow uses a
  remote action its lockfile entry doesn't list, not only the reverse.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@nodeselector
nodeselector force-pushed the nodeselector-test-suite-cleanup branch from 7b55c62 to e4264b2 Compare October 20, 2026 06:13

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

Live-token contamination, fixture-guard correctness gaps, and unexercised PTY support remain unresolved.

1 open issue

Review effort: Max

Comment thread test/e2e/e2e_test.go Outdated
liveToken was a package-level sync.OnceValue, so whichever test called it
first fixed the token for every later scenario; under -shuffle=on a test
that set a fake GH_TOKEN made live scenarios authenticate with it.
TestMain now resolves the token into Runner.Token and scenarioEnv reads
it from there. The E2E make targets shuffle test order.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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