Skip to content

Commit

Permalink
enhance with docs
Browse files Browse the repository at this point in the history
  • Loading branch information
Brian Mendoza committed Apr 22, 2024
1 parent f8402b2 commit 877f768
Show file tree
Hide file tree
Showing 8 changed files with 408 additions and 198 deletions.
46 changes: 23 additions & 23 deletions controller/linodeobjectstoragebucket_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,11 +184,11 @@ var _ = Describe("lifecycle", Ordered, Label("bucket", "lifecycle"), func() {
Expect(key.StringData.AccessKeyRO).To(Equal("access-key-1"))
Expect(key.StringData.SecretKeyRO).To(Equal("secret-key-1"))

Expect(mck.Events()).To(ContainSubstring("Object storage keys assigned"))
Expect(mck.Events()).To(ContainSubstring("Object storage keys stored in secret"))
Expect(mck.Events()).To(ContainSubstring("Object storage bucket synced"))
Expect(suite.Events()).To(ContainSubstring("Object storage keys assigned"))
Expect(suite.Events()).To(ContainSubstring("Object storage keys stored in secret"))
Expect(suite.Events()).To(ContainSubstring("Object storage bucket synced"))

logOutput := mck.Logs()
logOutput := suite.Logs()
Expect(logOutput).To(ContainSubstring("Reconciling apply"))
Expect(logOutput).To(ContainSubstring("Secret lifecycle-bucket-details was applied with new access keys"))
}),
Expand Down Expand Up @@ -252,9 +252,9 @@ var _ = Describe("lifecycle", Ordered, Label("bucket", "lifecycle"), func() {
Expect(k8sClient.Get(ctx, objectKey, &obj)).To(Succeed())
Expect(*obj.Status.LastKeyGeneration).To(Equal(1))

Expect(mck.Events()).To(ContainSubstring("Object storage keys assigned"))
Expect(suite.Events()).To(ContainSubstring("Object storage keys assigned"))

logOutput := mck.Logs()
logOutput := suite.Logs()
Expect(logOutput).To(ContainSubstring("Reconciling apply"))
Expect(logOutput).To(ContainSubstring("Secret lifecycle-bucket-details was applied with new access keys"))
}),
Expand Down Expand Up @@ -308,11 +308,11 @@ var _ = Describe("lifecycle", Ordered, Label("bucket", "lifecycle"), func() {
Expect(key.StringData.AccessKeyRO).To(Equal("access-key-3"))
Expect(key.StringData.SecretKeyRO).To(Equal("secret-key-3"))

Expect(mck.Events()).To(ContainSubstring("Object storage keys retrieved"))
Expect(mck.Events()).To(ContainSubstring("Object storage keys stored in secret"))
Expect(mck.Events()).To(ContainSubstring("Object storage bucket synced"))
Expect(suite.Events()).To(ContainSubstring("Object storage keys retrieved"))
Expect(suite.Events()).To(ContainSubstring("Object storage keys stored in secret"))
Expect(suite.Events()).To(ContainSubstring("Object storage bucket synced"))

logOutput := mck.Logs()
logOutput := suite.Logs()
Expect(logOutput).To(ContainSubstring("Reconciling apply"))
Expect(logOutput).To(ContainSubstring("Secret lifecycle-bucket-details was applied with new access keys"))
}),
Expand Down Expand Up @@ -348,9 +348,9 @@ var _ = Describe("lifecycle", Ordered, Label("bucket", "lifecycle"), func() {
Expect(err).NotTo(HaveOccurred())
Expect(apierrors.IsNotFound(k8sClient.Get(ctx, objectKey, &obj))).To(BeTrue())

Expect(mck.Events()).To(ContainSubstring("Object storage keys revoked"))
Expect(suite.Events()).To(ContainSubstring("Object storage keys revoked"))

logOutput := mck.Logs()
logOutput := suite.Logs()
Expect(logOutput).To(ContainSubstring("Reconciling delete"))
}),
),
Expand Down Expand Up @@ -411,7 +411,7 @@ var _ = Describe("errors", Label("bucket", "errors"), func() {
NamespacedName: client.ObjectKeyFromObject(bScope.Bucket),
})
Expect(err.Error()).To(ContainSubstring("non-404 error"))
Expect(mck.Logs()).To(ContainSubstring("Failed to fetch LinodeObjectStorageBucket"))
Expect(suite.Logs()).To(ContainSubstring("Failed to fetch LinodeObjectStorageBucket"))
}),
),
),
Expand All @@ -422,7 +422,7 @@ var _ = Describe("errors", Label("bucket", "errors"), func() {
NamespacedName: client.ObjectKeyFromObject(bScope.Bucket),
})
Expect(err.Error()).To(ContainSubstring("failed to create object storage bucket scope"))
Expect(mck.Logs()).To(ContainSubstring("Failed to create object storage bucket scope"))
Expect(suite.Logs()).To(ContainSubstring("Failed to create object storage bucket scope"))
}),
Call("scheme with no infrav1alpha1", func(ctx context.Context, mck Mock) {
prev := mck.K8sClient.EXPECT().Scheme().Return(scheme.Scheme)
Expand Down Expand Up @@ -456,8 +456,8 @@ var _ = Describe("errors", Label("bucket", "errors"), func() {
bScope.Client = mck.K8sClient
err := reconciler.reconcileApply(ctx, &bScope)
Expect(err.Error()).To(ContainSubstring("api error"))
Expect(mck.Events()).To(ContainSubstring("api error"))
Expect(mck.Logs()).To(ContainSubstring("Failed to ensure access key secret exists"))
Expect(suite.Events()).To(ContainSubstring("api error"))
Expect(suite.Logs()).To(ContainSubstring("Failed to ensure access key secret exists"))
}),
),
Call("secret deleted", func(ctx context.Context, mck Mock) {
Expand Down Expand Up @@ -485,9 +485,9 @@ var _ = Describe("errors", Label("bucket", "errors"), func() {
bScope.Client = mck.K8sClient
err := reconciler.reconcileApply(ctx, &bScope)
Expect(err.Error()).To(ContainSubstring("secret creation error"))
Expect(mck.Events()).To(ContainSubstring("keys retrieved"))
Expect(mck.Events()).To(ContainSubstring("secret creation error"))
Expect(mck.Logs()).To(ContainSubstring("Failed to apply key secret"))
Expect(suite.Events()).To(ContainSubstring("keys retrieved"))
Expect(suite.Events()).To(ContainSubstring("secret creation error"))
Expect(suite.Logs()).To(ContainSubstring("Failed to apply key secret"))
}),
),
Path(
Expand All @@ -504,9 +504,9 @@ var _ = Describe("errors", Label("bucket", "errors"), func() {
bScope.Client = mck.K8sClient
err := reconciler.reconcileApply(ctx, &bScope)
Expect(err.Error()).To(ContainSubstring("no kind is registered"))
Expect(mck.Events()).To(ContainSubstring("keys retrieved"))
Expect(mck.Events()).To(ContainSubstring("no kind is registered"))
Expect(mck.Logs()).To(ContainSubstring("Failed to generate key secret"))
Expect(suite.Events()).To(ContainSubstring("keys retrieved"))
Expect(suite.Events()).To(ContainSubstring("no kind is registered"))
Expect(suite.Logs()).To(ContainSubstring("Failed to generate key secret"))
}),
),
),
Expand All @@ -522,7 +522,7 @@ var _ = Describe("errors", Label("bucket", "errors"), func() {
bScope.Client = mck.K8sClient
err := reconciler.reconcileDelete(ctx, &bScope)
Expect(err.Error()).To(ContainSubstring("failed to remove finalizer from bucket"))
Expect(mck.Events()).To(ContainSubstring("failed to remove finalizer from bucket"))
Expect(suite.Events()).To(ContainSubstring("failed to remove finalizer from bucket"))
}),
))
})
171 changes: 171 additions & 0 deletions docs/src/developers/testing.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,177 @@ In order to run the unit tests run the following command
```bash
make test
```
### Adding tests
General unit tests of functions follow the same conventions for testing using Go's `testing` standard library, along with the [testify](https://github.com/stretchr/testify) toolkit for making assertions.

