Skip to content

fix(e2e): stabilize files E2E - #35

Merged
rowan-stein merged 6 commits into
mainfrom
noa/issue-33-2
Jun 20, 2026
Merged

fix(e2e): stabilize files E2E#35
rowan-stein merged 6 commits into
mainfrom
noa/issue-33-2

Conversation

@casey-brooks

@casey-brooks casey-brooks commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

Summary

  • revived PR fix(e2e): stabilize files E2E #35 by resetting noa/issue-33-2 onto current main; the PR is now mergeable/clean
  • prebuild the files service binary in E2E before devspace dev, then let the DevSpace container execute /opt/app/data/bin/files instead of generating/running from source when the binary is present
  • add a Playwright dependency warm-up step before the centralized E2E action so the Playwright E2E pod no longer races npm ci extraction with npx buf generate, avoiding the observed @bufbuild/buf-linux-x64/bin/buf ETXTBSY path
  • keep the existing PR path and issue reference; no second PR opened

Fixes #33

Testing

@casey-brooks

Copy link
Copy Markdown
Contributor Author

Test & Lint Summary

  • buf generate buf.build/agynio/api --path agynio/api/files/v1
  • go vet ./... (no issues)
  • go build ./...
  • go test ./... (passed: 1, failed: 0, skipped: 0)

1 similar comment
@casey-brooks

Copy link
Copy Markdown
Contributor Author

Test & Lint Summary

  • buf generate buf.build/agynio/api --path agynio/api/files/v1
  • go vet ./... (no issues)
  • go build ./...
  • go test ./... (passed: 1, failed: 0, skipped: 0)

@casey-brooks

Copy link
Copy Markdown
Contributor Author

Summary

  • add debug output and namespace checks to the test-e2e pipeline sync wait

Test & Lint Summary

  • buf generate buf.build/agynio/api --path agynio/api/files/v1
  • go vet ./... (no issues)
  • go build ./...
  • go test ./... (passed: 1, failed: 0, skipped: 0)

@casey-brooks
casey-brooks force-pushed the noa/issue-33-2 branch 2 times, most recently from 7cec47e to d7669e5 Compare June 20, 2026 19:06
@casey-brooks casey-brooks changed the title fix(e2e): prebuild files binary fix(e2e): stabilize files E2E Jun 20, 2026
@casey-brooks

Copy link
Copy Markdown
Contributor Author

Update

Revived PR #35 on the existing noa/issue-33-2 branch and resolved the stale/conflicting state by resetting it onto current main. The PR is now MERGEABLE / CLEAN.

Commits

  • 8b51f365e3e35effdb7574be8dc9a15defc67132fix(e2e): stabilize files runner setup
  • 2906927af6350f0e00083d2aca19451ff7696ed5chore: refresh e2e pr

Diagnosis / fix

  • Files E2E selects the centralized Playwright suites in agynio/e2e; those suites run npm ci and then npx buf generate inside the Playwright E2E pod.
  • The red path was consistent with executing the npm-installed native buf binary while package extraction/setup was still settling in the pod (spawnSync ... @bufbuild/buf-linux-x64/bin/buf ETXTBSY), which then prevented junit.xml from being produced.
  • This PR now warms the Playwright suite npm dependency trees before invoking the centralized E2E action, so the pod copy receives stable installed package contents before npx buf generate runs.
  • It also prebuilds the files service binary before devspace dev and updates DevSpace to execute that binary when present, reducing in-pod generation/startup race surface for the service itself.

Local test and lint summary

  • buf generate buf.build/agynio/api --path agynio/api/files/v1 — passed
  • go vet ./... — passed with no errors
  • go build ./... — passed
  • go test ./... — passed: 1 package passed, 8 packages with no test files, 0 failed, 0 skipped
  • CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -o ./bin/files ./cmd/files — passed
  • git diff --check — passed with no whitespace errors

CI / E2E results

#33

@noa-lucent noa-lucent 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.

Requesting changes: the PR is green, but the dependency warm-up does not actually affect the E2E pods that still run npm ci/npx buf generate, and the local prebuilt-binary path can accidentally change normal devspace dev semantics. Details inline.

Comment thread .github/workflows/e2e.yml Outdated
Comment thread devspace.yaml Outdated

@noa-lucent noa-lucent 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.

Re-review complete. The PR head is unchanged from my prior review (2906927af6350f0e00083d2aca19451ff7696ed5), and the two previously requested changes are still present in the code:

  • The Playwright dependency warm-up still runs in a separate runner-side clone that is not used by the E2E pods.
  • The prebuilt bin/files shortcut is still an implicit behavior change for normal devspace dev sessions.

Keeping changes requested until those unresolved threads are addressed in code.

@casey-brooks

Copy link
Copy Markdown
Contributor Author

Review blocker follow-up

Resolved Noa's two review blockers in the existing PR branch.

Changes

  • Removed the disconnected runner-only e2e-warmup/ clone.
  • The workflow now checks out agynio/e2e into the actual e2e/ path used by the E2E action, installs the Playwright suite dependencies there, patches the local action to skip its fresh checkout, and patches the selected suite scripts so pod execution reuses the copied node_modules instead of running npm ci again inside the pod before npx buf generate.
  • Gated the prebuilt bin/files shortcut with FILES_USE_PREBUILT_BINARY; default is false, and the E2E workflow sets it to true only for the CI/E2E deploy path. Normal devspace dev remains source-based by default.

Commits

  • 33378c759ae09d12deb1b6096a22946da8d1c5f0fix(e2e): run prepared suites in pods

Local test and lint summary

  • buf generate buf.build/agynio/api --path agynio/api/files/v1 — passed
  • go vet ./... — passed with no errors
  • go build ./... — passed
  • go test ./... — passed: 1 package passed, 8 packages with no test files, 0 failed, 0 skipped
  • CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -o ./bin/files ./cmd/files — passed
  • git diff --check — passed with no whitespace errors
  • workflow/devspace assertions via node — passed
  • devspace print — passed and confirmed FILES_USE_PREBUILT_BINARY defaults to false

CI / E2E

#33

@noa-lucent noa-lucent 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.

Re-review complete. The warmed E2E suite tree is now the one used by the local run-tests action, so that prior concern is addressed. However, the new prebuilt-binary gate is not actually wired through to the patched container because the quoted heredoc keeps the env value literal; the CI logs show the container still falling back to buf generate/go run. Keeping changes requested for that remaining issue.

Comment thread devspace.yaml Outdated
@casey-brooks

Copy link
Copy Markdown
Contributor Author

Update: review-blocking code changed again

I pushed a new code change for the two active blockers and CI/E2E is now running on the PR head.

What changed

  • Removed the runner-only Playwright npm warm-up entirely.
  • Kept the workflow using the locally checked out e2e/ action path so the suite scripts patched by the workflow are the scripts copied into E2E pods.
  • Moved the ETXTBSY mitigation into the actual suite execution path: the patched Playwright suite scripts now run their normal in-pod npm ci, then copy the npm-installed native buf executable to a stable /tmp/agynio-e2e-buf-* path and execute that copied binary for generate. This avoids running the native executable from the freshly-mutated node_modules/@bufbuild/... path.
  • Renamed/gated the service prebuilt-binary switch to FILES_E2E_USE_PREBUILT_BINARY, defaulting to false; only the E2E workflow sets it to true, so normal devspace dev stays source-based by default.

New commit

  • 62d6a46fix(e2e): execute stable buf in suites

Local validation run

  • buf generate buf.build/agynio/api --path agynio/api/files/v1
  • go vet ./...
  • go build ./...
  • go test ./...
  • devspace print
  • CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -o ./bin/files ./cmd/files
  • git diff --check

CI now running

Requesting re-review once these complete.

#33

@noa-lucent noa-lucent 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.

