Migrate unit tests to use gomock.

[amshuman-kr]

How to categorize this PR?

/area control-plane
/kind technical-debt

What this PR does / why we need it:

Migrate all controller unit tests to use gomock framework. I have also tried to make the test flow easier to understand, to contribute and maintain.

Which issue(s) this PR fixes:
Fixes #97

Special notes for your reviewer:

This is in preparation for #186 so that any regressions can be caught early in the unit tests.
Apologies for large changes to etcd_controller_test.go but it made sense to rationalise test flow. I have also added additional tests in the deletion use-case as well many other permutations and combinations for regular reconciliation.

Release note:

NONE

[amshuman-kr]

The controller suite test execution time has come down from 2m35s to 2.79s.

[amshuman-kr]

/hold It would be a good idea to merge this PR after #197 because it has some more changes to reconciliation and controller tests which I would prefer to cover in the migration to Gomock (in that case, I will need to rebase and migrate the additional tests from #197).

[gardener-robot]

@shreyas-s-rao, @abdasgupta, @ishan16696 You have pull request review open invite, please check

[amshuman-kr]

/unhold I have rebased the PR after #197 was merged. I have also migrated the additional tests from #197.

[timuthy]

/assign

[amshuman-kr]

I have rebased after the merge of #202 which conflicted with this PR.

[amshuman-kr]

Do you also want to remove the envtest tool from .ci/test?

It is still used in api_tests. I retained it just to verify if the CRDs are generated and working properly.

The test cover many different use-cases which is also the reason there is a lot of nesting. As a reviewer it's not trivial to understand or as a developer not easy to maintain this with all the different mocks and BeforeEach()/AfterEachs being involved.

The existing tests covered whole reconciliation. Wanting to be sure that I still cover all the existing test cases after the migration, I retained the migrated tests also to cover whole of reconciliation. But I used nesting to introduce more variations in the test cases by piecemeal nesting and also to avoid too much duplication of the test setup.

But I can imagine that these tests will help us to extract functionality into smaller junks, especially from etcd_controller.go with a lower risk of introducing regressions while doing so. I really hope that we're going to go that way with the Druid :)

Indeed, the goal is to modularise the reconciliation flow and to be able to then test these smaller functions independently. This way we should also be able to reduce the nesting of test blocks into independent test descriptions. I think this is best done over a few more PRs than in one single PR, if only to make sure we are not regressing or worse, losing test cases that were already covered.

[amshuman-kr]

@timuthy Thanks for the review! I have addressed your comments. Can you PTAL?

[timuthy]

/lgtm
Thank you 👍

[amshuman-kr]

Rebased and resolved conflicts. Made the gmock based tests to work for the lease listing changes introduced in #214.

[amshuman-kr]

Rebased after the v0.6.0 release.

[gardener-robot]

@amshuman-kr You need rebase this pull request with latest master branch. Please check.

[timuthy]

Let's close this in favor of #278 which will allow us to have sufficient coverage for single components (+ easy maintenance as opposed to maintaining plenty of Gomock assertions) and only a handful of envtest like tests which test the controller from end-to-end.
/close