Unit tests that require API clients use mock clients generated using [gomock](https://github.com/uber-go/mock). To simplify the usage of mock clients, this repo also uses an internal library defined in `mock/mocktest`.

`mocktest` is usually imported as a dot import along with the `mock` package:

```go
import (
"github.com/linode/cluster-api-provider-linode/mock"

. "github.com/linode/cluster-api-provider-linode/mock/mocktest"
)
```

Using `mocktest` involves creating a test suite that specifies the mock clients to be used within each test scope and running the test suite using a DSL for defnining test nodes belong to one or more `Paths`.

#### Example
The following is a contrived example using the mock Linode machine client.

Let's say we've written an idempotent function `EnsureInstanceRuns` that 1) gets an instance or creates it if it doesn't exist, 2) boots the instance if it's offline. Testing this function would mean we'd need to write test cases for all permutations, i.e.
* instance exists and is not offline
* instance exists but is offline, and is able to boot
* instance exists but is offline, and is not able to boot
* instance does not exist, and is not able to be created
* instance does not exist, and is able to be created, and is able to boot
* instance does not exist, and is able to be created, and is not able to boot

While writing test cases for each scenario, we'd likely find a lot of overlap between each. `mocktest` provides a DSL for defining each unique test case without needing to spell out all required mock client calls for each case. Here's how we could test `EnsureInstanceRuns` using `mocktest`:

