Skip to content

ci(e2e): sweep step-local e2e tests in make e2e#323

Merged
kunchenguid merged 1 commit into
kunchenguid:mainfrom
e-jung:fm/nm-e2e-makefile-j4
Jul 1, 2026
Merged

ci(e2e): sweep step-local e2e tests in make e2e#323
kunchenguid merged 1 commit into
kunchenguid:mainfrom
e-jung:fm/nm-e2e-makefile-j4

Conversation

@e-jung

@e-jung e-jung commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Intent

The developer wanted to extend no-mistakes' Makefile e2e target so it sweeps internal/pipeline/steps/*_e2e_test.go (the step-local JS/Swift provider e2e tests) in addition to the existing ./internal/e2e/... sweep, which currently misses them. The change had to be minimal, keep existing tests unaffected, and be verified by running make e2e. Explicit constraints: branch off origin/main (not the diverged local main), stay inside the worktree, and — per the overriding Rules and Definition-of-Done sections — ship local-only with no push, no PR, and no merge, keeping the branch a clean fast-forward onto the default branch on fm/nm-e2e-makefile-j4. Any durable project knowledge gained should be recorded in AGENTS.md, proportionate to the change.

What Changed

  • Extended the Makefile e2e target to sweep ./internal/pipeline/steps/... alongside ./internal/e2e/..., so step-local e2e tests (e.g. internal/pipeline/steps/*_e2e_test.go, gated by //go:build e2e) now run under make e2e instead of being silently missed.
  • Updated the e2e target's comment to document the broader sweep.
  • Added an AGENTS.md note recording that make e2e covers both locations and that step-local e2e tests must stay behind the e2e tag so go test ./... still skips them.

Risk Assessment

✅ Low: A two-file, forward-looking change to a test-invocation target that adds a package pattern and matching documentation; the side effect (re-running unit tests in parallel) is acknowledged, bounded by the existing 300s timeout, and introduces no correctness or security risk.

Testing

The full make e2e passes with the new ./internal/pipeline/steps/... path included (internal/e2e 82.5s, internal/pipeline/steps 42.2s, exit 0), confirming existing tests are unaffected. Because no committed step-local *_e2e_test.go files exist yet, I added a temporary e2e-tagged probe in internal/pipeline/steps/ and showed it is excluded from the plain go test ./... sweep but listed and executed under make e2e's -tags=e2e invocation, directly proving the stated intent; the probe was then removed and the tree left clean with only the Makefile + AGENTS.md changes committed.

Evidence: make e2e full run output

go test -tags=e2e -count=1 -timeout 300s ./internal/e2e/... ./internal/pipeline/steps/... ok github.com/kunchenguid/no-mistakes/internal/e2e 82.452s ok github.com/kunchenguid/no-mistakes/internal/pipeline/steps 42.197s ===make e2e exit: 0

go test -tags=e2e -count=1 -timeout 300s ./internal/e2e/... ./internal/pipeline/steps/...
ok  	github.com/kunchenguid/no-mistakes/internal/e2e	82.452s
ok  	github.com/kunchenguid/no-mistakes/internal/pipeline/steps	42.197s
Evidence: step-local e2e sweep proof (probe excluded from go test ./..., run by make e2e)

Makefile e2e target: go test -tags=e2e -count=1 -timeout 300s ./internal/e2e/... ./internal/pipeline/steps/... Committed *_e2e_test.go under internal/pipeline/steps: 0 1) go test ./... (no tag) -> probe NOT listed (excluded) 2) make e2e invocation (-tags=e2e) -> probe RUN and PASS: === RUN TestStepLocalE2eSweptByMakeE2e step-local e2e-tagged test reached by make e2e --- PASS: TestStepLocalE2eSweptByMakeE2e (0.00s) PASS

## Probe: e2e-tagged step-local test sweep

### Makefile e2e target (post-change):
e2e:
	go test -tags=e2e -count=1 -timeout 300s ./internal/e2e/... ./internal/pipeline/steps/...

### Confirm no committed *_e2e_test.go exist in internal/pipeline/steps (pre-probe):
(git-tracked e2e test files under steps:) 0

### Temporary probe file created: internal/pipeline/steps/sweep_probe_e2e_test.go
    build constraint: //go:build e2e

### 1) go test ./... sweep must EXCLUDE probe (no -tags=e2e):
ok  	github.com/kunchenguid/no-mistakes/internal/pipeline/steps	0.015s

### 2) make e2e's invocation (-tags=e2e) must INCLUDE + RUN probe:
=== RUN   TestStepLocalE2eSweptByMakeE2e
    sweep_probe_e2e_test.go:10: step-local e2e-tagged test reached by make e2e
--- PASS: TestStepLocalE2eSweptByMakeE2e (0.00s)
PASS
ok  	github.com/kunchenguid/no-mistakes/internal/pipeline/steps	0.016s

### 3) Full 'make e2e' result (this run):
    internal/e2e        ok  82.452s
    internal/pipeline/steps  ok  42.197s   (newly swept by e2e target)

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

✅ **Review** - passed

✅ No issues found.

✅ **Test** - passed

✅ No issues found.

  • go test -race ./...
  • make e2e (full suite: go test -tags=e2e -count=1 -timeout 300s ./internal/e2e/... ./internal/pipeline/steps/...) -> ok internal/e2e 82.5s, ok internal/pipeline/steps 42.2s, exit 0
  • go test ./internal/pipeline/steps/... -list 'TestStepLocalE2eSweptByMakeE2e' (no -tags=e2e) -> probe NOT listed, confirms e2e-tagged step-local tests stay excluded from the normal go test ./... sweep
  • go test -tags=e2e ./internal/pipeline/steps/... -list 'TestStepLocalE2eSweptByMakeE2e' -> probe LISTED, confirms it is compiled under the e2e tag that make e2e uses
  • go test -tags=e2e -count=1 -timeout 300s -run '^TestStepLocalE2eSweptByMakeE2e$' ./internal/pipeline/steps/... -v -> PASS, confirms make e2e's exact invocation runs the step-local e2e-tagged test
  • baseline go test -race ./... (already ran successfully per task context)
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

@kunchenguid

Copy link
Copy Markdown
Owner

Thanks for the contribution, @e-jung! One process note on this repo: PRs need to be raised through no-mistakes (git push no-mistakes), which runs the review/test/lint/CI pipeline and stamps the required signature into the PR body. This PR is currently failing the "PR must be raised via no-mistakes" check because it wasn't raised that way.

If working from a fork was the blocker, that's fixed as of v1.30.0 (#306). Per CONTRIBUTING.md: point origin at this parent repo, run no-mistakes init --fork-url <your-fork-url>, then git push no-mistakes.

I won't be merging PRs that bypass the gate going forward, but I'd genuinely love to land your work once it's re-raised. Thanks for understanding! 🙏

The `e2e` target only swept `./internal/e2e/...`, so e2e-tagged tests
that live next to the pipeline-step code (e.g. coverage provider
journeys in `internal/pipeline/steps/*_e2e_test.go`) were silently
skipped by `make e2e`. Add `./internal/pipeline/steps/...` to the
sweep so step-local e2e tests are covered too. The `//go:build e2e`
tag still excludes them from `go test ./...` / `make test`.

Note: `internal/pipeline/steps` is a mixed package, so the sweep also
runs that package's unit tests under the e2e tag (~42s); they pass and
run in parallel with the e2e journey suite, adding no measurable
wall-clock cost. AGENTS.md gains a bullet recording the sweep paths.
@e-jung e-jung force-pushed the fm/nm-e2e-makefile-j4 branch from 612069a to e8e55a6 Compare June 30, 2026 19:19
@kunchenguid kunchenguid merged commit f425557 into kunchenguid:main Jul 1, 2026
7 checks passed
@kunchenguid

Copy link
Copy Markdown
Owner

Thanks @e-jung - merged! Really appreciate the contribution.

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