Skip to content
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

KafkaEmbedded without @Rule/after() can cause build failures with System.exit #345

Closed
dgradl opened this issue Jun 6, 2017 · 6 comments
Closed

Comments

@dgradl
Copy link

dgradl commented Jun 6, 2017

Following the example of using @embeddedkafka with KafkaEmbedded in the documentation worked fine in many situations, but found that when executing on build servers as part of a gradle build my tests would succeed, but the overall build would fail with "Process 'Gradle Test Executor 1' finished with non-zero exit value 1".

This was due to improper shutdown of Zookeeper or Kafka originating from the embedded Kafka. I wasn't able to trace down exactly where it was happening, but I determined that if properly shutdown KafkaEmbedded.after() this would not occur. And this would would also get called if the JUnit Rule were used, which wasn't mentioned in the example from the docs and should look like this.

@Autowired @Rule public KafkaEmbedded kafkaEmbedded;

I don't know if anything more should be done about it, but I would propose to update the example in the documentation, it was rather difficult to isolate the cause.

@artembilan
Copy link
Member

The @DirtiesContext should help you.

The @EmbeddedKafka add KafkaEmbedded bean into the applicationContext, but without @DirtiesContext that applicationContext is cached after the test class has finished and, therefore KafkaEmbedded.after() isn't called in time.

Right, the @Rule does that independently of the Spring Test Framework, that's why we don't see issue there.

Your @Autowired @Rule isn't good because it is a mix of concerns. In this case you eliminate the result of the @EmbeddedKafka and just get a @Rule in the end.

Now quoting the Reference Manual:

Starting with version 2.0, if you are using Spring's test application context caching, you can also declare a KafkaEmbedded bean, so a single broker can be used across multiple test classes.

So, you may consider do not use @EmbeddedKafka, but just @Rule for your use-case.

@dgradl
Copy link
Author

dgradl commented Jun 7, 2017

Ok, I guess I didn't realize this was the purpose '@DirtiesContext' in the example. The '@ClassRule' usage shown later would probably accomplish what I need, but then what is the reason you would use the '@embeddedkafka' annotation instead?

@artembilan
Copy link
Member

One more time:

so a single broker can be used across multiple test classes.

Another sample is: you can use @Rule/@ClassRule for any tests, but sometime when we talk about Spring configurations we might need EmbeddedKafka as a bean to leverage dependency injection.
But right, mostly it is for Spring Test Framework caching capabilities.

@dgradl
Copy link
Author

dgradl commented Jun 7, 2017

Yeah I read that, but I guess it's really not clear how it enables that so I saw some opportunity to help improve the docs/example. @DirtiesContext will close out the context and then the other Test will get a fresh KafkaEmbedded instance. But without @DirtiesContext, it may not properly shut things down, so there must be a different way to structure the tests to enable both (sharing and proper shutdown). But I'm fine to let this go as the @ClassRule does what I need.

public @interface DirtiesContext
Test annotation which indicates that the ApplicationContext associated with a test is dirty and should therefore be closed and removed from the context cache.

@qrnik
Copy link

qrnik commented Apr 26, 2020

If someone stumps into similar problem (not necesarilly with spring-kafka), I've found a workaround:
gradle/gradle#11195 (comment)

@garyrussell
Copy link
Contributor

Thanks @pwamej - if'd you'd like to issue a pull request, please do.

Otherwise, I'll address this tomorrow.

garyrussell added a commit to garyrussell/spring-kafka that referenced this issue Apr 27, 2020
artembilan pushed a commit that referenced this issue Apr 27, 2020
See #194
See #345
See gradle/gradle#11195

**cherry-pick to 2.4.x, 2.3.x, 2.2.x, 1.3.x**
artembilan pushed a commit that referenced this issue Apr 27, 2020
See #194
See #345
See gradle/gradle#11195

**cherry-pick to 2.4.x, 2.3.x, 2.2.x, 1.3.x**
artembilan pushed a commit that referenced this issue Apr 27, 2020
See #194
See #345
See gradle/gradle#11195

**cherry-pick to 2.4.x, 2.3.x, 2.2.x, 1.3.x**
artembilan pushed a commit that referenced this issue Apr 27, 2020
See #194
See #345
See gradle/gradle#11195

**cherry-pick to 2.4.x, 2.3.x, 2.2.x, 1.3.x**

# Conflicts:
#	spring-kafka-test/src/main/java/org/springframework/kafka/test/EmbeddedKafkaBroker.java
artembilan pushed a commit that referenced this issue Apr 27, 2020
See #194
See #345
See gradle/gradle#11195

**cherry-pick to 2.4.x, 2.3.x, 2.2.x, 1.3.x**

# Conflicts:
#	spring-kafka-test/src/main/java/org/springframework/kafka/test/EmbeddedKafkaBroker.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants