Closed atoato88 closed 1 year ago
/assign @justinsb Could you review this?
/retest-required
umm, test.sh
passed on my laptop, but failure on CI...
It looks like that test can't use fake client? :thinking:
Current PR needs live k8s cluster, I think. I got same error without live k8s cluster on my laptop.
Is it possible that unit test on fake client without k8s cluster...?
/retitle [WIP] Add unit test for direct applier
/retitle Add unit test for direct applier Now, ci test become to pass, I remove WIP.
This lgtm. In #242 I'm proposing to expand our mockkubeapiserver to the point where the DirectApplier can target it. But this test is still very valuable to check the mapping/behaviour of the flags.
I'm going to approve this, and then we can tackle the questions around whether we need those commented-out cases later?
/approve /lgtm
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: atoato88, justinsb
The full list of commands accepted by this bot can be found here.
The pull request process is described here
What this PR does / why we need it: This PR adds unit tests for direct applier in
direct.go
.direct.go
doesn't have tests as far, we should add tests for prevent degrades.For implimenting tests, this PR changes direct applier to have some delegation structures. It is the same manner with that in
exec.go
Which issue(s) this PR fixes: Related to #201 comment
Special notes for your reviewer: None
Additional documentation: None