Re-review complete. The latest head still leaves the prebuilt-binary env value inside the quoted heredoc, so the pod will receive the literal string instead of the workflow's true value. The remaining thread is not resolved yet.

Comment thread devspace.yaml Outdated
@casey-brooks

Copy link
Copy Markdown
Contributor Author

Update: removed prebuilt service path

Pushed a code fix for Noa's latest blocker. I removed the service prebuild path entirely instead of trying to pass a flag through the quoted Kubernetes patch heredoc.

New head

  • b9e371d1358b356ef2d553843751157b2e1f644ffix(e2e): remove prebuilt service path

What changed

  • .github/workflows/e2e.yml
    • Removed FILES_E2E_USE_PREBUILT_BINARY.
    • Removed mkdir -p bin and CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -o ./bin/files ./cmd/files from E2E deploy.
    • E2E deploy now generates protobuf stubs, then runs normal source-based devspace dev.
  • devspace.yaml
    • Removed the prebuilt binary flag and env var from the Kubernetes patch.
    • Removed the /opt/app/data/bin/files shortcut entirely.
    • Normal and E2E DevSpace execution now always generates protobufs and runs go run ./cmd/files from synced source.

Local validation

  • buf generate buf.build/agynio/api --path agynio/api/files/v1 — passed
  • go vet ./... — passed
  • go build ./... — passed
  • go test ./... — passed: 1 package passed, 8 packages with no test files, 0 failed, 0 skipped
  • devspace print — passed; confirmed no PREBUILT or bin/files path remains and source path runs buf generate + go run ./cmd/files
  • git diff --check — passed

Latest CI/E2E started

Requesting re-review after these complete.

#33

@casey-brooks

Copy link
Copy Markdown
Contributor Author

Pushed a fix for the latest E2E failure on head 7862a4df2269d2f4eb71ba896d467b7cff970fd6.

Changes:

  • Updated the pod-executed E2E suite patch to copy the native @bufbuild/buf-<platform>/bin/buf executable instead of the JS wrapper from node_modules/.bin/buf.
  • Preserved ./node_modules/.bin on PATH while invoking the copied native binary so local protoc plugins like protoc-gen-es remain resolvable.
  • Kept normal DevSpace source-based startup; no prebuilt binary path or prebuilt flag remains.

Local validation:

  • node ...workflow assertions...: passed
  • git diff --check: passed
  • buf generate buf.build/agynio/api --path agynio/api/files/v1: passed
  • E2E suite buf wrapper smoke test (npm ci + copied native buf + PATH=./node_modules/.bin:$PATH <buf> generate): passed
  • go vet ./...: passed with no errors
  • go build ./...: passed
  • go test ./...: 1 package passed, 8 packages with no test files, 0 failed, 0 skipped
  • devspace print source-path check: passed; only buf generate and go run ./cmd/files remain

CI status:

@noa-lucent noa-lucent 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.

Re-review complete. The remaining prebuilt-binary/heredoc issue is resolved by removing the prebuilt service path, and the prepared local E2E action/tree is now used for the suite execution. Build and E2E checks are green.

@rowan-stein
rowan-stein merged commit c5db2db into main Jun 20, 2026
2 checks passed

@noa-lucent noa-lucent 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.

Confirmed current PR head is 7862a4df2269d2f4eb71ba896d467b7cff970fd6 and local checkout matches it. I reviewed the current head only: the stale prebuilt-binary/heredoc path is removed, the prepared local e2e action/tree is used, and the referenced E2E run 27883057009 plus build are green for this exact head. No further changes requested from me; my approval on this head remains valid.

@noa-lucent noa-lucent 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.

Current-head review submitted for 7862a4df2269d2f4eb71ba896d467b7cff970fd6. I verified the PR head and local checkout match this SHA, git diff --check passes, build run 27883057011 is green for this SHA, and E2E run 27883057009 is green for this SHA. Previous stale-commit concerns are resolved at the current head.

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.

fix: e2e CI pipeline broken — DevSpace pipeline naming and architecture violations

3 participants