Add unit tests to nats reconciler

[vpaskar]

Description

Changes proposed in this pull request:

• add unit tests for nats subscription controller
• move some reconciler login into separate function
• mock MessagingBackend

Related issue(s)
#13161

[mfaizanse]

(nit) Why don't do it in oneline:

return r.handleSubscriptionDeletion(ctx, subscription, log)

[vpaskar]

IMO it's harder to read such oneliners if you mean something like this:
return ctrl.Result{}, r.handleSubscriptionDeletion(ctx, subscription, log)

[nachtmaar]

@mfaizanse handleSubscriptionDeletion only returns an error. We need ctrl.Result and error though

[mfaizanse]

I mean you can change the return type of handleSubscriptionDeletion to (ctrl.Result, error).

[vpaskar]

Yes I could do that, but I think we could do it in this way two, because

• it's only one additional line
• imagine if you would want to add something after the deletion, i.e. an error log, in that case you don't need to return right away
• we already do it both ways in our code

So I would keep that.

[nachtmaar]

Thanks a lot for your work on adding unit tests to the nats controller 💪 👍

[nachtmaar]

@mfaizanse handleSubscriptionDeletion only returns an error. We need ctrl.Result and error though

[nachtmaar]

Feel free to merge, just left one comment which is not required for merging :)

[nachtmaar]

Alternative to this (which is more the gomega way I guess) would be something like this (example from the testing/matchers.go file).

func HaveAPIRuleSpecRules(ruleMethods []string, accessStrategy, path string) gomegatypes.GomegaMatcher {
	return WithTransform(func(a apigatewayv1alpha1.APIRule) []apigatewayv1alpha1.Rule {
		return a.Spec.Rules
	}, ContainElement(
		MatchFields(IgnoreExtras|IgnoreMissing, Fields{
			"Methods":          ConsistOf(ruleMethods),
			"AccessStrategies": ConsistOf(haveAPIRuleAccessStrategies(accessStrategy)),
			"Gateway":          Equal(constants.ClusterLocalAPIGateway),
			"Path":             Equal(path),
		}),
	))
}

It would read like this then (assuming the helper function is called HaveConditions instead of HaveAPIRuleSpecRules):

			g.Expect(sub.Status.Conditions).Should(HaveConditions(testCase.expectedConditions))

Oh I just discovered there is already HaveCondition in the same file already :)

func HaveCondition(condition eventingv1alpha1.Condition) gomegatypes.GomegaMatcher {
	return WithTransform(func(s *eventingv1alpha1.Subscription) []eventingv1alpha1.Condition { return s.Status.Conditions }, ContainElement(MatchFields(IgnoreExtras|IgnoreMissing, Fields{
		"Type":    Equal(condition.Type),
		"Reason":  Equal(condition.Reason),
		"Message": Equal(condition.Message),
		"Status":  Equal(condition.Status),
	})))
}

[kyma-bot]

@VladislavPaskar: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pre-main-kyma-skr-eventing	4a15f73	link	false	/test pre-main-kyma-skr-eventing

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.