implement integration tests #96

hasheddan · 2019-11-15T19:30:06Z

Description of your changes

Initial implementation of basic integration tests that simply install CRDs, start the stack controller, and create instances of the classes defined in examples/

Checklist

I have:

Run make reviewable to ensure this PR is ready for review.
Ensured this PR contains a neat, self documenting set of commits.
Updated any relevant documentation, examples, or release notes.
Updated the dependencies in app.yaml to include any new role permissions.

upbound-bot · 2019-11-15T22:47:57Z

upbound-bot · 2019-11-19T18:02:55Z

negz

We have to install core Crossplane CRD's for the stack managers to be able to run. However, it is likely undesirable to have them actually exist in this repo as I do here. I would support adding the ability to specify remote locations in the CRD paths passed to athodyd.

Would anything discussed in crossplane/crossplane#1085 help with this at all? I agree supporting remote locations (I assume you mean for example HTTP) is preferable to duplicating them in each stack, though we'd still need to play the game of keeping the referenced CRD files aligned with the code level definitions we imported.

How often do we want these to be run initially? I would support once daily using a Jenkins pipeline to start out.

That seems reasonable. It would be nice to allow maintainers to trigger integration tests via Tiller-style commands, but we could add that later. I'd still like to see us running these tests (or a subset thereof) against fake cloud provider APIs for every PR, eventually.

Can we move all examples for each stack to their respective repos? That we we could actually run integration tests using the examples by creating resources from the raw yaml.

I think we should just do this. We'll need to check for and figure out a solution to references to examples from the documentation, though.

test/integration/createProvider_test.go

negz · 2019-11-20T01:41:46Z

test/integration/createProvider_test.go

+limitations under the License.
+*/
+
+package integration


Is there an advantage to having a distinct integration test package? Could we instead keep the integration tests closer to the controllers they're testing, and perhaps gate them behind a build tag?

I like this idea! However, I do think we will likely have many of our integration tests span multiple controllers, so we may want a general location as well. For instance, if we wanted to test deploying Wordpress on GCP or something on the more complex side like that (not intending to do so in initial implementation) it would be useful to have something like an e2e directory.

hasheddan · 2019-11-20T16:26:46Z

@negz while sorting out the dependencies for this implementation, I began to think that it may be useful to just include what is currently encompassed and will be implemented in the future in athodyd as a package in crossplane-runtime. This, along with some discussion on crossplane/crossplane#1085 suggests that stacks could eventually only depend on crossplane-runtime in the crossplane ecosystem. If the claim types are eventually included in crossplane-runtime that would also solve loading them into the test environment as we could just have an option of something like WithCrossplaneCRDs(). Any thoughts?

negz · 2019-11-21T19:48:03Z

Any thoughts?

I think it makes sense to include athodyd in crossplane-runtime, perhaps under pkg/test/integration or similar. I'm a little wary of introducing testing dependencies on a relatively new library in a personal org.

I don't think it's a sure thing that claims will move into crossplane-runtime. They fit better there than in crossplane core, but I'm not yet convinced they don't belong in a separate API package.

hasheddan · 2019-11-21T19:49:49Z

@negz sounds good! I'll open a PR to add current work in athodyd to crossplane-runtime :)

upbound-bot · 2019-11-22T16:41:38Z

upbound-bot · 2019-12-03T01:32:26Z

upbound-bot · 2019-12-10T23:24:43Z

upbound-bot · 2020-01-08T21:49:05Z

upbound-bot · 2020-01-09T00:41:09Z

upbound-bot · 2020-01-09T00:50:57Z

upbound-bot · 2020-01-09T01:03:28Z

Signed-off-by: hasheddan <georgedanielmangum@gmail.com>

upbound-bot · 2020-01-09T20:13:51Z

hasheddan

@negz @jbw976 I think this is ready. For the time being, we are utilizing a long-running cluster and the tests are only run ad-hoc via user invocation on Jenkins. While it is desirable to spin up net new clusters for integration tests in the future (probably using Crossplane), we can continue to build out the tests in the actual repos while that system is being designed, then switch over when ready without having to rewrite.

