Skip to content
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

Add test phase to skaffold runner #992

Closed
nkubala opened this issue Sep 18, 2018 · 11 comments
Closed

Add test phase to skaffold runner #992

nkubala opened this issue Sep 18, 2018 · 11 comments
Labels
area/testing Issues concerning the testing phase of Skaffold kind/feature-request priority/p2 May take a couple of releases

Comments

@nkubala
Copy link
Contributor

nkubala commented Sep 18, 2018

We've had a few out of band discussions about this so opening a broader issue here to track.

I'm considering adding a test phase that the runner executes between build and deploy to complete the story, with the deploy phase relying on all the tests to pass. This could potentially be extended to support other types of tests in the future, either specified by us or by the user through custom scripts, etc. Opening this issue for discussion, any suggestions, specific use cases or feedback welcome.

@nkubala nkubala added the area/testing Issues concerning the testing phase of Skaffold label Sep 18, 2018
@nkubala
Copy link
Contributor Author

nkubala commented Sep 18, 2018

One idea I'm currently toying with is treating testers in a similar way to the builders and deployers, in that Skaffold supports multiple different implementations. The only real difference is that we can allow multiple testers specified for a given artifact (whereas with builders for example Skaffold only allows one). This would keep things in line with the pluggable architecture of the builders/deployers and make it relatively easy to add new implementations.

@balopat
Copy link
Contributor

balopat commented Oct 3, 2018

Other thoughts here:

  • unit tests are typically executed within the same build context as the building - e.g. go test, mvn test - this could happen within/somehow in conjunction with the build phase
  • after the a single artifact is built, container structure tests should focus on that artifact
  • multi-artifact verification would be some kind of integration testing that would be typically on-cluster and maybe off-cluster somehow?

@balopat
Copy link
Contributor

balopat commented Oct 4, 2018

One more set of thoughts:

What would be really nice is "easy to use knobs" in skaffold to run 0 unittests, 1-2 focused tests or all my unittests in an easy way per artifact, because that's what I constantly shift around as a developer, and sometimes I even want to do structure tests and integration tests, but those are significantly more rare.

@balopat
Copy link
Contributor

balopat commented Oct 23, 2018

Another idea: I just remembered using https://infinitest.github.io/ back in the day and it was working pretty well for me while doing TDD in an IDE setting.

@thomas-riccardi
Copy link

Unit-tests also produce artifacts: tests reports (tests results, timings, code coverage (xml, html), linters warnings stats...)
Gotchas:

  • Tests reports are useful also when tests failed (we probably can't just use a new type of builder to build "test reports" artifacts and properly fail on error).
  • When/how to execute unit-tests ? It can be executed during the build, but then we cannot produce tests artifacts; or it can be executed after the image is built, but then the image must include all runtime dependencies for the test, not great either.. (ideally we should be able to generate artefacts distincts from docker images in the middle of a multi-stage build; this is a larger issue than just skaffold)

@thomas-riccardi
Copy link

Multi-artifacts verification is indeed some kind of integration test, so we should reuse the deployers, but the workflow is more complicated, notably if we tag/push images to mark them as validated (for example using a release-candidate docker image registry before multi-artifact verification, then a different registry for real deployment).
We also don't want to deploy on the final/real deployment context for integration tests.

@tejal29 tejal29 added the priority/p3 agreed that this would be good to have, but no one is available at the moment. label Aug 21, 2019
@tstromberg tstromberg added the triage/discuss Items for discussion label Apr 30, 2020
@tstromberg
Copy link
Contributor

Adding discuss label to bring this back up for discussion.

@tstromberg tstromberg added priority/awaiting-more-evidence Lowest Priority. May be useful, but there is not yet enough supporting evidence. and removed triage/discuss Items for discussion priority/p3 agreed that this would be good to have, but no one is available at the moment. labels May 4, 2020
@nkubala nkubala added priority/p2 May take a couple of releases and removed priority/awaiting-more-evidence Lowest Priority. May be useful, but there is not yet enough supporting evidence. labels Jul 15, 2020
@nkubala nkubala added this to the Icebox [P2+] milestone Sep 1, 2020
@gnarea
Copy link

gnarea commented Nov 18, 2020

It's likely I haven't run into cases where the functionality proposed here would've been handy, but I don't see myself using it:

  • Helm already supports tests, and they're run automatically after the deployment. This works great in skaffold and with wait: true. (You don't need Helm for this anyway; I'd imagine a good old k8s job will do)
  • I don't want skaffold or any other tool to run my unit or functional test suites:
    • When developing locally, I rarely run the whole test suite. I tend to run a subset of it, and I always do it via my IDE (which has an amazing integration with test runners).
    • On CI, where I do run the whole unit and functional test suites, I don't really see the benefit of letting skaffold run npm test/gradle test/etc. I actually think that'd be way too implicit.

In other words, the way I see it: If I need to run tests in the cluster, I'd use Helm tests. If I need to run functional tests against the cluster, I'd run them from the host (which requires port forwarding). And if I need to run unit tests, I don't need Kubernetes at all.

I believe skaffold can be improved to make testing easier, but what I'm missing is already covered in #4022. And it's a far simpler change.

@thomas-riccardi
Copy link

To add to the discussion, we have several potential use cases. I'm not sure which would be the best to add support for directly in skaffold (or more generally which should or should not be done in general):

  • unit tests: may depend on extra resources than just the docker image that will end-up in production (run on different image?); need to access the test reports (not just the process return code)
  • some unit tests may need an external service running (e.g. redis, postgresql...)
  • linters: similar to unit tests;need source code, generates reports (which we probably don't want in the final image)
  • run the integration tests before deploying to the target environment: the idea could be to deploy and test with helm in a testing k8s context (for example a local minikube). Maybe the recommended way would be to run skaffold multiple times with different profiles (one for integration test, one for final deployment)?

@tejal29
Copy link
Member

tejal29 commented Dec 7, 2020

Thanks @thomas-riccardi We are working on a PRD and this seems very helpful.

@nkubala
Copy link
Contributor Author

nkubala commented Mar 19, 2021

we're expanding on our testing story in 2021, but the test phase now exists in skaffold, so i'll close this issue.

@nkubala nkubala closed this as completed Mar 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing Issues concerning the testing phase of Skaffold kind/feature-request priority/p2 May take a couple of releases
Projects
None yet
Development

No branches or pull requests

7 participants