Skip to content

Conversation

@piotrpdev
Copy link
Contributor

@piotrpdev piotrpdev commented Dec 2, 2025

Type of change

  • Refactoring

Description

  • Testcontainers bumped to 2.0.2.
    • Dependencies renamed accordingly.
  • Switched JUnit 4 assertTrue to Jupiter assertTrue.
  • Add JUnit 4 to enforcer's banned dependencies.

Additional Context

See kroxylicious/kroxylicious#2801

First removing JUnit 4 from classpath in this repo, then eventually removing JUnit 4 from main kroxylicious repo.

Checklist

  • Write tests
  • Make sure all tests pass
  • Update documentation
  • Reference relevant issue(s) and close them after merging
  • Update CHANGELOG.md

@k-wall
Copy link
Member

k-wall commented Dec 3, 2025

@piotrpdev thanks for picking this up.

The tests are consistently failing for me when this PR is applied. I haven't investigated the root cause. The stack traces look like this:

2025-12-03 11:34:03 WARN  kafka-admin-client-thread | adminclient-78 io.kroxylicious.testing.kafka.common.Utils:146 - Unexpected failure describing topic: __org_kroxylicious_testing_consistencyTest due to org.apache.kafka.common.errors.TimeoutException: Timed out waiting for a node assignment. Call: listNodes
java.util.concurrent.CompletionException: org.apache.kafka.common.errors.TimeoutException: Timed out waiting for a node assignment. Call: listNodes
	at java.base/java.util.concurrent.CompletableFuture.encodeThrowable(CompletableFuture.java:332)
	at java.base/java.util.concurrent.CompletableFuture.completeThrowable(CompletableFuture.java:347)
	at java.base/java.util.concurrent.CompletableFuture$UniApply.tryFire(CompletableFuture.java:636)
[ERROR] Errors: 
[ERROR]   TestcontainersKafkaClusterTest.scramUsersCreated:303 Runtime org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in io.kroxylicious.testing.kafka.common.Utils was not fulfilled within 2 minutes.
[ERROR]   TestcontainersKafkaClusterTest.scramUsersCreated:303 Runtime java.lang.RuntimeException: startup failed or timed out
[ERROR]   TestcontainersKafkaClusterTest.scramUsersCreated:303 Runtime java.lang.RuntimeException: startup failed or timed out
[ERROR]   TestcontainersKafkaClusterTest.scramUsersCreated:303 Runtime java.lang.RuntimeException: startup failed or timed out

@piotrpdev piotrpdev force-pushed the build/testcontainers-2 branch from eb3e48a to 43ac0b1 Compare December 4, 2025 13:19
@piotrpdev
Copy link
Contributor Author

