soarez commented on code in PR #22193:
URL: https://github.com/apache/kafka/pull/22193#discussion_r3181245475
##########
build.gradle:
##########
@@ -1574,6 +1578,32 @@ project(':group-coordinator') {
srcJar.dependsOn 'processMessages'
}
+project(':test-common:test-common-base') {
+ // Generic utilities for testing. Depends on `clients` only to minimize
classpath leakage.
+ // e.g. SSL utilities, wait functions, file-related operations
+ // Java 11 is the minimum Java version required.
+
+ base {
+ archivesName = "kafka-test-common-base"
+ }
+
+ dependencies {
+ implementation project(':clients')
Review Comment:
This was creating a circular dependency, which I think by itself is
undesirable. The AppInfoParserTest issue came from this making tests depend on
the clients shadowjar which runs `createVersionFile`. The problem can be fixed
by replacing the dependency with the raw main classpath,, done in
[2bbdd9e](https://github.com/apache/kafka/pull/22193/commits/2bbdd9eeb65ddd0ff5646088cd2919173182e5a4).
But I agree: it begs the question if this is the right approach.
I gave test fixtures approach a shot in #22201. This approach also creates
challenges: shadowjar and test-fixtures do not play well together, and we have
to sort of corrupt the idea of test fixtures is meant to be to make it work.
But overall, I think it is still a cleaner and simpler approach than
introducing this new test-common-base module. So if you agree my current
inclination would be to close this PR and merge #22201.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
