-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
adds basic setup for e2e testing #115
Conversation
michelleN
commented
Feb 29, 2024
- adds a simple e2e test for the containerd executor
This PR now has an image available for testing:
|
c74aeee
to
9d82c22
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Michelle, this is some great work. The actual code itself LGTM but I have a number of overarching questions:
- How do these e2e tests compare to the tests we have like
spinapp_controller_test.go
. It's clear to me the setup is at a different level of abstraction, but what will this allow us to test that the other can't? - How do these e2e tests compare to the smoke tests we have? Does this obviate the need for some of our smoke tests?
- Would it be possible to add these tests into the CI in this PR? It'd be nice to be able to see they're passing.
e2e/default_test.go
Outdated
// TestDefaultSetup is a test that checks that the minimal setup works | ||
// | ||
// with the containerd wasm shim runtime as the default runtime. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Formatting for this comment
e2e/k3d_provider.go
Outdated
} | ||
|
||
func (c *Cluster) Destroy() error { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Extra newline
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm gonna give this a proper review tomorrow but generally LGTM - I don't love the bdd-i-ness, but it's at least go-test-y bdd
.
@calebschoepp this is gonna be great for testing things like #122 - where we want to be able to validate that the provided runtime config is something that the shim can understand - because that relies on having a real cluster, rather than just an API serverr.
jobs: | ||
e2e: | ||
runs-on: ubuntu-latest | ||
steps: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needs the standard boilerplate of checkout steps / installing Go
+ adds a simple e2e test for the containerd executor Signed-off-by: Michelle Dhanani <[email protected]>
|
+ adds e2e make target for running e2es + updates return err to include output for better debugging experience + add workflow for checking e2e on PRs to main Signed-off-by: Michelle Dhanani <[email protected]>