Remove Kubebuilder Generated Tests

[kensipe]

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:
/kind cleanup
/kind enhancement

What this PR does / why we need it:
Removes generated kubebuilder tests and TestMains

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

[zen-dog]

I think that kubebuilder test harness actually brings a lot of value to the table. I wouldn't know how to mock the API server (or if it is even possible) and relying solely on SI tests is IMHO not an option. If anything, we should add more tests, especially for the controllers.

[jbarrick-mesosphere]

I think that kubebuilder test harness actually brings a lot of value to the table. I wouldn't know how to mock the API server (or if it is even possible) and relying solely on SI tests is IMHO not an option. If anything, we should add more tests, especially for the controllers.

@zen-dog we do have efforts to increase our integration testing landing nearly concurrently to this (see KEP-0008 and KEP-0004), this is simply removing autogenerated tests that do nothing except for start the control plane and cause flakiness.

[zen-dog]

@zen-dog we do have efforts to increase our integration testing landing nearly concurrently to this (see KEP-0008 and KEP-0004), this is simply removing autogenerated tests that do nothing except for start the control plane and cause flakiness.

Two things:

1. I believe relying solely on the SI is the wrong approach. These tests require provisioning actual clusters and are slow and expensive. As a general rule of thumb, one should test code at the lowest level possible.
2. I don't have issues with removing the autogenerated stubs. However, I'm working on an instance_controller_test.go as we speak, so whatever issue @kensipe has with build tags and those stubs - won't we have it again, once I merge my test?

See this #363 for more context.

[jbarrick-mesosphere]

I believe relying solely on the SI is the wrong approach. These tests require provisioning actual clusters and are slow and expensive. As a general rule of thumb, one should test code at the lowest level possible.

Yeah.... See: https://github.com/kudobuilder/kudo/blob/master/keps/0004-add-testing-infrastructure.md#tests

The test harness itself supports integration (using only a mocked etcd and kubernetes API) as well as end-to-end tests (for testing frameworks). You can also write Go-based integration and unit tests.

I don't have issues with removing the autogenerated stubs. However, I'm working on an instance_controller_test.go as we speak, so whatever issue @kensipe has with build tags and those stubs - won't we have it again, once I merge my test?

Nothing prevents you from writing these tests, this PR only removes these autogenerated stubs. They currently start the envtest environment for each of these stubs (each takes around 5 seconds), so it just adds wasted time as well as making it more likely for us to hit issues like #302.

We should of course fix flakes as they come up, but keeping these stubs around is not the way to do that IMO.

[jbarrick-mesosphere]

I don't have issues with removing the autogenerated stubs. However, I'm working on an instance_controller_test.go as we speak, so whatever issue @kensipe has with build tags and those stubs - won't we have it again, once I merge my test?

We have some tests that do this already that are not being removed: https://github.com/kudobuilder/kudo/blob/master/pkg/test/utils/kubernetes_test.go#L55

Though, note that your test could be written as a regular integration test using the KUDO test harness and doesn't actually need to be written in Go, see:

• Our KEP describing testing procedures: https://github.com/kudobuilder/kudo/blob/master/keps/0004-add-testing-infrastructure.md#integration-tests
• And the example tests written here: https://github.com/kudobuilder/kudo/tree/master/pkg/test/test_data

Those integration tests are run against the envtest environment and not a Kubernetes cluster.

[zen-dog]

hey currently start the envtest environment for each of these stubs (each takes around 5 seconds),

Agreed, let's remove the "unused" TestMains. However, my PR uses the one in instance_controller_test_suite so let's leave the instance controller tests out to not produce unnecessary conflicts /cc @kensipe

[jbarrick-mesosphere]

I'd prefer if we could use the declarative test harness for writing your test. And leave this PR as is.