You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "soarez (via GitHub)" <gi...@apache.org> on 2026/05/04 01:48:42 UTC
[PR] MINOR: Move TestSslUtils to client test fixtures [kafka]
soarez opened a new pull request, #22201:
URL: https://github.com/apache/kafka/pull/22201
This change moves TestSslUtils, TestUtils and accessory classes from the test classpath in clients to test fixtures, allowing other modules to use these classes for testing without having to include all of the test classpath for clients.
For further details on the context and purpose of this change, check these threads:
https://github.com/apache/kafka/pull/20376#discussion_r3167281569 https://github.com/apache/kafka/pull/22193#discussion_r3178891141
Delete this text and replace it with a detailed description of your change. The
PR title and body will become the squashed commit message.
If you would like to tag individuals, add some commentary, upload images, or
include other supplemental information that should not be part of the eventual
commit message, please use a separate comment.
If applicable, please include a summary of the testing strategy (including
rationale) for the proposed change. Unit and/or integration tests are expected
for any behavior change and system tests should be considered for larger
changes.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Move client test utilities to test fixtures and resolve shadow jar conflicts [kafka]
Posted by "omkreddy (via GitHub)" <gi...@apache.org>.
omkreddy commented on PR #22201:
URL: https://github.com/apache/kafka/pull/22201#issuecomment-4437144149
@soarez I see that we are not publishing `kafka-clients-test-fixtures-4.4.0-SNAPSHOT.jar`. I wanted to know if there is a specific reason for this. The reason I’m asking is that there are downstream projects that depend on these test utilities, so it would be useful to publish them.
cc @chia7712
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Move client test utilities to test fixtures and resolve shadow jar conflicts [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #22201:
URL: https://github.com/apache/kafka/pull/22201#issuecomment-4437442297
@omkreddy thanks for pointing that out. The test-fixtures jar definitely needs to be published. We’ll file a patch for it shortly.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Move TestSslUtils to client test fixtures [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #22201:
URL: https://github.com/apache/kafka/pull/22201#discussion_r3183789533
##########
build.gradle:
##########
@@ -1879,11 +1896,34 @@ project(':clients') {
archivesName = "kafka-clients"
}
+ apply plugin: 'java-test-fixtures'
+
configurations {
generator
shadowed
}
+ // java-test-fixtures creates dependencies testImplementation -> testFixturesApi -> (main artifact)
Review Comment:
This is definitely a magic workaround... I noticed there are several issues discussing this behavior
https://github.com/gradle/gradle/issues/19946
https://github.com/gradle/gradle/issues/11696
Anyway, good job finding this approach!
##########
build.gradle:
##########
@@ -313,6 +313,16 @@ subprojects {
task allDepInsight(type: DependencyInsightReportTask) {showingAllVariants = false} doLast {}
apply plugin: 'java-library'
+
+ // Resolve conflict between testFixtures and shadow in :clients.
Review Comment:
Could you add a comment referencing the "magic workaround" in the clients project?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Move client test utilities to test fixtures and resolve shadow jar conflicts [kafka]
Posted by "soarez (via GitHub)" <gi...@apache.org>.
soarez commented on PR #22201:
URL: https://github.com/apache/kafka/pull/22201#issuecomment-4438009291
No, I didn’t think of downstream dependencies. It does make sense to publish it. And I see there’s already a patch! Thanks @majialoong
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Move client test utilities to test fixtures and resolve shadow jar conflicts [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 merged PR #22201:
URL: https://github.com/apache/kafka/pull/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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Move client test utilities to test fixtures and resolve shadow jar conflicts [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #22201:
URL: https://github.com/apache/kafka/pull/22201#discussion_r3609354999
##########
build.gradle:
##########
@@ -2540,7 +2583,7 @@ project(':tools') {
compileOnly libs.spotbugs
testImplementation project(':clients')
- testImplementation project(':clients').sourceSets.test.output
+ testImplementation testFixtures(project(':clients'))
Review Comment:
Seeing that `tools` depends on the `testFixtures(project(':clients'))`, the `testImplementation libs.bcpkix` could be deleted from `tools` module.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Move TestSslUtils to client test fixtures [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #22201:
URL: https://github.com/apache/kafka/pull/22201#discussion_r3185648234
##########
build.gradle:
##########
@@ -1120,6 +1139,7 @@ project(':core') {
implementation libs.re2j
testImplementation project(':clients').sourceSets.test.output
Review Comment:
It would be great if we could remove all references to `project(':clients').sourceSets.test.output`. We could move the classes used across modules to the test-fixtures folder. WDYT?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Move client test utilities to test fixtures and resolve shadow jar conflicts [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #22201:
URL: https://github.com/apache/kafka/pull/22201#discussion_r3491163510
##########
build.gradle:
##########
@@ -1879,11 +1898,35 @@ project(':clients') {
archivesName = "kafka-clients"
}
+ apply plugin: 'java-test-fixtures'
+
configurations {
generator
shadowed
}
+ // Rewire test fixtures dependencies to avoid depending on the shadow JAR:
+ // java-test-fixtures creates dependencies testImplementation -> testFixturesApi -> (main artifact)
+ // It prefers the main artifact over the raw main classpath so that dependents on fixtures get the same
+ // packaged artifact as production consumers. In this module the main artifact is the shadow JAR,
+ // so this creates a test dependency on createVersionFile breaking AppInfoParserTest, and worse:
+ // test code is compiled against the original packages, but the shadow JAR contains relocated bytecode,
+ // causing runtime errors for various tests. To fix this, we rewire the test fixtures dependency to the raw
+ // source set output, so tests continue to use compiled classes and original dependencies instead of the shadow JAR.
+ // https://github.com/gradle/gradle/blob/v9.4.1/platforms/jvm/plugins-jvm-test-fixtures/src/main/java/org/gradle/api/plugins/JavaTestFixturesPlugin.java#L89-L90
+ afterEvaluate {
+ configurations.testFixturesApi.dependencies.removeIf { dep ->
+ dep instanceof ProjectDependency && dep.name == project.name
+ }
+ dependencies {
+ testFixturesApi files(sourceSets.main.output.classesDirs)
Review Comment:
We should revisit those tests instead of hacking the dependencies. The test code in the clients module should NOT depend on `io.opentelemetry`, and that can be addressed by moving the `io.opentelemetry`-related code to `ClientTelemetryUtils`. It is not perfect, but it's better than a hack in `build.gradle`
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Move client test utilities to test fixtures and resolve shadow jar conflicts [kafka]
Posted by "hjtiun852 (via GitHub)" <gi...@apache.org>.
hjtiun852 commented on code in PR #22201:
URL: https://github.com/apache/kafka/pull/22201#discussion_r3609448374
##########
build.gradle:
##########
@@ -2540,7 +2583,7 @@ project(':tools') {
compileOnly libs.spotbugs
testImplementation project(':clients')
- testImplementation project(':clients').sourceSets.test.output
+ testImplementation testFixtures(project(':clients'))
Review Comment:
Hi @chia7712,
I've opened #22868 to address this. PTAL when you have time, thanks!
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Move TestSslUtils to client test fixtures [kafka]
Posted by "soarez (via GitHub)" <gi...@apache.org>.
soarez commented on code in PR #22201:
URL: https://github.com/apache/kafka/pull/22201#discussion_r3190365523
##########
build.gradle:
##########
@@ -1120,6 +1139,7 @@ project(':core') {
implementation libs.re2j
testImplementation project(':clients').sourceSets.test.output
Review Comment:
All such references were removed and disallowed - adding them now triggers a build error.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Move TestSslUtils to client test fixtures [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #22201:
URL: https://github.com/apache/kafka/pull/22201#discussion_r3191905960
##########
clients/src/testFixtures/java/org/apache/kafka/clients/admin/FailureInjectingTimeoutProcessorFactory.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kafka.clients.admin;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class FailureInjectingTimeoutProcessorFactory extends KafkaAdminClient.TimeoutProcessorFactory {
Review Comment:
similar to `KafkaAdminClientInternalFactory`, which can be moved out of testfixtures.
##########
clients/src/testFixtures/java/org/apache/kafka/clients/admin/KafkaAdminClientInternalFactory.java:
##########
@@ -0,0 +1,23 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kafka.clients.admin;
+
+public class KafkaAdminClientInternalFactory {
Review Comment:
Exposing a package-private method through this 'factory' feels very hacky. I'll submit a PR to move the specific test from core to clients-integration-test so we can close this door for good.
##########
build.gradle:
##########
@@ -4014,6 +4056,25 @@ task aggregatedJavadoc(type: Javadoc) {
}
gradle.projectsEvaluated {
+ // Validate that no module depends on `project(':clients').sourceSets.test.output`
+ allprojects { proj ->
+ proj.configurations.all { config ->
+ config.dependencies.each { dep ->
+ if (dep instanceof FileCollectionDependency) {
+ dep.files.files.each { file ->
+ if (file.absolutePath.endsWith('/clients/build/classes/java/test')) {
Review Comment:
Nice catch. Right now it only rejects client test code, but since other modules still depend on other's test code, this will eventually result in all test scopes being rejected as we clean them up. A beautiful world! :smile:
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Move client test utilities to test fixtures and resolve shadow jar conflicts [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #22201:
URL: https://github.com/apache/kafka/pull/22201#discussion_r3212511936
##########
build.gradle:
##########
@@ -1902,17 +1945,17 @@ project(':clients') {
compileOnly libs.jacksonJDK8Datatypes
compileOnly libs.jose4j // for SASL/OAUTHBEARER JWT validation; only used by broker
+ testFixturesApi project(':test-common:test-common-util')
Review Comment:
What do you think about using `testFixturesImplementation` instead? It provides better isolation for our dependencies.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Move client test utilities to test fixtures and resolve shadow jar conflicts [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #22201:
URL: https://github.com/apache/kafka/pull/22201#discussion_r3196614321
##########
clients/src/testFixtures/java/org/apache/kafka/clients/admin/KafkaAdminClientInternalFactory.java:
##########
@@ -0,0 +1,23 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kafka.clients.admin;
+
+public class KafkaAdminClientInternalFactory {
Review Comment:
#22220
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Move client test utilities to test fixtures and resolve shadow jar conflicts [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on code in PR #22201:
URL: https://github.com/apache/kafka/pull/22201#discussion_r3212601219
##########
build.gradle:
##########
@@ -1902,17 +1945,17 @@ project(':clients') {
compileOnly libs.jacksonJDK8Datatypes
compileOnly libs.jose4j // for SASL/OAUTHBEARER JWT validation; only used by broker
+ testFixturesApi project(':test-common:test-common-util')
Review Comment:
Addressed in https://github.com/apache/kafka/pull/22243
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Move client test utilities to test fixtures and resolve shadow jar conflicts [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #22201:
URL: https://github.com/apache/kafka/pull/22201#discussion_r3609355826
##########
build.gradle:
##########
@@ -2540,7 +2583,7 @@ project(':tools') {
compileOnly libs.spotbugs
testImplementation project(':clients')
- testImplementation project(':clients').sourceSets.test.output
+ testImplementation testFixtures(project(':clients'))
Review Comment:
We will file a minor patch for it
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org