You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "FrankYang0529 (via GitHub)" <gi...@apache.org> on 2026/06/09 13:48:11 UTC
[PR] KAFKA-20553: Eliminate the dependencies on sourceSets.test.output for streams [kafka]
FrankYang0529 opened a new pull request, #22521:
URL: https://github.com/apache/kafka/pull/22521
Gradle test fixtures to the clients module and removes all dependencies
on sourceSets.test.output. We should revisit other usages in this JIRA.
This will simplify the dependency graph and unblock the upgrade to
Gradle 9.5+
This patch is working on `streams` and `streams:integration-tests`.
--
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] KAFKA-20553: Eliminate the dependencies on sourceSets.test.output for streams [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #22521:
URL: https://github.com/apache/kafka/pull/22521#discussion_r3476997894
##########
build.gradle:
##########
@@ -3081,6 +3109,14 @@ project(':streams:integration-tests') {
srcDirs = []
}
}
+ testFixtures {
+ java {
+ srcDirs = []
+ }
+ scala {
Review Comment:
#22675
--
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] KAFKA-20553: Eliminate the dependencies on sourceSets.test.output for streams [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #22521:
URL: https://github.com/apache/kafka/pull/22521#issuecomment-5081269168
@FrankYang0529 please rebase code
--
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] KAFKA-20553: Eliminate the dependencies on sourceSets.test.output for streams [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #22521:
URL: https://github.com/apache/kafka/pull/22521#discussion_r3653796424
##########
streams/src/testFixtures/java/org/apache/kafka/streams/StateListenerStub.java:
##########
@@ -0,0 +1,37 @@
+/*
+ * 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.streams;
+
+import java.util.HashMap;
+import java.util.Map;
+
+public class StateListenerStub implements KafkaStreams.StateListener {
+ public int numChanges = 0;
Review Comment:
I don't think this class should be moved to `test-fixtures`, as `QueryableStateIntegrationTest` and `KafkaStreamsTest` have different requirements for it. For example, all that `KafkaStreamsTest` needs is a simple stub like the one below:
```java
public static class StateListenerStub implements KafkaStreams.StateListener {
private int numChanges = 0;
@Override
public void onChange(final KafkaStreams.State newState,
final KafkaStreams.State oldState) {
numChanges++;
}
}
```
--
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] KAFKA-20553: Eliminate the dependencies on sourceSets.test.output for streams [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #22521:
URL: https://github.com/apache/kafka/pull/22521#discussion_r3491032876
##########
build.gradle:
##########
@@ -3043,16 +3051,34 @@ project(':streams:integration-tests') {
archivesName = "kafka-streams-integration-tests"
}
+ apply plugin: 'java-test-fixtures'
Review Comment:
I guess the purpose of adding testfixtures to `streams:integration-tests` was to let the tools module use `EmbeddedKafkaCluster`. However, I grepped the usages and it seems they can be replaced by `ClusterInstance`, which would prevent introducing unnecessary module complexity
--
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] KAFKA-20553: Eliminate the dependencies on sourceSets.test.output for streams [kafka]
Posted by "FrankYang0529 (via GitHub)" <gi...@apache.org>.
FrankYang0529 commented on PR #22521:
URL: https://github.com/apache/kafka/pull/22521#issuecomment-5098603753
> why to move `StreamsUpgradeTest` and its related classes to testfixture?
The `StreamsUpgradeTestIntegrationTest` still uses 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
Re: [PR] KAFKA-20553: Eliminate the dependencies on sourceSets.test.output for streams [kafka]
Posted by "FrankYang0529 (via GitHub)" <gi...@apache.org>.
FrankYang0529 commented on code in PR #22521:
URL: https://github.com/apache/kafka/pull/22521#discussion_r3492347811
##########
build.gradle:
##########
@@ -3043,16 +3051,34 @@ project(':streams:integration-tests') {
archivesName = "kafka-streams-integration-tests"
}
+ apply plugin: 'java-test-fixtures'
Review Comment:
https://github.com/apache/kafka/pull/22701
--
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] KAFKA-20553: Eliminate the dependencies on sourceSets.test.output for streams [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #22521:
URL: https://github.com/apache/kafka/pull/22521#discussion_r3474859811
##########
build.gradle:
##########
@@ -3081,6 +3109,14 @@ project(':streams:integration-tests') {
srcDirs = []
}
}
+ testFixtures {
+ java {
+ srcDirs = []
+ }
+ scala {
Review Comment:
> StreamToTableJoinScalaIntegrationTestImplicitSerdes which tests Scala implicits.
nice point. I'd like to open another PR to handle it first.
1. rewrite `StreamToTableJoinScalaIntegrationTestImplicitSerdes` using `TopologyTestDriver` to avoid using a true kafka cluster
2. move it to the streams-scala module
3. remove `StreamToTableJoinScalaIntegrationTestBase`, `StreamToTableJoinTestData`, and `WordCountTest`
4. remove the scala dependency from streams:integration-tests 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] KAFKA-20553: Eliminate the dependencies on sourceSets.test.output for streams [kafka]
Posted by "FrankYang0529 (via GitHub)" <gi...@apache.org>.
FrankYang0529 commented on code in PR #22521:
URL: https://github.com/apache/kafka/pull/22521#discussion_r3657338718
##########
streams/src/testFixtures/java/org/apache/kafka/streams/StateListenerStub.java:
##########
@@ -0,0 +1,37 @@
+/*
+ * 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.streams;
+
+import java.util.HashMap;
+import java.util.Map;
+
+public class StateListenerStub implements KafkaStreams.StateListener {
+ public int numChanges = 0;
Review Comment:
Thanks for the review. Updated 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
Re: [PR] KAFKA-20553: Eliminate the dependencies on sourceSets.test.output for streams [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #22521:
URL: https://github.com/apache/kafka/pull/22521#issuecomment-4803344476
@FrankYang0529 please fix the conflicts
--
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] KAFKA-20553: Eliminate the dependencies on sourceSets.test.output for streams [kafka]
Posted by "mjsax (via GitHub)" <gi...@apache.org>.
mjsax commented on code in PR #22521:
URL: https://github.com/apache/kafka/pull/22521#discussion_r3472172859
##########
build.gradle:
##########
@@ -3081,6 +3109,14 @@ project(':streams:integration-tests') {
srcDirs = []
}
}
+ testFixtures {
+ java {
+ srcDirs = []
+ }
+ scala {
Review Comment:
I think it would reduce test coverage, particularly `StreamToTableJoinScalaIntegrationTestImplicitSerdes` which tests Scala implicits.
But I can frankly not remember when I reviewed a Scala PR the last time, so the risk of reducing test coverage should be very small.
--
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] KAFKA-20553: Eliminate the dependencies on sourceSets.test.output for streams [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 merged PR #22521:
URL: https://github.com/apache/kafka/pull/22521
--
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] KAFKA-20553: Eliminate the dependencies on sourceSets.test.output for streams [kafka]
Posted by "FrankYang0529 (via GitHub)" <gi...@apache.org>.
FrankYang0529 commented on PR #22521:
URL: https://github.com/apache/kafka/pull/22521#issuecomment-4762202125
> There are many tests that have been moved to testfixtures. Would you mind explaining the reasoning? IIRC, the test files should be in the test folder
These classes are referenced by test suite in integration tests:
* ForeignKeyJoinSuite
* KTableKTableForeignKeyJoinScenarioTest.java
* CombinedKeySchemaTest.java
* ResponseJoinProcessorSupplierTest.java
* SubscriptionResponseWrapperSerdeTest.java
* SubscriptionWrapperSerdeTest.java
* StoreQuerySuite
* CompositeReadOnlyKeyValueStoreTest.java
* CompositeReadOnlySessionStoreTest.java
* CompositeReadOnlyWindowStoreTest.java
* GlobalStateStoreProviderTest.java
* StreamThreadStateStoreProviderTest.java
* WrappingStoreProviderTest.java
Following classes are referenced by above test classes:
* InternalMockProcessorContext.java
* MockInternalProcessorContext.java
* MockRecordCollector.java
* NoOpReadOnlyStore.java
* ReadOnlySessionStoreStub.java
* StateStoreProviderStub.java
* KeyValueIteratorStub.java
* StateManagerStub.java
* MockStreamsMetrics.java
* NoOpWindowStore.java
* ReadOnlyWindowStoreStub.java
--
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] KAFKA-20553: Eliminate the dependencies on sourceSets.test.output for streams [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #22521:
URL: https://github.com/apache/kafka/pull/22521#issuecomment-4775370068
> These classes are referenced by test suite in integration tests:
Would it be better to split `ForeignKeyJoinSuite` into both the `streams` and `integration-tests` modules? Stashing actual tests in `testfixtures` is a clear anti-pattern
--
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] KAFKA-20553: Eliminate the dependencies on sourceSets.test.output for streams [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #22521:
URL: https://github.com/apache/kafka/pull/22521#discussion_r3469550278
##########
build.gradle:
##########
@@ -3081,6 +3109,14 @@ project(':streams:integration-tests') {
srcDirs = []
}
}
+ testFixtures {
+ java {
+ srcDirs = []
+ }
+ scala {
Review Comment:
There are only four files left that require Scala: `StreamToTableJoinScalaIntegrationTestImplicitSerdes`, `StreamToTableJoinScalaIntegrationTestBase`, `StreamToTableJoinTestData`, and `WordCountTest`. They seem to be quite old, and we already have similar tests like `WordCountDemoTest`. Since we are phasing out Scala, I would prefer to just remove them.
@mjsax 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