```go
func TestEnsureInstanceNotOffline(t *testing.T) {
suite := NewTestSuite(t, mock.MockLinodeMachineClient{})

suite.Run(t, Paths(
Either(
Path(
Call("instance exists and is not offline", func(ctx context.Context, mck Mock) {
mck.MachineClient.EXPECT().GetInstance(ctx, /* ... */).Return(&linodego.Instance{Status: linodego.InstanceRunning}, nil)
}),
Result("success", func(ctx context.Context, mck Mock) {
inst, err := EnsureInstanceNotOffline(ctx, /* ... */)
require.NoError(t, err)
assert.Equal(t, inst.Status, linodego.InstanceRunning)
})
),
Path(
Call("instance does not exist", func(ctx context.Context, mck Mock) {
mck.MachineClient.EXPECT().GetInstance(ctx, /* ... */).Return(nil, linodego.Error{Code: 404})
}),
Either(
Call("able to be created", func(ctx context.Context, mck Mock) {
mck.MachineClient.EXPECT().CreateInstance(ctx, /* ... */).Return(&linodego.Instance{Status: linodego.InstanceOffline}, nil)
}),
Path(
Call("not able to be created", func(ctx context.Context, mck Mock) {/* ... */})
Result("error", func(ctx context.Context, mck Mock) {
inst, err := EnsureInstanceNotOffline(ctx, /* ... */)
require.ErrorContains(t, err, "instance was not booted: failed to create instance: reasons...")
assert.Empty(inst)
})
)
),
),
Call("instance exists but is offline", func(ctx context.Context, mck Mock) {
mck.MachineClient.EXPECT().GetInstance(ctx, /* ... */).Return(&linodego.Instance{Status: linodego.InstanceOffline}, nil)
}),
),
Either(
Path(
Call("able to boot", func(ctx context.Context, mck Mock) {/* */})
Result("success", func(ctx context.Context, mck Mock) {
inst, err := EnsureInstanceNotOffline(ctx, /* ... */)
require.NoError(t, err)
assert.Equal(t, inst.Status, linodego.InstanceBooting)
})
),
Path(
Call("not able to boot", func(ctx context.Context, mck Mock) {/* returns API error */})
Result("error", func(ctx context.Context, mck Mock) {
inst, err := EnsureInstanceNotOffline(/* options */)
require.ErrorContains(t, err, "instance was not booted: boot failed: reasons...")
assert.Empty(inst)
})
)
),
)
}
```
In this example, the nodes passed into `Paths` are used to describe each permutation of the function being called with different results from the mock Linode machine client.

#### Nodes
* `Call` describes the behavior of method calls by mock clients. A `Call` node can belong to one or more paths.
* `Result` invokes the function with mock clients and tests the output. A `Result` node terminates each path it belongs to.
* `Path` is a list of nodes that all belong to the same test path. Each child node of a `Path` is evaluated in order.
* `Either` is a list of nodes that all belong to different test paths. It is used to define diverging test path, with each path containing the set of all preceding `Call` nodes.

