Skip to content

Commit 14f6600

Browse files
address comments
1 parent 1945111 commit 14f6600

File tree

1 file changed

+19
-18
lines changed

1 file changed

+19
-18
lines changed

docs/book/src/developer/testing.md

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,11 @@ be fast and great for getting the first signal on the current implementation, bu
1212
allowing integration bugs to slip through.
1313

1414
In Cluster API most of the unit tests are developed using [go test], [gomega] and the [fakeclient]; however using
15-
[fakeclient] is not suitable for all the use cases due to some limitation derived by how it is implemented,
16-
so in some cases contributors will be required to use [envtest]. See the [quick reference](#quick-reference) below for more details.
15+
[fakeclient] is not suitable for all the use cases due to some limitations in how it is implemented. In some cases
16+
contributors will be required to use [envtest]. See the [quick reference](#quick-reference) below for more details.
1717

1818
### Mocking external APIs
19-
In some cases when writing tests it is required to mock external API, e.g. etcd client API or the AWS sdk API.
19+
In some cases when writing tests it is required to mock external API, e.g. etcd client API or the AWS SDK API.
2020

2121
This problem is usually well scoped in core Cluster API, and in most cases it is already solved by using fake
2222
implementations of the target API to be injected during tests.
@@ -26,11 +26,12 @@ some providers can use simulators reproducing the behaviour of a real infrastruc
2626
if this is not possible, a viable solution is to use mocks (e.g CAPA).
2727

2828
### Generic providers
29-
While testing core Cluster API contributors should ensure that the code works with any providers, and thus it is required
30-
to not use any specific provider implementation. Instead, the so-called generic providers should be used because they
31-
implement the plain Cluster API contract, thus preventing developers to make assumptions we cannot rely on.
29+
When writing tests core Cluster API contributors should ensure that the code works with any providers, and thus it is required
30+
to not use any specific provider implementation. Instead, the so-called generic providers e.g. "GenericInfrastructureCluster"
31+
should be used because they implement the plain Cluster API contract. This prevents tests from relying on assumptions that
32+
may not hold true in all cases.
3233

33-
Please note that in the long tern we would like to improve current implementation of generic providers, centralizing
34+
Please note that in the long term we would like to improve the implementation of generic providers, centralizing
3435
the existing set of utilities scattered across the codebase, but while details of this work will be defined do not
3536
hesitate to reach out to reviewers and maintainers for guidance.
3637

@@ -39,7 +40,7 @@ hesitate to reach out to reviewers and maintainers for guidance.
3940
Integration tests are focused on testing the behavior of an entire controller or the interactions between two or
4041
more Cluster API controllers.
4142

42-
In Cluster API, integration test are based on [envtest] and one or more controllers configured to run against
43+
In Cluster API, integration tests are based on [envtest] and one or more controllers configured to run against
4344
the test cluster.
4445

4546
With this approach it is possible to interact with Cluster API almost like in a real environment, by creating/updating
@@ -49,9 +50,9 @@ Also in case of integration tests, considerations about [mocking external APIs](
4950

5051
## Test maintainability
5152

52-
As a matter of fact test are integral part of the project codebase.
53+
Tests are an integral part of the project codebase.
5354

54-
Cluster API maintainers and all the contributors should be committed to help in ensuring that test are easily maintainable,
55+
Cluster API maintainers and all the contributors should be committed to help in ensuring that tests are easily maintainable,
5556
easily readable, well documented and consistent across the code base.
5657

5758
In light of continuing improving our practice around this ambitious goal, we are starting to introduce a shared set of:
@@ -63,7 +64,7 @@ Each contribution in growing this set of utilities or their adoption across the
6364

6465
Another consideration that can help in improving test maintainability is the idea of testing "by layers"; this idea could
6566
apply whenever we are testing "higher-level" functions that internally uses one or more "lower-level" functions;
66-
in order to avoid writing/maintaining redundant test, whenever possible contributors should take care of testing
67+
in order to avoid writing/maintaining redundant tests, whenever possible contributors should take care of testing
6768
_only_ the logic that is implemented in the "higher-level" function, delegating the test function called internally
6869
to a "lower-level" set of unit tests.
6970

@@ -317,8 +318,8 @@ func TestMain(m *testing.M) {
317318
}
318319
```
319320

320-
By the combination of pre-configured validation and mutating webhooks and reconcilers/indexes it is possible
321-
to use [envtest] for developing Cluster API integration tests that can mimic very closely how the system
321+
By combining pre-configured validation and mutating webhooks and reconcilers/indexes it is possible
322+
to use [envtest] for developing Cluster API integration tests that can mimic how the system
322323
behaves in real Cluster.
323324

324325
Please note that, because [envtest] uses a real kube-apiserver that is shared across many test cases, the developer
@@ -327,10 +328,10 @@ should take care in ensuring each test runs in isolation from the others, by:
327328
- Creating objects in separated namespaces.
328329
- Avoiding object name conflict.
329330

330-
Another thing that the developer should usually take care of when using [envtest] is the fact that the informers cache
331-
used internally the controller runtime client depends on actual etcd watches/API calls for updates, and thus it could
332-
happen that after creating or deleting objects the cache takes a few milliseconds to get updated. This can lead to
333-
test flakes, and thus it always recommended to use patterns like create and wait or delete and wait; Cluster API env
331+
Developers should also be aware of the fact that the informers cache used to access the [envtest]
332+
depends on actual etcd watches/API calls for updates, and thus it could happen that after creating
333+
or deleting objects the cache takes a few milliseconds to get updated. This can lead to test flakes,
334+
and thus it always recommended to use patterns like create and wait or delete and wait; Cluster API env
334335
test provides a set of utils for this scope.
335336

336337
However, developers should be aware that in some ways, the test control plane will behave differently from “real”
@@ -414,7 +415,7 @@ comes with a set of limitations that could hamper the validity of a test, most n
414415

415416
- it does not properly handle a set of fields which are common in the Kubernetes API objects (and Cluster API objects as well)
416417
like e.g. `creationTimestamp`, `resourceVersion`, `generation`, `uid`
417-
- API calls does not execute defaulting or validation webhooks, so there are no enforced guarantees about the semantic accuracy
418+
- [fakeclient] operations do not trigger defaulting or validation webhooks, so there are no enforced guarantees about the semantic accuracy
418419
of the test objects.
419420
- the [fakeclient] does not use a cache based on informers/API calls/etcd watches, so the test written in this way
420421
can't help in surfacing race conditions related to how those components behave in real cluster.

0 commit comments

Comments
 (0)