Skip to content
This repository was archived by the owner on May 8, 2026. It is now read-only.

feat: add devspace config and e2e tests#6

Merged
rowan-stein merged 3 commits intomainfrom
noa/issue-5
Mar 13, 2026
Merged

feat: add devspace config and e2e tests#6
rowan-stein merged 3 commits intomainfrom
noa/issue-5

Conversation

@casey-brooks
Copy link
Copy Markdown
Contributor

Summary

  • add DevSpace configuration to patch the docker-runner deployment for node dev workflows
  • add e2e vitest config and docker-runner gRPC lifecycle coverage
  • wire up the test:e2e script and keep default test runs scoped to unit/integration tests

Testing

  • pnpm lint
  • pnpm build
  • pnpm test

Fixes #5

@casey-brooks
Copy link
Copy Markdown
Contributor Author

Summary

  • add DevSpace configuration for docker-runner dev deployments and sync setup
  • add e2e vitest config plus gRPC lifecycle coverage with auth helpers
  • include test:e2e script and exclude e2e suite from default vitest runs

Testing

  • pnpm lint
  • pnpm build
  • pnpm test

@casey-brooks
Copy link
Copy Markdown
Contributor Author

Test & Lint Summary

  • pnpm lint
    • Lint: no errors
  • pnpm build
  • pnpm test
    • Tests: 12 passed, 0 failed, 0 skipped

Copy link
Copy Markdown

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

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

The DevSpace configuration, vitest e2e config, and package.json changes are clean and match the issue spec well. The e2e test cases cover all 8 scenarios from the issue.

Main concern: The e2e test helper functions are near-identical copies of the integration test helpers. Please extract the shared gRPC test utilities (client wrapper, auth metadata builder, container lifecycle helpers) into a common module to avoid code duplication across test suites.

Two minor items on test assertion completeness are also flagged.

Comment thread __tests__/e2e/docker-runner.e2e.test.ts Outdated
Comment thread __tests__/e2e/docker-runner.e2e.test.ts
Comment thread __tests__/e2e/docker-runner.e2e.test.ts Outdated
@casey-brooks
Copy link
Copy Markdown
Contributor Author

Summary

  • extract shared gRPC test helpers into tests/helpers/grpc-test-client.ts
  • update integration and e2e tests to use response.id and shared helpers
  • add stop-state assertion in e2e stop test

Testing

  • pnpm lint
  • pnpm build
  • pnpm test

Copy link
Copy Markdown

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

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

All three prior review comments are properly addressed:

  • Shared helpers extracted — clean createGrpcTestClient factory in __tests__/helpers/grpc-test-client.ts, both test files now import from it. Nicely done.
  • Canonical response.id — fallback removed in both files.
  • Stopped-state assertionsstateRunning and stateStatus checks added.

However, the refactor left one dangling reference: line 48 in the e2e test calls ready() which no longer exists as a local function. This will fail at runtime. Quick fix — change to grpcTestClient.ready().

Comment thread __tests__/e2e/docker-runner.e2e.test.ts Outdated
@casey-brooks
Copy link
Copy Markdown
Contributor Author

Summary

  • switch e2e ready health check to use grpcTestClient.ready()

Testing

  • pnpm lint
  • pnpm build
  • pnpm test

Copy link
Copy Markdown

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

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

All review feedback resolved and verified in code:

  • ✅ Shared gRPC test helpers extracted into __tests__/helpers/grpc-test-client.ts
  • ✅ Canonical response.id used consistently (no fallback)
  • ✅ Stopped-state assertions added (stateRunning, stateStatus)
  • ✅ Dangling ready() reference fixed to grpcTestClient.ready()

Clean PR — good to merge.

@rowan-stein rowan-stein merged commit ee12921 into main Mar 13, 2026
1 check passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add DevSpace configuration and e2e tests

3 participants