hasheddan · 2020-01-09T20:10:52Z

test/integration/create_classes_test.go

+	cfg, err := clientcmd.BuildConfigFromFlags("", "../../kubeconfig.yaml")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if err := os.Setenv("GOOGLE_APPLICATION_CREDENTIALS", "../../sa.json"); err != nil {


Note: we look for these specific files that Jenkins drops into the project directory. Falling back on local kubeconfig was discussed but given that these test could eventually spin up new infrastructure, I actually like that a specific file path is hard coded.

negz

@hasheddan I'm not sure how I feel about these integration tests being defined so separately from the code they're testing. That said, I'm conscious that this PR has been open a long time so I'd feel comfortable merging it and then assessing whether we should refactor it a little later.

negz · 2020-01-13T19:42:24Z

test/integration/create_classes_test.go

+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/


Should we use a build tag to ensure these tests are not run by folks attempting to run unit tests?

https://mickey.dev/posts/go-build-tags-testing/

Yes, great point! I will add that. Another safe-guard here is the way that the k8s client is configured: you have to literally drop a kubeconfig.yaml into the project dir, it will not just use your local configuration automatically.

negz · 2020-01-13T19:46:15Z

test/integration/create_classes_test.go

+		"CreateProvider": {
+			reason: "A GCP Provider should be created without error.",
+			test: func(c client.Client) error {
+				dat, err := ioutil.ReadFile("../../examples/gcp-provider.yaml")


I like that we're testing our examples, but I'm a little worried that it might not be obvious to folks that these examples are tied to the integration tests. Is there something we could do to bring them closer together? How will we know which examples are used by integration tests and which are just examples?

Ideally we would always validate all examples in our integration tests. Therefore, if someone makes a change to examples/ then integration tests should also be updated. If we need to enforce this in the PR process that could be an option to make sure we do not get out of sync.

negz · 2020-01-13T19:47:29Z

test/integration/create_classes_test.go

+				return c.Create(context.Background(), p)
+			},
+		},
+		"CreateBucketClass": {


What is the thinking around having one integration test that creates all extant resource class kinds, vs having an integration test alongside each of the existing unit tests? Would it be better if we tested code closer to where it was defined, like Go unit tests do?

This create_classes_test.go is a bit of a one-off. I think that most tests will not necessarily make sense to be next to a specific piece of code like unit tests. For example, I have a local test that creates a Provider, GKECluster, CloudSQLInstance, KubernetesCluster claim, MySQLInstance claim, and a KubernetesApplication. I don't think there is a specific set of code that it should be associated with.

Signed-off-by: hasheddan <georgedanielmangum@gmail.com>

hasheddan · 2020-01-13T20:40:08Z

@negz updated with build tags, I want to manually execute Jenkins pipeline again before merge though

Signed-off-by: hasheddan <georgedanielmangum@gmail.com>

upbound-bot · 2020-01-13T20:57:53Z

hasheddan

@negz integration testing pipeline executed successfully: https://jenkinsci.upbound.io/blue/organizations/jenkins/crossplaneio%2Fstack-gcp-pipelines%2Fstack-gcp-integration/detail/PR-96/18/pipeline

hasheddan · 2020-01-13T20:50:41Z

test/integration/doc.go

+limitations under the License.
+*/
+
+package integration


Adding this doc.go was required for the generate target to run successfully.

anitsh · 2021-02-04T07:25:43Z

Hi @hasheddan,
The link examples leads to 404 page not found.

hasheddan force-pushed the inttestsinitial branch 3 times, most recently from c663b12 to 4a2b945 Compare November 15, 2019 22:39

hasheddan mentioned this pull request Nov 18, 2019

Bump controller-runtime to 0.4.0 crossplane/crossplane-runtime#84

Closed

4 tasks

hasheddan force-pushed the inttestsinitial branch from 4a2b945 to f0d87d0 Compare November 19, 2019 17:55

hasheddan requested a review from negz November 19, 2019 18:00

negz reviewed Nov 20, 2019

View reviewed changes

hasheddan force-pushed the inttestsinitial branch from f0d87d0 to 192d246 Compare November 22, 2019 16:33

hasheddan mentioned this pull request Nov 22, 2019

Add integration testing package crossplane/crossplane-runtime#89

Merged

4 tasks

hasheddan mentioned this pull request Dec 3, 2019

integration tests: support downloading remote CRDs crossplane/crossplane-runtime#91

Merged

4 tasks

hasheddan force-pushed the inttestsinitial branch from cc6a748 to df1e7ec Compare January 8, 2020 21:37

hasheddan force-pushed the inttestsinitial branch 2 times, most recently from bd46ff0 to a805f08 Compare January 9, 2020 19:28

test: implement initial integration tests and pipeline

b5ec4b4

Signed-off-by: hasheddan <georgedanielmangum@gmail.com>

hasheddan force-pushed the inttestsinitial branch from a805f08 to b5ec4b4 Compare January 9, 2020 20:04

hasheddan marked this pull request as ready for review January 9, 2020 20:05

hasheddan requested review from negz and jbw976 January 9, 2020 20:05

hasheddan commented Jan 9, 2020

View reviewed changes

negz approved these changes Jan 13, 2020

View reviewed changes

tests: use integration build tags

1dfa06b

Signed-off-by: hasheddan <georgedanielmangum@gmail.com>

test: add doc.go for integration pkg

af2f450

Signed-off-by: hasheddan <georgedanielmangum@gmail.com>

hasheddan commented Jan 13, 2020

View reviewed changes

negz merged commit 3690e4c into crossplane-contrib:master Jan 13, 2020

hasheddan mentioned this pull request Jan 14, 2020

Implement integration tests #87

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

implement integration tests #96

implement integration tests #96

hasheddan commented Nov 15, 2019 •

edited

Loading

upbound-bot commented Nov 15, 2019

upbound-bot commented Nov 19, 2019

negz left a comment

negz Nov 20, 2019

hasheddan Nov 20, 2019

hasheddan commented Nov 20, 2019

negz commented Nov 21, 2019

hasheddan commented Nov 21, 2019

upbound-bot commented Nov 22, 2019

upbound-bot commented Dec 3, 2019

upbound-bot commented Dec 10, 2019

upbound-bot commented Jan 8, 2020

upbound-bot commented Jan 9, 2020

upbound-bot commented Jan 9, 2020

upbound-bot commented Jan 9, 2020

upbound-bot commented Jan 9, 2020

hasheddan left a comment

hasheddan Jan 9, 2020

negz left a comment

negz Jan 13, 2020

hasheddan Jan 13, 2020

negz Jan 13, 2020

hasheddan Jan 13, 2020

negz Jan 13, 2020

hasheddan Jan 13, 2020

hasheddan commented Jan 13, 2020

upbound-bot commented Jan 13, 2020

hasheddan left a comment

hasheddan Jan 13, 2020

anitsh commented Feb 4, 2021

implement integration tests #96

implement integration tests #96

Conversation

hasheddan commented Nov 15, 2019 • edited Loading

Description of your changes

Checklist

upbound-bot commented Nov 15, 2019

upbound-bot commented Nov 19, 2019

negz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hasheddan commented Nov 20, 2019

negz commented Nov 21, 2019

hasheddan commented Nov 21, 2019

upbound-bot commented Nov 22, 2019

upbound-bot commented Dec 3, 2019

upbound-bot commented Dec 10, 2019

upbound-bot commented Jan 8, 2020

upbound-bot commented Jan 9, 2020

upbound-bot commented Jan 9, 2020

upbound-bot commented Jan 9, 2020

upbound-bot commented Jan 9, 2020

hasheddan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

negz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hasheddan commented Jan 13, 2020

upbound-bot commented Jan 13, 2020

hasheddan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anitsh commented Feb 4, 2021

hasheddan commented Nov 15, 2019 •

edited

Loading