Closed glyn closed 4 years ago
As discussed on projectriff.slack.com, I am overall a bit skeptical about the matrix approach for testing, as the number of attributes of the test cases seem to grow over time to a point where it sometimes becomes hard to understand which attribute to use or not for a particular situation.
Moreover, the context is spread between the custom test runner and the relevant test case one is interested in. It may be difficult to understand a test case without first understanding what the custom test runner does.
It is also not possible to improve a particular matrix test case. Structural changes apply to all of them.
In my opinion, “traditional” tests offer a greater flexibility, with a smaller learning curve (I assume most developers are familiar with given-when-then structures + before/after hooks).
Maybe there is some solution to be found in the middle, i.e. standard enough so that we do not duplicate code for common tasks and flexible enough to allow local one-off variations without surfacing these subtleties to the general "framework".
@fbiville The table approach seems to be working out quite well so far in that the test framework seems to be reusable across controllers. If we had to add features to the framework for particular controllers, that would be more of a concern.
My main concern about the table approach was that introducing minor variations on existing "rows" would result in a lot of duplication between rows, but that is being avoided by using helper functions to avoid excessive duplication. For example, @scothis has introduced a nice collection of helper functions for constructing certain common objects, in pkg/controllers/testing/factories
.
I share your concern about the table approach being a little impenetrable for newcomers, but I think that's mitigated by using the same test framework across all the controller tests. Also, I think the table tests are pretty easy to read once you've grokked the general design of reconcilers (admittedly a bit of a hurdle for newcomers). I think using non-table tests wouldn't really help much as there is a lot of machinery associated with testing controllers and non-table tests would be more likely to diverge from each other in the use of such machinery.
This is similar to the pattern we follow for testing commands in the CLI. Which it should because the CLI tests are modeled after the Knative reconciler tests. The main difference is that the trigger is a reconciliation key rather than a cli invocation and in our case, the client interface is different since we're building on controller-runtime.
See: https://github.com/projectriff/cli/blob/master/pkg/testing/command_table.go
Usage: https://github.com/projectriff/cli/blob/master/pkg/build/commands/function_create_test.go#L282-L1103