#### Setup, tear down, and event triggers
Setup and teardown nodes can be scheduled before and after each run:
* `suite.BeforeEach` receives a `func(context.Context, Mock)` function that will run before each path is evaluated. Likewise, `suite.AfterEach` will run after each path is evaluated.
* `suite.BeforeAll` receives a `func(context.Context, Mock)` function taht will run once before all paths are evaluated. Likewise, `suite.AfterEach` will run after each path is evaluated.

In addition to the path nodes listed in the section above, a special node type `Once` may be specified to inject a function that will only be evaluated one time across all paths. It can be used to trigger side effects outside of mock client behavior that can impact the output of the function being tested.

#### Control flow
When `Run` is called on a test suite, paths are evaluated in parallel using `t.Parallel()`. Each path will be run with a separate `t.Run` call, and each test run will be named according to the descriptions specified in each node.

To help with visualizing the paths that will be rendered from nodes, a `Describe` helper method can be called which returns a slice of strings describing each path. For instance, the following shows the output of `Describe` on the paths described in the example above:

```go
paths := Paths(/* see example above */)

paths.Describe() /* [
"instance exists and is not offline > success",
"instance does not exist > not able to be created > error",
"instance does not exist > able to be created > able to boot > success",
"instance does not exist > able to be created > not able to boot > error",
"instance exists but is offline > able to boot > success",
"instance exists but is offline > not able to boot > error"
] */
```

#### Testing controllers
CAPL uses controller-runtime's [envtest](https://book.kubebuilder.io/reference/envtest) package which runs an instance of etcd and the Kubernetes API server for testing controllers. The test setup uses [ginkgo](https://onsi.github.io/ginkgo/) as its test runner as well as [gomega](https://onsi.github.io/gomega/) for assertions.

`mocktest` is also recommended when writing tests for controllers. The following is another contrived example of how to use it within the context of a Ginkgo `Describe` node:

```go
// Add the `Ordered` decorator so that tests run in order versus in parallel.
// This is needed when relying on EnvTest for managing Kubernetes API server state.
var _ = Describe("test name", Ordered, func() {
// Create a mocktest controller test suite.
suite := NewControllerTestSuite(GinkgoT(), mock.MockLinodeMachineClient{})

obj := infrav1alpha1.LinodeMachine{
ObjectMeta: metav1.ObjectMeta{/* ... */}
Spec: infrav1alpha1.LinodeMachineSpec{/* ... */}
}

suite.Run(Paths(
Once("create resource", func(ctx context.Context, _ Mock) {
// Use the EnvTest k8sClient to create the resource in the test server
Expect(k8sClient.Create(ctx, &obj).To(Succeed()))
})
Call("create a linode", func(ctx context.Context, mck Mock) {
mck.MachineClient.CreateInstance(ctx, gomock.Any(), gomock.Any()).Return(&linodego.Instance{/* ... */}, nil)
})
Result("update the resource with info about the linode", func(ctx context.Context, mck Mock) {
reconciler := LinodeMachineReconciler{
// Configure the reconciler to use the mock client for this test path
LinodeClient: mck.MachineClient,
// Use a managed recorder for capturing events published during this test
Recorder: suite.Recorder(),
// Use a managed logger for capturing logs written during the test
Logger: suite.Logger(), // Note: This isn't a real struct field. A logger is configured elsewhere.
}

_, err := reconciler.Reconcile(ctx, reconcile.Request{/* ... */})
Expect(err).NotTo(HaveOccurred())

// Fetch the updated object in the test server and confirm it was updated
Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(obj))).To(Succeed())
Expect(obj.Status.Ready).To(BeTrue())

// Check for expected events and logs
Expect(suite.Events()).To(ContainSubstring("Linode created!"))
Expect(suite.Logs()).To(ContainSubstring("Linode created!"))
})
))
})
```

## E2E Tests
For e2e tests CAPL uses the [Chainsaw project](https://kyverno.github.io/chainsaw) which leverages `kind` and `tilt` to
Expand Down
78 changes: 0 additions & 78 deletions mock/mocktest/controller_suite.go
View file

This file was deleted.

Loading

0 comments on commit 877f768

Please sign in to comment.