Move install routine into common code and add tests. #75
Conversation
knative-prow-robot
added
approved
There was a problem hiding this comment.
@markusthoemmes: 0 warnings.
In response to this:
Proposed Changes
As the title says mostly. This factors the install routine into common code and adds unit tests to them. Reuses the now common handler in both reconcilers.
Release Note
NONE/assign @aliok @jcrossley3 @houshengbo
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
|
The following is the coverage report on the affected files.
|
|
/test pull-knative-sandbox-operator-upgrade-tests |
| // Expect things to be applied in order. | ||
| want := []*unstructured.Unstructured{role, clusterRole, roleBinding, clusterRoleBinding, deployment} | ||
|
|
||
| client := &fakeClient{} |
There was a problem hiding this comment.
I should publish a mf.fake.Client, I think. I've wanted one more than once.
There was a problem hiding this comment.
Not quite sure if that's necessary. A little helper that makes me only implement the functions really under test would be nice maybe.
| deployment := NamespacedResource("apps/v1", "Deployment", "test", "test-deployment") | ||
| role := NamespacedResource("rbac.authorization.k8s.io/v1", "Role", "test", "test-role") | ||
| roleBinding := NamespacedResource("rbac.authorization.k8s.io/v1", "RoleBinding", "test", "test-role-binding") | ||
| clusterRole := ClusterScopedResource("rbac.authorization.k8s.io/v1", "ClusterRole", "test-cluster-role") | ||
| clusterRoleBinding := ClusterScopedResource("rbac.authorization.k8s.io/v1", "ClusterRoleBinding", "test-cluster-role-binding") |
There was a problem hiding this comment.
If this ever became unwieldy, I might toss a yaml file beneath testdata/
There was a problem hiding this comment.
I like to have them available programmatically a ton more I must say. As in this case, I can reuse the same objects to create the "in" and "want" lists and even reuse their memory adresses.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jcrossley3, markusthoemmes The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Proposed Changes
As the title says mostly. This factors the install routine into common code and adds unit tests to them. Reuses the now common handler in both reconcilers.
Release Note
/assign @aliok @jcrossley3 @houshengbo