MockIntegrationContext.reset() does not behave as expected

[hgarus]

Affects Version(s): 5.2.3.RELEASE using spring-boot 2.2.4.RELEASE

I am developing an application containing multiple IntegrationFlows passing messages to each other using DirectChannels.

I have created several small tests which are testing a single IntegrationFlow each. I have used MockIntegrationContext.substituteMessageHandlerFor() to capture the message once it is passed to the "output channel" of a flow.

However even though I'm calling MockIntegrationContext.reset() in an @AfterEach method, the substituted MockMessageHandlers seem to stick around until the ApplicationContext is torn down.

I have created a minimal gist demonstrating the behavior: https://gist.github.com/hgarus/0ec9714f1d381bd8404c08ddc88f55e3

The assertEquals in line 80 fails because the ArgumentCaptor receives the second message. Even though I called reset() beforehand.

I did some debugging and it seems the substitution only works, when the IntegrationConsumer is stopped before the replacement MessageHandler is assigned to the it - which is done in substituteMessageHandlerFor() (with the default autoStartup = true), but not in reset().

[artembilan]

Yeah... Confirmed. This is a bug: we have to stop endpoints and start them again to apply replacement.
Technically this is the same substituteMessageHandlerFor() but with a handler from internal beans map.

Of course, a workaround is to stop() and start() an endpoint before and after that this.mockIntegrationContext.resetBeans(); call.

[artembilan]

@hgarus ,

any thoughts that you can contribute a fix while we are out for President Day today: https://github.com/spring-projects/spring-integration/blob/master/CONTRIBUTING.adoc ?

Thanks

[hgarus]

@artembilan ,

no President's Day for me, but I'll take a stab at a PR.

[hgarus]

I'm afraid just stopping and starting the endpoint, in reset() leads to errors in tests with explicit lifecycle handling such as MockMessageSourceTests.

I currently see two ways around that:

1. Only do the stop() - start() in reset() if the endpoint is currently running.

2. Add an explicit autoStart parameter to reset() similar to the other methods and default to false in the existing method. Compared to the other methods with autoStartup default values this is exactly the other way round, which seems pretty ugly.

I noticed reset() always clears the whole map, wether it was called with or without arguments. Is this intentional?

[artembilan]

Yes, of course we need some logic over there.
And since we really talk about resetting we definitely need to have an endpoint in a state it was before resetting, so, yes your first point is correct.
I just wanted to let you know that observation is correct and has to be fixed: everything else is implementation details.

And yes, good catch about beans map clean - bug 😄

So, if you are good with all of that don't hesitate to raise a Pull Request and we continue a discussion from the code lines.
If that of course.

Thanks