Separate E2E and operator tests #1540
Conversation
nfnt
force-pushed
the
nfnt/separate-e2e-operator-tests
branch
2 times, most recently
from
9854fb2
to
d145b28
Compare
This runs operator tests as a separate test in parallel with the other tests. It makes operator tests independent from E2E test results and their failures distinguishable. Signed-off-by: Jan Schlicht <jan@d2iq.com>
nfnt
force-pushed
the
nfnt/separate-e2e-operator-tests
branch
from
d145b28
to
2a06d7a
Compare
|
I verified that test logs and artifacts are collected for E2E tests as well as operator tests. |
| @@ -23,6 +23,17 @@ jobs: | |||
| - store_artifacts: | |||
| path: kind-logs.tar.bz2 | |||
|
|
|||
| operator-test: | |||
There was a problem hiding this comment.
nit:
| operator-test: | |
| operators-test: |
|
could we get clarity on what is expected in each of these categories of tests? unit, integration, e2e and operator? If we are going expand the category of testing options... should be document the purpose and guide the community? |
|
Operator tests have the same setup but different concerns from E2E tests. The motivation is to get earlier feedback when these tests fail. In the past, the operator tests were only run if the E2E tests succeeded. This made it hard for larger PRs to distinguish between changes that fix operators and changes that fix E2E tests. |
| set -o xtrace | ||
|
|
||
| INTEGRATION_OUTPUT_JUNIT=${INTEGRATION_OUTPUT_JUNIT:-false} | ||
| KUDO_VERSION=${KUDO_VERSION:-test} |
There was a problem hiding this comment.
we need documentation in these scripts that there are preconditions / expectations..
I don't understand the default here... if no KUDO_VERSION is set, shouldn't the script fail? what are we testing if this value isn't properly provided?
There was a problem hiding this comment.
This is to allow users to override KUDO_VERSION. The default value of test is used as a tag for the Docker image that is build and injected to the kind cluster. This is to avoid clashes with existing Docker images as test is only used in this context.
| INTEGRATION_OUTPUT_JUNIT=${INTEGRATION_OUTPUT_JUNIT:-false} | ||
| KUDO_VERSION=${KUDO_VERSION:-test} | ||
|
|
||
| docker build . \ |
There was a problem hiding this comment.
why is there a docker build in this script? shouldn't this be part of the precondition of this script? don't we already have a build for docker in make?
docker build is now in 3+ locations now.. can we consolidate?
There was a problem hiding this comment.
perhaps this docker build is slightly different... where it includes run-test-* scripts. making a need for a different build.
there is a lot of commonality between the run-test scripts and even in 1 script there is duplication that could be cleaned up. It is hard to see if consolidation will make things better or not... I would hate jumping from file to file to track down something simple... today that could mean 1. circle-ci file, 2. makefile, 3. run-script file
The line sed "s/%version%/$KUDO_VERSION/" kudo-e2e-test.yaml.tmpl > kudo-e2e-test.yaml why it has different names across these files... it doesn't need to and they all do the same thing.
I don't see any reason to change the file name to match "operator" tests vs e2e. "operator" tests is another grouping of e2e tests with a different purpose is what I imagine without more clarification.
There was a problem hiding this comment.
This has to do with the test setup that is working around limitations in CircleCI. run-tests.sh starts a Docker container that can run Docker-in-Docker. This container then runs a test script in hack/, e.g. hack/run-operator-tests.sh in this case. The script builds a Docker image of the controller containing the changes of the PR under test and injects this into the kind cluster.
I don't like this complicated test setup and tried to refactor it to be simpler but ran into limitations in CircleCI: We have to run the tests in a VM environment to be able to start privileged containers in this VM. But we also need to install prerequisites for our tests in this VM and the easiest way is trough Docker, using the test/Dockerfile. That's why we have this complicated Docker-in-Docker setup here.
Signed-off-by: Jan Schlicht <jan@d2iq.com>
Signed-off-by: Ken Sipe <kensipe@gmail.com>
| rm -rf operators | ||
| git clone https://github.com/kudobuilder/operators | ||
| mkdir operators/bin/ | ||
| cp ./bin/kubectl-kudo operators/bin/ |
Signed-off-by: Ken Sipe <kensipe@gmail.com>
What this PR does / why we need it:
This runs operator tests as a separate test in parallel with the other tests. It makes operator tests independent from E2E test results and their failures distinguishable.
Fixes #1539