spring-projects / spring-kafka

Provides Familiar Spring Abstractions for Apache Kafka
https://projects.spring.io/spring-kafka
Apache License 2.0
2.19k stars 1.56k forks source link

No parameter resolver for EmbeddedKafkaZKBroker #2927

Closed Sh1ftry closed 10 months ago

Sh1ftry commented 10 months ago

In what version(s) of Spring for Apache Kafka are you seeing this issue? 3.1.0

Describe the bug

Starting a test without spring context with embeded kafka without kraft enabled and EmbeddedKafkaZKBroker as a test's parameter results in an exception.

No ParameterResolver registered for parameter [org.springframework.kafka.test.EmbeddedKafkaZKBroker broker] in method [public void test.SomeTest.test(org.springframework.kafka.test.EmbeddedKafkaZKBroker)].
org.junit.jupiter.api.extension.ParameterResolutionException: No ParameterResolver registered for parameter [org.springframework.kafka.test.EmbeddedKafkaZKBroker broker] in method [public void test.SomeTest.test(org.springframework.kafka.test.EmbeddedKafkaZKBroker)].
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

To Reproduce

import org.junit.jupiter.api.Test;
import org.springframework.kafka.test.EmbeddedKafkaZKBroker;
import org.springframework.kafka.test.context.EmbeddedKafka;

@EmbeddedKafka(kraft = false)
public class SomeTest {

  @Test
  public void test(EmbeddedKafkaZKBroker broker) {}
}

Expected behavior

EmbeddedKafkaZKBroker is resolved properly.

artembilan commented 10 months ago

Confirmed as a bug. Looks like EmbeddedKafkaCondition.supportsParameter() must be fixed for something like:

return EmbeddedKafkaBroker.class.isAssignableFrom(parameterContext.getParameter().getType());

Feel free to contribute the fix!

As a workaround you can just use public void test(EmbeddedKafkaBroker broker).

Sh1ftry commented 10 months ago

Thanks for quick response @artembilan. Should the supportsParameter method take into account whether kraft is enabled or not? I guess it shouldn’t support EmbeddedKafkaKraftBroker when kraft is not enabled and it shouldn’t support EmbeddedKafkaZKBroker when kraft is enabled.

artembilan commented 10 months ago

Sounds reasonable , but if we cannot reach that logic easily in the condition , I wouldn’t spend too much time on that. Rather simple fix as we have discussed before . Or even better to say: let’s fix the bug first! The logic you are asking for could be considered as an improvement afterwards.