The stack frames displayed using assert.Fail was not very informative.
That is due to t.Cleanup being called "outside" of the test case
context. There was no mention of the test file itself, gatherer_test.go
in the following example:
certificaterequest.go:205:
Error Trace: certificaterequest.go:205
testing.go:872
testing.go:866
testing.go:873
testing.go:949
testing.go:1121
Error: lister.CertificateRequests was expected to be called but was not called
Test: TestDataForCertificate/should_return_error_when_the_list_func_returns_an_error
With this patch that vendors a simple version of assert.Fail, we get the
correct stack frames that the user needs in order to locate where this
failure happened:
certificaterequest.go:254:
Error Trace: gatherer_test.go:230
gatherer_test.go:240
Error: lister.CertificateRequests was expected to be called but was not called
Test: TestDataForCertificate/should_return_error_when_the_list_func_returns_an_error
Signed-off-by: Maël Valais <mael@vls.dev>
At first, I tried to follow the "generator" pattern that had already
been implemented for the order and secret objects. These generators look
like:
import (
"github.com/jetstack/cert-manager/test/unit/listers"
)
fake := listers.FakeSecretListerFrom(listers.NewFakeSecretLister(),
listers.SetFakeSecretNamespaceListerGet(nil, errors.New("not found")),
)
The major issue I was finding with this approach is that you cannot
enforce any behavior with these fakes: no way to check (or prevent)
unwanted called, no way to check that the correct namespace was used for
the call:
fake.Secrets("default").Get("secret-1")
which is annoying; I want to be able to check every input, output and
call numbers made to the mocked function.
So I propose a gomock-like approach. I could not use mockgen due to the
fact that (again) client-go is overly nested, which means I would have
to use quite a lot of glue code in order to use mockgen-generated mocks.
Signed-off-by: Maël Valais <mael@vls.dev>
I initially thought about using the fake clientset like anywhere else,
but this time I thought: what about trying out the hard way, i.e.,
writing all the mocking code myself?
Result: not that hard, but requires more time than just using the fake
clientset.
Signed-off-by: Maël Valais <mael@vls.dev>
The issuing controller wasn't checking if the certificate request that
it picked up is up to date. That resulted in the certificate being set
to "Failing" and "Issuing = False" due to an old certificate request
that was created during a previous issuance. The certificate would then
become stale.
Signed-off-by: Maël Valais <mael.valais@gmail.com>
This new unit test highlights an unexpected behavior of the issuing
controller: the issuing controller is updating the certificate's status
when the certificate request has a failure ("Reason = Failed"), but the
controller might have picked up an out-of-date certificate request.
The consequence is that the issuing controller would set the certificate
to "Issuing = False". That happens when a re-issuance is triggered with
an old failing certificate request.
Signed-off-by: Maël Valais <mael.valais@gmail.com>
As Maartje mentioned, it doesn't make sense to return backoff = true
while returning a delay of 0. Also, use time.UTC instead of time.Local.
Signed-off-by: Maël Valais <mael.valais@gmail.com>
The trigger_controller_test.go has many unrelated test cases and I
thought it would be good to have more tightly scoped functions that are
easy to review (and most importantly, the unit tests are easy to
review).
Signed-off-by: Maël Valais <mael.valais@gmail.com>
jetstack-botandGitHub
irbekrm
Josh Soref
joshvanl
Maartje Eyskens
exceptionfactory
Maël Valais