Skip to content
This repository was archived by the owner on Nov 18, 2020. It is now read-only.

Unit tests, revendor, logging updates #45

Merged
merged 4 commits into from
Nov 19, 2018
Merged

Unit tests, revendor, logging updates #45

merged 4 commits into from
Nov 19, 2018

Conversation

estroz
Copy link
Member

@estroz estroz commented Nov 15, 2018

Added a unit test example using controller-runtime/pkg/client/fake. This is a first stab at unit testing using fake. In the future we should scaffold a simple unit test for every controller created by the SDK. Docs incoming.

Updated dependencies (wasn't building before) and import paths.

Updated logging statements in memcached_controller.go.

@openshift-ci-robot openshift-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Nov 15, 2018
Copy link
Contributor

@AlexNPavel AlexNPavel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

dsize := *dep.Spec.Replicas
if dsize != replicas {
t.Errorf("dep size (%d) is not the expected size (%d)", dsize, replicas)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was just about to comment on checking for the deployment before you updated it 😄

Can we also check if the reconciler successfully updates the CR's status.nodes with the pod names?
Of course we'll first have to manually create the 3 sample pods ourselves with the fake client.

Copy link
Contributor

@hasbro17 hasbro17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@estroz estroz merged commit 3af4c87 into operator-framework:master Nov 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants