Skip to content

GH-3182: Properly reset bean in the MockIntCtx #3190

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 21, 2020

Conversation

artembilan
Copy link
Member

Fixes #3182

The MockIntegrationContext.resetBeans() doesn't really restore a state
of endpoint beans: the handle/source is applied only after start for that endpoint

  • Stop endpoints in the MockIntegrationContext.resetBeans() is they are running currently
  • Start them back only if their autoStartup == true
  • Clear only those beans which names are provided for the resetBeans()

Cherry-pick to 5.2.x

Fixes spring-projects#3182

The `MockIntegrationContext.resetBeans()` doesn't really restore a state
of endpoint beans: the handle/source is applied only after start for that endpoint

* Stop endpoints in the `MockIntegrationContext.resetBeans()` is they are running currently
* Start them back only if their `autoStartup == true`
* Clear only those beans which names are provided for the `resetBeans()`

**Cherry-pick to 5.2.x**
Copy link
Contributor

@garyrussell garyrussell left a comment

Choose a reason for hiding this comment

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

Just one question/comment.

@garyrussell garyrussell merged commit b38c9e1 into spring-projects:master Feb 21, 2020
garyrussell pushed a commit that referenced this pull request Feb 21, 2020
* GH-3182: Properly reset bean in the MockIntCtx

Fixes #3182

The `MockIntegrationContext.resetBeans()` doesn't really restore a state
of endpoint beans: the handle/source is applied only after start for that endpoint

* Stop endpoints in the `MockIntegrationContext.resetBeans()` is they are running currently
* Start them back only if their `autoStartup == true`
* Clear only those beans which names are provided for the `resetBeans()`

**Cherry-pick to 5.2.x**

* * Ensure that only provided beans are reset
@garyrussell
Copy link
Contributor

Cherry-picked as 4e4e8db

@@ -108,9 +114,19 @@ else if (endpoint instanceof ReactiveStreamsConsumer) {
else if (endpoint instanceof IntegrationConsumer) {
directFieldAccessor.setPropertyValue(HANDLER, e.getValue());
}
if (lifecycle != null && lifecycle.isAutoStartup()) {
Copy link

Choose a reason for hiding this comment

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

Doesn't this have potential to break existing tests?

Maybe this just matters for constructed examples. But I could imagine something like this:

@BeforeClass
void init() {
  nonAutostartupHandler.start();
}

@After 
void cleanup() {
  mockIntegrationContext.reset()
}

@Test
void t1() {
  // do something with handler
}

@Test 
void t2() {
  // expect handler to be still running
}

Admittedly the @BeforeClass is somewhat artificial. But I could see a handler being manually started as part of context initialization.

I'm sorry for not contributing a fix myself, I had a pretty busy week at work.

Copy link
Member Author

Choose a reason for hiding this comment

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

nonAutostartupHandler.start(); - you definitely start it manually. The framework can't assume when and how you have started it or going to.
Only the way for the framework to start it when you have that autoStartup = true.

I admit that it may break some peculiar test, but at the same time the test is wrong because it has been relied on bug which we are fixing here.

On the other hand I don't worry too much about breaking changes for tests. That's not that part of project which may make your customer unhappy.

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.

MockIntegrationContext.reset() does not behave as expected
3 participants