@k-wall Looks like Testcontainers 2.x uses a newer commons-io version and that was conflicting with the one in Kafka, so I excluded the Kafka one in 43ac0b1 (#556) (let me know if you think shading would be better).

The tests should (hopefully) pass now.

@robobario
Copy link
Member

It looks like it only comes in via Zookeeper, I think we could remove the exclusion from Kafka.

I think it's good enough without introducing shading. Testing with Zookeeper isn't a priority and if users hit on any incompatibilities running ZK in-VM with this commons-io version they can instead swap to using containers.

@k-wall
Copy link
Member

k-wall commented Dec 19, 2025

It looks like it only comes in via Zookeeper, I think we could remove the exclusion from Kafka.

Yeah, agreed.

ZK brings it in.

[INFO] +- org.apache.zookeeper:zookeeper:jar:3.8.4:provided
[INFO] |  +- org.apache.zookeeper:zookeeper-jute:jar:3.8.4:provided
[INFO] |  +- org.apache.yetus:audience-annotations:jar:0.12.0:provided
[INFO] |  +- io.netty:netty-handler:jar:4.1.105.Final:provided
[INFO] |  |  +- io.netty:netty-common:jar:4.1.105.Final:compile
[INFO] |  |  +- io.netty:netty-resolver:jar:4.1.105.Final:provided
[INFO] |  |  +- io.netty:netty-buffer:jar:4.1.105.Final:provided
[INFO] |  |  +- io.netty:netty-transport:jar:4.1.105.Final:provided
[INFO] |  |  +- io.netty:netty-transport-native-unix-common:jar:4.1.105.Final:provided
[INFO] |  |  \- io.netty:netty-codec:jar:4.1.105.Final:provided
[INFO] |  +- io.netty:netty-transport-native-epoll:jar:4.1.105.Final:provided
[INFO] |  |  \- io.netty:netty-transport-classes-epoll:jar:4.1.105.Final:provided
[INFO] |  +- ch.qos.logback:logback-core:jar:1.2.13:provided
[INFO] |  \- commons-io:commons-io:jar:2.11.0:compile

Kafka doesn't.

[INFO] +- org.apache.kafka:kafka_2.13:jar:4.1.1:provided
[INFO] |  +- org.scala-lang:scala-library:jar:2.13.16:provided
[INFO] |  +- org.apache.kafka:kafka-server-common:jar:4.1.1:provided
[INFO] |  |  \- org.pcollections:pcollections:jar:4.0.2:provided
[INFO] |  +- org.apache.kafka:kafka-group-coordinator-api:jar:4.1.1:provided
[INFO] |  +- org.apache.kafka:kafka-group-coordinator:jar:4.1.1:provided
[INFO] |  |  +- org.hdrhistogram:HdrHistogram:jar:2.2.2:provided
[INFO] |  |  \- com.dynatrace.hash4j:hash4j:jar:0.22.0:provided
[INFO] |  +- org.apache.kafka:kafka-transaction-coordinator:jar:4.1.1:provided
[INFO] |  +- org.apache.kafka:kafka-metadata:jar:4.1.1:provided
[INFO] |  +- org.apache.kafka:kafka-storage-api:jar:4.1.1:provided
[INFO] |  +- org.apache.kafka:kafka-tools-api:jar:4.1.1:provided
[INFO] |  +- org.apache.kafka:kafka-raft:jar:4.1.1:provided
[INFO] |  +- org.apache.kafka:kafka-storage:jar:4.1.1:provided
[INFO] |  |  \- com.github.ben-manes.caffeine:caffeine:jar:3.2.0:provided
[INFO] |  |     \- org.jspecify:jspecify:jar:1.0.0:provided
[INFO] |  +- org.apache.kafka:kafka-server:jar:4.1.1:provided
[INFO] |  +- org.apache.kafka:kafka-coordinator-common:jar:4.1.1:provided
[INFO] |  +- org.apache.kafka:kafka-share-coordinator:jar:4.1.1:provided
[INFO] |  +- net.sourceforge.argparse4j:argparse4j:jar:0.7.0:provided
[INFO] |  +- commons-validator:commons-validator:jar:1.9.0:provided
[INFO] |  |  +- commons-beanutils:commons-beanutils:jar:1.9.4:provided
[INFO] |  |  +- commons-digester:commons-digester:jar:2.1:provided
[INFO] |  |  +- commons-logging:commons-logging:jar:1.3.2:provided
[INFO] |  |  \- commons-collections:commons-collections:jar:3.2.2:provided
[INFO] |  +- com.fasterxml.jackson.core:jackson-databind:jar:2.19.0:provided
[INFO] |  |  +- com.fasterxml.jackson.core:jackson-annotations:jar:2.19.0:compile
[INFO] |  |  \- com.fasterxml.jackson.core:jackson-core:jar:2.19.0:provided
[INFO] |  +- com.fasterxml.jackson.dataformat:jackson-dataformat-csv:jar:2.19.0:provided
[INFO] |  +- com.fasterxml.jackson.datatype:jackson-datatype-jdk8:jar:2.19.0:provided
[INFO] |  +- com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:jar:2.19.0:provided
[INFO] |  |  \- org.yaml:snakeyaml:jar:2.4:provided
[INFO] |  +- net.sf.jopt-simple:jopt-simple:jar:5.0.4:provided
[INFO] |  +- org.bitbucket.b_c:jose4j:jar:0.9.6:provided
[INFO] |  +- com.yammer.metrics:metrics-core:jar:2.2.0:provided
[INFO] |  +- org.scala-lang:scala-reflect:jar:2.13.16:provided
[INFO] |  +- com.typesafe.scala-logging:scala-logging_2.13:jar:3.9.5:provided
[INFO] |  \- com.google.re2j:re2j:jar:1.8:provided

I think it's good enough without introducing shading. Testing with Zookeeper isn't a priority and if users hit on any incompatibilities running ZK in-VM with this commons-io version they can instead swap to using containers.

+1

@k-wall k-wall enabled auto-merge (rebase) December 19, 2025 14:33
@k-wall
Copy link
Member

k-wall commented Dec 19, 2025

It looks like it only comes in via Zookeeper, I think we could remove the exclusion from Kafka.

Kafka 3.9.0 does depend on commons-io 2.14.0, so the test matrix will fail on Kafka 3.9.0 with a NoSuchMethodError.

Error: Exception in thread "Thread-6" java.lang.NoSuchMethodError: 'long org.apache.commons.io.file.attribute.FileTimes.toUnixTime(java.nio.file.attribute.FileTime)'
	at org.apache.commons.compress.archivers.tar.TarArchiveOutputStream.transferModTime(TarArchiveOutputStream.java:616)
	at org.apache.commons.compress.archivers.tar.TarArchiveOutputStream.writePaxHeaders(TarArchiveOutputStream.java:663)
	at org.apache.commons.compress.archivers.tar.TarArchiveOutputStream.putArchiveEntry(TarArchiveOutputStream.java:541)
	at org.testcontainers.images.builder.Transferable.transferTo(Transferable.java:81)
	at org.testcontainers.containers.ContainerState.lambda$copyFileToContainer$2(ContainerState.java:358)
	at java.base/java.lang.Thread.run(Thread.java:840)

I think rather than excluding the commons-io dependency, a more obvious solution would be to dependency manage it.

In the end I prefered @piotrpdev's way. I improved the comment to explain why it was needed for Kafka too.

@k-wall k-wall closed this Dec 19, 2025
auto-merge was automatically disabled December 19, 2025 17:03

Pull request was closed

@k-wall k-wall reopened this Dec 19, 2025
@k-wall k-wall force-pushed the build/testcontainers-2 branch from 4646d3f to ffc2fcf Compare December 19, 2025 18:47
@sonarqubecloud
Copy link

@k-wall k-wall merged commit 3463d4b into kroxylicious:main Dec 19, 2025
7 checks passed
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.

3 participants