[Chore]: Backfill unit tests for repositories package

[matt-royal]

Background

We've been feeling the pain of our integration-only strategy lately, and have been unable to test some common failure cases. We've already established patterns for testing the controllers with k8s client fakes, and we're confident we can do the same here.

Action to take

Find the // untested comments in the repositories package and backfill unit tests for those cases.

Impact

This will give us more confidence that we're properly implementing error handling, and also help us avoid regressions of untested behavior

Dev Notes

• Start with CFServiceInstance and write chores for others

[gcapizzi]

I would generally advice against mocking types we don't own, in this case the controller-runtime client. We'd be making assumptions about how the client (and the Kubernetes API) work, which might be wrong. It would allow us to test some particularly hard to reach error cases, but I could only find 3 instances of // untested in the repositories package, so maybe it's not that big of a deal? Overall I don't think it warrants the introduction of a new test suite.

[matt-royal]

Thank you, @gcapizzi. I'll discuss this with the AMER team today. I agree that, if we add these, it only makes sense to test the // untested branches.

[gcapizzi]

@matt-royal someone also pointed out that some // untested branches have been deleted because we have a coverage report now, not so easy to spot them via that webpage though 😕 I'm sure there's a way to extract all uncovered lines and extract error checks from them. We should probably be looking at all our uncovered lines anyway, but that is not necessarily related to this issue.

[julian-hj]

We created a POC to add unit level tests to the existing suite, but it didn't get a lot of support, so we abandoned it. @georgethebeatle has another approach to try by adding an interface layer on top of the k8s cliet, but for now we will close this issue.