[Chore]: Standardize how unit and integration tests are organized

[matt-royal]

Background

There's some disagreement on the team regarding this work, so it's blocked until we've solicited feedback

Some of our packages have both unit and integration tests, and generally we place the unit tests in the package itself and the integration-style tests in a nested package called integration. The exception is repositories, where we only have integration tests, though https://github.com/cloudfoundry/cf-k8s-controllers/issues/601 will address this.

For most of these packages we use the integration-style tests to drive out the behavior and only test edge cases and error cases in the unit tests. For this reason, it would be more intuitive to place the integration-style tests in the package itself and have the unit tests in a nested package.

Action to take

Reorganize and standardize our tests across the codebase so that integration tests live in the same package as the code and unit tests live in a nested package with a consistent name (it's up to the pair what that name is).

Impact

This will make our testing structure more intuitive

Dev Notes

No response

[davewalter]

integration tests live in the same package as the code and unit tests live in a nested package with a consistent name

Personally, I feel like it is more idiomatic for the unit tests to live with the code, and the integration tests to live in a nested package.

[gcapizzi]

Some thoughts:

• I don't think we should default to integration tests: I'd rather us default to unit tests and only write integration tests for the bits that actually integrate with third parties (in our case that's just the repositories).
• For the same reason, I think we should actually try to get rid of integration tests that overlap with our unit tests, e.g. the handlers integration tests.
• I don't think we should try to unit test the repositories: that would involve mocking Kubernetes behaviour, which we don't own, and that's a bad idea.

In other words, if we imagine our system as something like this:

Then we want to:

• unit test all objects in the core: this could be our handlers (although they are actually exposed via HTTP, they are already abstracted enough from it and I don't think they deserve integration tests for that);
• integration test all objects at the boundary: this could be our repositories;
• end-to-end test the whole system by only exercising its public API, to prove that all objects are wired together correctly and to have something to rely on for large scale refactorings.

This would result in a test suite that is fast and robust, as we'd be minimising the amount of slow and brittle tests while maintaining a high level of confidence.

[davewalter]

I don't think we should try to unit test the repositories

The main reason I want to unit test the repositories is to allow us to test code that handles errors from the K8s API client that we cannot currently test due to an inability to reliably cause it to fail when communicating with a real K8s API server/etcd.

[gcapizzi]

I have seen some proposed techniques to make some failures occur (e.g. passing expired contexts), so we could try those but in general I don't think it is worth it - we know we're handling the errors and we have linters checking that too, we can test the paths that are easy to reach (which should be 99.99%) and leave the others untested. The risk of mocking Kubernetes incorrectly and getting false positives is far higher in my opinion.

[georgethebeatle]

It feels like this chore is out of date and does not reflect what we have agreed on after discussing the topic for a while. This agreement is summarized in this proposal. Should we close this one?

[julian-hj]

I was going to upvote the idea to close it, but instead I will just close the thing. Anyone please feel free to re-open if you don't agree that was the right thing to do.