Skip to content

Conversation

@csviri
Copy link
Collaborator

@csviri csviri commented Dec 8, 2021

No description provided.

@csviri csviri self-assigned this Dec 8, 2021
when(configService.getResourceCloner()).thenReturn(ConfigurationService.DEFAULT_CLONER);
when(reconciler.reconcile(eq(customResource), any()))
.thenReturn(UpdateControl.updateResource(customResource));
when(configService.getResourceCloner()).thenReturn(new Cloner() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why was this changed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To make sure to make easier to do when(..).thenReturn() with mockito. Since otherwise its a different instance.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, but then you're not really testing the actual behavior?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This can be understood as a form of a mocking of that clone method / cloner. The goal here is not to test the cloner but only if it is called in case it's relevant.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I understand but the problem is what if the behavior changes when the instance is not actually cloned?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually not, is just what instance we move around, for some special cases.

@csviri csviri merged commit 02e9171 into main Dec 8, 2021
@csviri csviri deleted the error-status-handler-improvements branch December 8, 2021 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improving ErrorStatusHandler to be able to set status on every retry (not last one)

3 participants