Closed karlkfi closed 2 years ago
/hold
Working on adding some tests. Apparently we never had any tests added for the inventory status, so none of them broke when I disabled it.
The e2e testcases with StatusPolicyAll
looks good. Is there any testcase for StatusPolicyNone
?
The ConfigMap provider uses StatusPolicyNone and the Custom provider uses StatusPolicyAll.
There's no tests for a custom provider with StatusPolicyAll and no status subresource, but we don't have nay clients doing that atm, so I didn't bother.
I also didn't add any unit tests. I would really like it if we had some for the ClusterClient that tested the various edge cases. But I'm at least relatively confident it works as-is now. Since you added the status update, it would be awesome if you could back fill some unit tests, @Liujingfang1. But they don't need to block this PR.
/unhold
/lgtm /approve
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: karlkfi, Liujingfang1
The full list of commands accepted by this bot can be found here.
The pull request process is described here