Proposal: Refactoring of KnParameters to an interface
- Dominant language
- Go
- Stars
- 387
- Forks
- 274
- Avg merge
- 1d 6h
- Merged PRs (30d)
- 8
Description
Curently we are using a struct `KnParams` as the _context_ that we pass down into specific commands so that they can access the various services:
```golang
type KnParams struct {
Output io.Writer
KubeCfgPath string
ClientConfig clientcmd.ClientConfig
NewServingClient func(namespace string) (clientservingv1.KnServingClient, error)
NewSourcesClient func(namespace string) (v1alpha2.KnSourcesClient, error)
NewEventingClient func(namespace string) (clienteventingv1beta1.KnEventingClient, error)
NewDynamicClient func(namespace string) (clientdynamic.KnDynamicClient, error)
// General global options
LogHTTP bool
// Set this if you want to nail down the namespace
fixedCurrentNamespace string
}
```
This struct was mainly added in this form for allowing injection of fake clients for unit testing. However, this construct has some drawbacks:
* It leaks testing concerns into the business logic
* Anytime something you need for testing is added here (see `fixedCurrentNamespace` or also the `Output` writer which should be used from the command itself.
* Test initialization is done lazy with a side-effect in `Initialize()` which is hard to understand why that call is needed from business logic PoV
* `KnParameters` is a poor name because it does not reflect its purpose (i.e. the services included are not parameters, but part of the environment/context for which a command is created).
The suggested refactoring is:
* Move to a `KnContext` interface with direct access to the services needed.
* Provide an implementation `KnDefaultContext`, `KnFakeContext` (for fake based testing) and `KnMockContext` (for mock based testing)
* Consider to also move the output streams out & error to the context, too, so that any output goes over that streams. Test implementations `KnFakeContext` and `KnMockContext` the can use a buffer for these streams.
* The context remains the single constructor parameter for a command.
Please vote for this refactoring here with 👍 / 👎 , preferably with some comment to reason about your vote.
Contributor guide
Assessment
This issue has not been assessed yet.