search

infinispan / infinispan-operator

Infinispan Operator
https://infinispan.org/docs/infinispan-operator/main/operator.html
Apache License 2.0
48 stars 54 forks source link

Convert golang e2e test to unit tests #229

Open Crumby opened 4 years ago

Crumby commented 4 years ago

Issue: Current GoLand e2e tests used within CI pipeline spin up Minikube and go through process of redeploying Infinispan cluster using Operator per test case which is causing long tests execution per PR.

Proposal: Reimplement the tests as unit tests that'd be sending actual and desired state to the Operator/Reconciler. The resulting operations from the Operator/Reconciler would be then evaluated against expected output.

rigazilla commented 4 years ago

Sorry if I'm late on this but most of the e2e tests also check if the actual Infinispan cluster is working properly. Part of the .Status field is also build inspecting the real status. Can we really do these tests without deploying the cluster?

dmvolod commented 4 years ago

Yes, @rigazilla , did the same things and almost done for mostly tests, except cases where Reconciling process where pod's data is accessing. All cases could be done with fake Kubernetes and rest client, but still not found solution with mocking ExecWithOptions invocation.

rigazilla commented 4 years ago

@dmvolod I probably need to see a prototype. I'm a little bit scared to not run tests against a real cluster :)

Crumby commented 4 years ago

@dmvolod @rigazilla I don't think it really makes sense convert all the tests in the e2e module into unit ones.

Main point of unit tests should be verification of Reconciler. Verifying that it correctly reacts on inputs it gets. We want to verify that Reconciler will correctly reacts on different CRs creation, updates and deletions. Well suitable tests from the e2e module at this moment are for example all "update tests". Testing if cluster really forms or if data will stay persisted is in my opinion outside of its scope and belongs to e2e/integration tests.

We still want to have some smoke tests that will verify that cluster spins up properly on Minikube so we can include at least some base verification into the CI/CD pipeline.

For the rest of tests that are not suitable for unit execution my suggestion would be to make them more complex so we don't need to spin up and down so many clusters during tests execution. TestNodeStartup is basically subtest of TestClusterFormation. Only difference is we are verifying that Reconciler creates StatefulSet with one replica. That can be done via unit testing. Same way TestClusterFormation(|WithTls) could be merged into integration tests (once the suite'll be able to run against Minikube) to reduce number of cluster spin ups and down during execution.

rigazilla commented 4 years ago

@Crumby thanks for the clarification, I was confused by the title of the issue and I though the proposal was more "radical". I generally agree with you, specially for tests that works on basic features with well defined I/O (TestNodeStartup is a good example). Some of the tests are testing things outside the operator boundaries and actually I find them useful in this phase, but yeah if we want to grow the testsuite we need to narrow the scope and focus on unit tests.

rigazilla commented 4 years ago

I think that @dmvolod #284 could be a good starting point to work on the unit test approach

dmvolod commented 4 years ago

Yes, completely agree with @Crumby and @rigazilla . We will use a number of tests against fake cluster and keep some important and complex e2e test.