[jira] [Commented] (KAFKA-6474) Rewrite test to use new public TopologyTestDriver
[ https://issues.apache.org/jira/browse/KAFKA-6474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16843287#comment-16843287 ] ASF GitHub Bot commented on KAFKA-6474: --- guozhangwang commented on pull request #6732: KAFKA-6474: remove KStreamTestDriver URL: https://github.com/apache/kafka/pull/6732 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rewrite test to use new public TopologyTestDriver > - > > Key: KAFKA-6474 > URL: https://issues.apache.org/jira/browse/KAFKA-6474 > Project: Kafka > Issue Type: Improvement > Components: streams, unit tests >Affects Versions: 1.1.0 >Reporter: Matthias J. Sax >Assignee: Filipe Agapito >Priority: Major > Labels: newbie > > With KIP-247 we added public TopologyTestDriver. We should rewrite out own > test to use this new test driver and remove the two classes > ProcessorTopoogyTestDriver and KStreamTestDriver. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (KAFKA-6474) Rewrite test to use new public TopologyTestDriver
[ https://issues.apache.org/jira/browse/KAFKA-6474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16840537#comment-16840537 ] John Roesler commented on KAFKA-6474: - Hey, [~h314to], I just sent https://github.com/apache/kafka/pull/6732 as well. Sorry for stepping on your toes, I was tracking down some other problem and needed to remove the class for other reasons. I sent the PR before someone reminded me about this ticket. > Rewrite test to use new public TopologyTestDriver > - > > Key: KAFKA-6474 > URL: https://issues.apache.org/jira/browse/KAFKA-6474 > Project: Kafka > Issue Type: Improvement > Components: streams, unit tests >Affects Versions: 1.1.0 >Reporter: Matthias J. Sax >Assignee: Filipe Agapito >Priority: Major > Labels: newbie > > With KIP-247 we added public TopologyTestDriver. We should rewrite out own > test to use this new test driver and remove the two classes > ProcessorTopoogyTestDriver and KStreamTestDriver. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (KAFKA-6474) Rewrite test to use new public TopologyTestDriver
[ https://issues.apache.org/jira/browse/KAFKA-6474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16812597#comment-16812597 ] Guozhang Wang commented on KAFKA-6474: -- Hi [~h314to] could you provide an update on the remaining of PRs you are planning on to remove the deprecated KStreamTestDriver? > Rewrite test to use new public TopologyTestDriver > - > > Key: KAFKA-6474 > URL: https://issues.apache.org/jira/browse/KAFKA-6474 > Project: Kafka > Issue Type: Improvement > Components: streams, unit tests >Affects Versions: 1.1.0 >Reporter: Matthias J. Sax >Assignee: Filipe Agapito >Priority: Major > Labels: newbie > > With KIP-247 we added public TopologyTestDriver. We should rewrite out own > test to use this new test driver and remove the two classes > ProcessorTopoogyTestDriver and KStreamTestDriver. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (KAFKA-6474) Rewrite test to use new public TopologyTestDriver
[ https://issues.apache.org/jira/browse/KAFKA-6474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16768792#comment-16768792 ] Matthias J. Sax commented on KAFKA-6474: [~h314to] Just merge your latest PR. How many more do we need to resolve this completely (not urgent, but I would like to resolve this eventually). Thanks a lot for all your work! > Rewrite test to use new public TopologyTestDriver > - > > Key: KAFKA-6474 > URL: https://issues.apache.org/jira/browse/KAFKA-6474 > Project: Kafka > Issue Type: Improvement > Components: streams, unit tests >Affects Versions: 1.1.0 >Reporter: Matthias J. Sax >Assignee: Filipe Agapito >Priority: Major > Labels: newbie > > With KIP-247 we added public TopologyTestDriver. We should rewrite out own > test to use this new test driver and remove the two classes > ProcessorTopoogyTestDriver and KStreamTestDriver. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (KAFKA-6474) Rewrite test to use new public TopologyTestDriver
[ https://issues.apache.org/jira/browse/KAFKA-6474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16768790#comment-16768790 ] ASF GitHub Bot commented on KAFKA-6474: --- mjsax commented on pull request #5433: KAFKA-6474: Rewrite tests to use new public TopologyTestDriver [part 4] URL: https://github.com/apache/kafka/pull/5433 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rewrite test to use new public TopologyTestDriver > - > > Key: KAFKA-6474 > URL: https://issues.apache.org/jira/browse/KAFKA-6474 > Project: Kafka > Issue Type: Improvement > Components: streams, unit tests >Affects Versions: 1.1.0 >Reporter: Matthias J. Sax >Assignee: Filipe Agapito >Priority: Major > Labels: newbie > > With KIP-247 we added public TopologyTestDriver. We should rewrite out own > test to use this new test driver and remove the two classes > ProcessorTopoogyTestDriver and KStreamTestDriver. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (KAFKA-6474) Rewrite test to use new public TopologyTestDriver
[ https://issues.apache.org/jira/browse/KAFKA-6474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16749234#comment-16749234 ] Matthias J. Sax commented on KAFKA-6474: Thanks [~h314to]. SGTM. \cc [~guozhang] and [~Yohan123] > Rewrite test to use new public TopologyTestDriver > - > > Key: KAFKA-6474 > URL: https://issues.apache.org/jira/browse/KAFKA-6474 > Project: Kafka > Issue Type: Improvement > Components: streams, unit tests >Affects Versions: 1.1.0 >Reporter: Matthias J. Sax >Assignee: Filipe Agapito >Priority: Major > Labels: newbie > > With KIP-247 we added public TopologyTestDriver. We should rewrite out own > test to use this new test driver and remove the two classes > ProcessorTopoogyTestDriver and KStreamTestDriver. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (KAFKA-6474) Rewrite test to use new public TopologyTestDriver
[ https://issues.apache.org/jira/browse/KAFKA-6474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16749129#comment-16749129 ] Filipe Agapito commented on KAFKA-6474: --- Hi [~mjsax]. Thanks for pinging me. Sorry, I haven't been able to contribute lately. I have an open PR which removes a few more instances of KStreamTest driver. I also have some work done on the remaining classes which use KStreamTestDriver (only two remain after that PR, if I'm not mistaken). To avoid duplication of efforts, if you will give me just 1 or 2 days more I can fix and submit my work. I think [https://github.com/apache/kafka/pull/5433] should be easy enough to fix. I'll just rebase it, fix the conflicts, and make the changes you suggested (I'm doing that now). If you can't spare the additional couple of days and this is blocking you in any way I'm ok with you proceeding as discussed in the other ticket. > Rewrite test to use new public TopologyTestDriver > - > > Key: KAFKA-6474 > URL: https://issues.apache.org/jira/browse/KAFKA-6474 > Project: Kafka > Issue Type: Improvement > Components: streams, unit tests >Affects Versions: 1.1.0 >Reporter: Matthias J. Sax >Assignee: Filipe Agapito >Priority: Major > Labels: newbie > > With KIP-247 we added public TopologyTestDriver. We should rewrite out own > test to use this new test driver and remove the two classes > ProcessorTopoogyTestDriver and KStreamTestDriver. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (KAFKA-6474) Rewrite test to use new public TopologyTestDriver
[ https://issues.apache.org/jira/browse/KAFKA-6474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16748947#comment-16748947 ] Matthias J. Sax commented on KAFKA-6474: [~h314to] We are thinking about resolving this ticket and track the remaining work via https://issues.apache.org/jira/browse/KAFKA-7850 – thoughts (please see the comments on the other ticket)? > Rewrite test to use new public TopologyTestDriver > - > > Key: KAFKA-6474 > URL: https://issues.apache.org/jira/browse/KAFKA-6474 > Project: Kafka > Issue Type: Improvement > Components: streams, unit tests >Affects Versions: 1.1.0 >Reporter: Matthias J. Sax >Assignee: Filipe Agapito >Priority: Major > Labels: newbie > > With KIP-247 we added public TopologyTestDriver. We should rewrite out own > test to use this new test driver and remove the two classes > ProcessorTopoogyTestDriver and KStreamTestDriver. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (KAFKA-6474) Rewrite test to use new public TopologyTestDriver
[
https://issues.apache.org/jira/browse/KAFKA-6474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16511455#comment-16511455
]
ASF GitHub Bot commented on KAFKA-6474:
---
guozhangwang closed pull request #4986: KAFKA-6474: Rewrite tests to use new
public TopologyTestDriver [part 2]
URL: https://github.com/apache/kafka/pull/4986
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/streams/src/test/java/org/apache/kafka/streams/TopologyTestDriverWrapper.java
b/streams/src/test/java/org/apache/kafka/streams/TopologyTestDriverWrapper.java
new file mode 100644
index 000..bec4b5f79ff
--- /dev/null
+++
b/streams/src/test/java/org/apache/kafka/streams/TopologyTestDriverWrapper.java
@@ -0,0 +1,70 @@
+/*
+ * 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 org.apache.kafka.streams.errors.StreamsException;
+import org.apache.kafka.streams.processor.ProcessorContext;
+import org.apache.kafka.streams.processor.internals.ProcessorContextImpl;
+import org.apache.kafka.streams.processor.internals.ProcessorNode;
+
+import java.util.Properties;
+
+/**
+ * This class provides access to {@link TopologyTestDriver} protected methods.
+ * It should only be used for internal testing, in the rare occasions where the
+ * necessary functionality is not supported by {@link TopologyTestDriver}.
+ */
+public class TopologyTestDriverWrapper extends TopologyTestDriver {
+
+
+public TopologyTestDriverWrapper(final Topology topology,
+ final Properties config) {
+super(topology, config);
+}
+
+/**
+ * Get the processor context, setting the processor whose name is given as
current node
+ *
+ * @param processorName processor name to set as current node
+ * @return the processor context
+ */
+public ProcessorContext setCurrentNodeForProcessorContext(final String
processorName) {
+final ProcessorContext context = task.context();
+((ProcessorContextImpl)
context).setCurrentNode(getProcessor(processorName));
+return context;
+}
+
+/**
+ * Get a processor by name
+ *
+ * @param name the name to search for
+ * @return the processor matching the search name
+ */
+public ProcessorNode getProcessor(final String name) {
+for (final ProcessorNode node : processorTopology.processors()) {
+if (node.name().equals(name)) {
+return node;
+}
+}
+for (final ProcessorNode node : globalTopology.processors()) {
+if (node.name().equals(name)) {
+return node;
+}
+}
+throw new StreamsException("Could not find a processor named '" + name
+ "'");
+}
+}
diff --git
a/streams/src/test/java/org/apache/kafka/streams/kstream/internals/KTableFilterTest.java
b/streams/src/test/java/org/apache/kafka/streams/kstream/internals/KTableFilterTest.java
index c37078df99c..2cf192b9f45 100644
---
a/streams/src/test/java/org/apache/kafka/streams/kstream/internals/KTableFilterTest.java
+++
b/streams/src/test/java/org/apache/kafka/streams/kstream/internals/KTableFilterTest.java
@@ -16,45 +16,40 @@
*/
package org.apache.kafka.streams.kstream.internals;
-import org.apache.kafka.common.serialization.Serde;
+import org.apache.kafka.common.serialization.IntegerSerializer;
import org.apache.kafka.common.serialization.Serdes;
+import org.apache.kafka.common.serialization.StringSerializer;
import org.apache.kafka.common.utils.Bytes;
-import org.apache.kafka.streams.kstream.Consumed;
import org.apache.kafka.streams.StreamsBuilder;
import org.apache.kafka.streams.Topology;
+import org.apache.kafka.streams.TopologyTestDriver;
+import org.apache.kafka.streams.TopologyTestDriverWrapper;
+import org.apache.kafka.streams.TopologyWrapper;
+import org.apache.kafka.streams.kstream.Consumed;
import
[jira] [Commented] (KAFKA-6474) Rewrite test to use new public TopologyTestDriver
[
https://issues.apache.org/jira/browse/KAFKA-6474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16484168#comment-16484168
]
ASF GitHub Bot commented on KAFKA-6474:
---
guozhangwang closed pull request #5052: KAFKA-6474: Rewrite tests to use new
public TopologyTestDriver [part 3]
URL: https://github.com/apache/kafka/pull/5052
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamWindowReduceTest.java
b/streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamWindowReduceTest.java
index aa2397170f6..4ae2f76698b 100644
---
a/streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamWindowReduceTest.java
+++
b/streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamWindowReduceTest.java
@@ -17,25 +17,32 @@
package org.apache.kafka.streams.kstream.internals;
import org.apache.kafka.common.serialization.Serdes;
-import org.apache.kafka.streams.kstream.Consumed;
+import org.apache.kafka.common.serialization.StringSerializer;
import org.apache.kafka.streams.StreamsBuilder;
+import org.apache.kafka.streams.TopologyTestDriver;
+import org.apache.kafka.streams.kstream.Consumed;
import org.apache.kafka.streams.kstream.Reducer;
import org.apache.kafka.streams.kstream.Serialized;
import org.apache.kafka.streams.kstream.TimeWindows;
import
org.apache.kafka.streams.processor.internals.testutil.LogCaptureAppender;
-import org.apache.kafka.test.KStreamTestDriver;
-import org.apache.kafka.test.TestUtils;
+import org.apache.kafka.streams.test.ConsumerRecordFactory;
+import org.apache.kafka.test.StreamsTestUtils;
import org.junit.Test;
+import java.util.Properties;
+
import static org.apache.kafka.test.StreamsTestUtils.getMetricByName;
import static org.hamcrest.CoreMatchers.hasItem;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
public class KStreamWindowReduceTest {
+
+private final Properties props =
StreamsTestUtils.topologyTestConfig(Serdes.String(), Serdes.String());
+private final ConsumerRecordFactory recordFactory = new
ConsumerRecordFactory<>(new StringSerializer(), new StringSerializer());
+
@Test
public void shouldLogAndMeterOnNullKey() {
-final KStreamTestDriver driver = new KStreamTestDriver();
final StreamsBuilder builder = new StreamsBuilder();
builder
@@ -49,14 +56,14 @@ public String apply(final String value1, final String
value2) {
}
});
-driver.setUp(builder, TestUtils.tempDirectory(), 0);
-final LogCaptureAppender appender =
LogCaptureAppender.createAndRegister();
-driver.process("TOPIC", null, "asdf");
-driver.flushState();
-LogCaptureAppender.unregister(appender);
+try (final TopologyTestDriver driver = new
TopologyTestDriver(builder.build(), props)) {
+final LogCaptureAppender appender =
LogCaptureAppender.createAndRegister();
+driver.pipeInput(recordFactory.create("TOPIC", null, "asdf"));
+LogCaptureAppender.unregister(appender);
-assertEquals(1.0,
getMetricByName(driver.context().metrics().metrics(), "skipped-records-total",
"stream-metrics").metricValue());
-assertThat(appender.getMessages(), hasItem("Skipping record due to
null key. value=[asdf] topic=[TOPIC] partition=[-1] offset=[-1]"));
+assertEquals(1.0, getMetricByName(driver.metrics(),
"skipped-records-total", "stream-metrics").metricValue());
+assertThat(appender.getMessages(), hasItem("Skipping record due to
null key. value=[asdf] topic=[TOPIC] partition=[0] offset=[0]"));
+}
}
}
diff --git
a/streams/src/test/java/org/apache/kafka/streams/kstream/internals/KTableMapKeysTest.java
b/streams/src/test/java/org/apache/kafka/streams/kstream/internals/KTableMapKeysTest.java
index 14552d6b325..081c6a069aa 100644
---
a/streams/src/test/java/org/apache/kafka/streams/kstream/internals/KTableMapKeysTest.java
+++
b/streams/src/test/java/org/apache/kafka/streams/kstream/internals/KTableMapKeysTest.java
@@ -17,40 +17,30 @@
package org.apache.kafka.streams.kstream.internals;
-import org.apache.kafka.common.serialization.Serde;
+import org.apache.kafka.common.serialization.IntegerSerializer;
import org.apache.kafka.common.serialization.Serdes;
-import org.apache.kafka.streams.kstream.Consumed;
+import org.apache.kafka.common.serialization.StringSerializer;
import org.apache.kafka.streams.StreamsBuilder;
+import org.apache.kafka.streams.TopologyTestDriver;
+import org.apache.kafka.streams.kstream.Consumed;
[jira] [Commented] (KAFKA-6474) Rewrite test to use new public TopologyTestDriver
[ https://issues.apache.org/jira/browse/KAFKA-6474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16482758#comment-16482758 ] ASF GitHub Bot commented on KAFKA-6474: --- h314to opened a new pull request #5052: KAFKA-6474: Rewrite tests to use new public TopologyTestDriver [part 3] URL: https://github.com/apache/kafka/pull/5052 This PR is a further step towards the complete replacement of `KStreamTestDriver` with `TopologyTestDriver`. These straightforward changes were split from another [PR](https://github.com/apache/kafka/pull/4986) to simplify the review process. * Refactor: - KStreamWindowReduceTest - KTableMapKeysTest - SessionWindowedKStreamImplTest - TimeWindowedKStreamImplTest This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rewrite test to use new public TopologyTestDriver > - > > Key: KAFKA-6474 > URL: https://issues.apache.org/jira/browse/KAFKA-6474 > Project: Kafka > Issue Type: Improvement > Components: streams, unit tests >Affects Versions: 1.1.0 >Reporter: Matthias J. Sax >Assignee: Filipe Agapito >Priority: Major > Labels: beginner, newbie > > With KIP-247 we added public TopologyTestDriver. We should rewrite out own > test to use this new test driver and remove the two classes > ProcessorTopoogyTestDriver and KStreamTestDriver. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (KAFKA-6474) Rewrite test to use new public TopologyTestDriver
[ https://issues.apache.org/jira/browse/KAFKA-6474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16468686#comment-16468686 ] ASF GitHub Bot commented on KAFKA-6474: --- h314to opened a new pull request #4986: KAFKA-6474: Rewrite tests to use new public TopologyTestDriver [part 2] URL: https://github.com/apache/kafka/pull/4986 This PR is a further step towards the complete replacement of `KStreamTestDriver` with `TopologyTestDriver`. Currently there's only a few tests in `KTableAggregateTest` which don't work with `TopologyTestDriver` . Everything else has now been migrated to the new driver. * Refactor: - TimeWindowedKStreamImplTest - SessionWindowedKStreamImplTest - KTableMapKeysTest - KTableFilterTest - KTableKTableInnerJoinTest - KTableSourceTest - KTableMapValuesTest - KTableKTableOuterJoinTest - KTableKTableLeftJoinTest - KStreamWindowReduceTest - KTableImplTest. - KTableAggregateTest (partial) * Rename TopologyTestDriverWrapper to TopologyTestDriverAccessor. * Add task, processorTopology, and globalTopology access to TopologyTestDriverAccessor * Add condition to prevent NPE in ProcessorContextImpl This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rewrite test to use new public TopologyTestDriver > - > > Key: KAFKA-6474 > URL: https://issues.apache.org/jira/browse/KAFKA-6474 > Project: Kafka > Issue Type: Improvement > Components: streams, unit tests >Affects Versions: 1.1.0 >Reporter: Matthias J. Sax >Assignee: Filipe Agapito >Priority: Major > Labels: beginner, newbie > > With KIP-247 we added public TopologyTestDriver. We should rewrite out own > test to use this new test driver and remove the two classes > ProcessorTopoogyTestDriver and KStreamTestDriver. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (KAFKA-6474) Rewrite test to use new public TopologyTestDriver
[ https://issues.apache.org/jira/browse/KAFKA-6474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16456764#comment-16456764 ] ASF GitHub Bot commented on KAFKA-6474: --- h314to opened a new pull request #4939: KAFKA-6474: Rewrite tests to use new public TopologyTestDriver [cleanup] URL: https://github.com/apache/kafka/pull/4939 This implements the suggestions made after the previous [PR](https://github.com/apache/kafka/pull/4832) for KAFKA-6474 was merged. The majority of changes deals with using try-with-resources and a new method in `StreamsTestUtils` to set the test properties and instantiate the `TopologyTestDriver`, thus allowing the removal of the cumbersome `@Before` and `@After` methods. I also replaced `stringSerde` and `intSerde` variables with (almost equally succinct) inline calls to `Serdes.String()` and `Serdes.Integer()`. * Add method to create test properties to StreamsTestUtils * Make TopologyTestDriver protected constructor package-private * Add comment suggesting the use of TopologyTestDriver to KStreamTestDriver * Cleanup: - GlobalKTableJoinsTest - KGroupedStreamImplTest - KGroupedTableImplTest - KStreamBranchTest - KStreamFilterTest - KStreamFlatMapTest - KStreamFlatMapValuesTest - KStreamForeachTest - KStreamGlobalKTableJoinTest - KStreamGlobalKTableLeftJoinTest - KStreamImplTest - KStreamKStreamJoinTest - KStreamKStreamLeftJoinTest - KStreamGlobalKTableLeftJoinTest - KStreamKTableJoinTest - KStreamKTableLeftJoinTest - KStreamMapTest - KStreamMapValuesTest - KStreamPeekTest - StreamsBuilderTest - KStreamSelectKeyTest - KStreamTransformTest - KStreamTransformValuesTest - KStreamWindowAggregateTest - KTableForeachTest This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rewrite test to use new public TopologyTestDriver > - > > Key: KAFKA-6474 > URL: https://issues.apache.org/jira/browse/KAFKA-6474 > Project: Kafka > Issue Type: Improvement > Components: streams, unit tests >Affects Versions: 1.1.0 >Reporter: Matthias J. Sax >Assignee: Filipe Agapito >Priority: Major > Labels: beginner, newbie > > With KIP-247 we added public TopologyTestDriver. We should rewrite out own > test to use this new test driver and remove the two classes > ProcessorTopoogyTestDriver and KStreamTestDriver. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (KAFKA-6474) Rewrite test to use new public TopologyTestDriver
[ https://issues.apache.org/jira/browse/KAFKA-6474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16428837#comment-16428837 ] John Roesler commented on KAFKA-6474: - Thanks for the PR. I'll review it and comment as soon as I can. -John > Rewrite test to use new public TopologyTestDriver > - > > Key: KAFKA-6474 > URL: https://issues.apache.org/jira/browse/KAFKA-6474 > Project: Kafka > Issue Type: Improvement > Components: streams, unit tests >Affects Versions: 1.1.0 >Reporter: Matthias J. Sax >Assignee: Filipe Agapito >Priority: Major > Labels: beginner, newbie > > With KIP-247 we added public TopologyTestDriver. We should rewrite out own > test to use this new test driver and remove the two classes > ProcessorTopoogyTestDriver and KStreamTestDriver. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (KAFKA-6474) Rewrite test to use new public TopologyTestDriver
[ https://issues.apache.org/jira/browse/KAFKA-6474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16427805#comment-16427805 ] ASF GitHub Bot commented on KAFKA-6474: --- h314to opened a new pull request #4832: KAFKA-6474: [WIP] Rewrite tests to use new public TopologyTestDriver URL: https://github.com/apache/kafka/pull/4832 **Please do not merge yet. This is still work in progress.** * Remove ProcessorTopologyTestDriver from TopologyTest * Fix ProcessorTopologyTest * Remove ProcessorTopologyTestDriver and InternalTopologyAccessor * Partially refactored StreamsBuilderTest but missing one test * Refactor KStreamBuilderTest * Refactor AbstractStreamTest * Further cleanup of AbstractStreamTest * Refactor GlobalKTableJoinsTest * Refactor InternalStreamsBuilderTest * Fix circular dependency in build.gradle * Refactor KGroupedStreamImplTest * Partial modifications to KGroupedTableImplTest * Refactor KGroupedTableImplTest * Refactor KStreamBranchTest * Refactor KStreamFilterTest * Refactor KStreamFlatMapTest KStreamFlatMapValuesTest * Refactor KStreamForeachTest * Refactor KStreamGlobalKTableJoinTest * Refactor KStreamGlobalKTableLeftJoinTest * Refactor KStreamImplTest * Refactor KStreamImplTest * Refactor KStreamKStreamJoinTest * Refactor KStreamKStreamLeftJoinTest * Refactor KStreamKTableJoinTest * Refactor KStreamKTableLeftJoinTest * Refactor KStreamMapTest and KStreamMapValuesTest * Refactor KStreamPeekTest and KStreamTransformTest * Refactor KStreamSelectKeyTest * Refactor KStreamTransformValuesTest * Refactor KStreamWindowAggregateTest * Add Depercation anotation to KStreamTestDriver and rollback failing tests in StreamsBuilderTest and KTableAggregateTest * Refactor KTableFilterTest * Refactor KTableForeachTest * Add getter for ProcessorTopology, and simplify tests in StreamsBuilderTest * Refactor KTableImplTest * Remove unused imports * Refactor KTableAggregateTest This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Rewrite test to use new public TopologyTestDriver > - > > Key: KAFKA-6474 > URL: https://issues.apache.org/jira/browse/KAFKA-6474 > Project: Kafka > Issue Type: Improvement > Components: streams, unit tests >Affects Versions: 1.1.0 >Reporter: Matthias J. Sax >Assignee: Filipe Agapito >Priority: Major > Labels: beginner, newbie > > With KIP-247 we added public TopologyTestDriver. We should rewrite out own > test to use this new test driver and remove the two classes > ProcessorTopoogyTestDriver and KStreamTestDriver. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (KAFKA-6474) Rewrite test to use new public TopologyTestDriver
[
https://issues.apache.org/jira/browse/KAFKA-6474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16427801#comment-16427801
]
Filipe Agapito commented on KAFKA-6474:
---
Hi John. Thanks for the warning. Great work! Breaking the cycle on the other
side much cleaner. Unfortunately, I started having some issues with tests after
rebasing on the current trunk. For instance, running:
{noformat}
./gradlew streams:test -Dtest.single=KStreamKStreamJoinTest
{noformat}
I would get:
{noformat}
org.apache.kafka.streams.kstream.internals.KStreamKStreamJoinTest >
initializationError FAILED
java.lang.NoClassDefFoundError:
Lorg/apache/kafka/streams/test/ConsumerRecordFactory;
Caused by:
java.lang.ClassNotFoundException:
org.apache.kafka.streams.test.ConsumerRecordFactory
1 test completed, 1 failed
{noformat}
So, following your lead, I added the following to build.gradle:
{noformat}
diff --git a/build.gradle b/build.gradle
index 33fa7a750..ca8b0ed4b 100644
--- a/build.gradle
+++ b/build.gradle
@@ -929,6 +929,8 @@ project(':streams') {
testCompile libs.easymock
testCompile libs.bcpkix
+// testRuntimeOnly makes dependencies available at test runtime, while
preventing the cyclic dependency as noted above
+testRuntimeOnly project(':streams:test-utils')
testRuntime libs.slf4jlog4j
}
{noformat}
Like before I don't know if this is the best solution, but at least it keeps
the tests from failing. I'll be creating a [WIP] pull request with my current
changes shortly, to test on Jenkins and get some early feedback as advised.
Hopefully the tests all pass there too.
I'm close to finishing the migration to the new test driver, but I'm still
missing about 8 classes. There are also a few tests which still don't work well
with the new TopologyTestDriver (I'm still relying on KStreamTestDriver for
those), and I'll have to figure what's wrong before I'm done.
> Rewrite test to use new public TopologyTestDriver
> -
>
> Key: KAFKA-6474
> URL: https://issues.apache.org/jira/browse/KAFKA-6474
> Project: Kafka
> Issue Type: Improvement
> Components: streams, unit tests
>Affects Versions: 1.1.0
>Reporter: Matthias J. Sax
>Assignee: Filipe Agapito
>Priority: Major
> Labels: beginner, newbie
>
> With KIP-247 we added public TopologyTestDriver. We should rewrite out own
> test to use this new test driver and remove the two classes
> ProcessorTopoogyTestDriver and KStreamTestDriver.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (KAFKA-6474) Rewrite test to use new public TopologyTestDriver
[ https://issues.apache.org/jira/browse/KAFKA-6474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16427267#comment-16427267 ] John Roesler commented on KAFKA-6474: - Hey Filipe, Just to keep you in the loop, we discovered a problem with the dependency strategy we previously discussed, but it's fixed by going back to the regular dependency from `test-utils` to `streams` and declaring a `testCompileOnly` dependency from `streams` to `test-utils`. (See [https://github.com/apache/kafka/pull/4821)] Relevant to you is just that you'll again see a conflict next time you rebase. Hope you're well, -John > Rewrite test to use new public TopologyTestDriver > - > > Key: KAFKA-6474 > URL: https://issues.apache.org/jira/browse/KAFKA-6474 > Project: Kafka > Issue Type: Improvement > Components: streams, unit tests >Affects Versions: 1.1.0 >Reporter: Matthias J. Sax >Assignee: Filipe Agapito >Priority: Major > Labels: beginner, newbie > > With KIP-247 we added public TopologyTestDriver. We should rewrite out own > test to use this new test driver and remove the two classes > ProcessorTopoogyTestDriver and KStreamTestDriver. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (KAFKA-6474) Rewrite test to use new public TopologyTestDriver
[ https://issues.apache.org/jira/browse/KAFKA-6474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16420589#comment-16420589 ] John Roesler commented on KAFKA-6474: - Yeah, I agree. Just prefix it as WIP, and no one will judge ;) I personally like to collect a high-level round of feedback asap for my PRs. > Rewrite test to use new public TopologyTestDriver > - > > Key: KAFKA-6474 > URL: https://issues.apache.org/jira/browse/KAFKA-6474 > Project: Kafka > Issue Type: Improvement > Components: streams, unit tests >Affects Versions: 1.1.0 >Reporter: Matthias J. Sax >Assignee: Filipe Agapito >Priority: Major > Labels: beginner, newbie > > With KIP-247 we added public TopologyTestDriver. We should rewrite out own > test to use this new test driver and remove the two classes > ProcessorTopoogyTestDriver and KStreamTestDriver. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (KAFKA-6474) Rewrite test to use new public TopologyTestDriver
[ https://issues.apache.org/jira/browse/KAFKA-6474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16419456#comment-16419456 ] Guozhang Wang commented on KAFKA-6474: -- Definitely :) That will let reviewers to provide some early feedbacks to your changes. > Rewrite test to use new public TopologyTestDriver > - > > Key: KAFKA-6474 > URL: https://issues.apache.org/jira/browse/KAFKA-6474 > Project: Kafka > Issue Type: Improvement > Components: streams, unit tests >Affects Versions: 1.1.0 >Reporter: Matthias J. Sax >Assignee: Filipe Agapito >Priority: Major > Labels: beginner, newbie > > With KIP-247 we added public TopologyTestDriver. We should rewrite out own > test to use this new test driver and remove the two classes > ProcessorTopoogyTestDriver and KStreamTestDriver. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (KAFKA-6474) Rewrite test to use new public TopologyTestDriver
[ https://issues.apache.org/jira/browse/KAFKA-6474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16419175#comment-16419175 ] Filipe Agapito commented on KAFKA-6474: --- Hi John. I hope you are also doing well. Thanks for the warning! Since fixing this issue requires changing a lot of files I've been rebasing frequently to keep conflicts to a minimum, so I already have your commit. Nice to see that it passed all checks and that it is all working fine. By the way, I've already migrated a lot of tests from KStreamTestDriver and ProcessorTopoogyTestDriver to TopologyTestDriver (about 40 classes so far). Should I open a pull request, marked as [WIP], to start getting some feedback, or is it preferable to wait until I finish the refactor (I'm missing about 10 classes) before opening the PR? > Rewrite test to use new public TopologyTestDriver > - > > Key: KAFKA-6474 > URL: https://issues.apache.org/jira/browse/KAFKA-6474 > Project: Kafka > Issue Type: Improvement > Components: streams, unit tests >Affects Versions: 1.1.0 >Reporter: Matthias J. Sax >Assignee: Filipe Agapito >Priority: Major > Labels: beginner, newbie > > With KIP-247 we added public TopologyTestDriver. We should rewrite out own > test to use this new test driver and remove the two classes > ProcessorTopoogyTestDriver and KStreamTestDriver. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (KAFKA-6474) Rewrite test to use new public TopologyTestDriver
[ https://issues.apache.org/jira/browse/KAFKA-6474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16417922#comment-16417922 ] John Roesler commented on KAFKA-6474: - Hey [~h314to], I hope you're doing well. FYI, we have just merged [https://github.com/apache/kafka/pull/4760] incorporating your pattern for hooking test-utils into the Streams test dependencies. You may have conflicts to resolve when you rebase on trunk. Thanks, -John > Rewrite test to use new public TopologyTestDriver > - > > Key: KAFKA-6474 > URL: https://issues.apache.org/jira/browse/KAFKA-6474 > Project: Kafka > Issue Type: Improvement > Components: streams, unit tests >Affects Versions: 1.1.0 >Reporter: Matthias J. Sax >Assignee: Filipe Agapito >Priority: Major > Labels: beginner, newbie > > With KIP-247 we added public TopologyTestDriver. We should rewrite out own > test to use this new test driver and remove the two classes > ProcessorTopoogyTestDriver and KStreamTestDriver. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (KAFKA-6474) Rewrite test to use new public TopologyTestDriver
[ https://issues.apache.org/jira/browse/KAFKA-6474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16411288#comment-16411288 ] Filipe Agapito commented on KAFKA-6474: --- Hi John. Great, I'm glad I could help. I haven't bumped into the dependencies error yet, but I certainly will. Thanks for the heads up! > Rewrite test to use new public TopologyTestDriver > - > > Key: KAFKA-6474 > URL: https://issues.apache.org/jira/browse/KAFKA-6474 > Project: Kafka > Issue Type: Improvement > Components: streams, unit tests >Affects Versions: 1.1.0 >Reporter: Matthias J. Sax >Assignee: Filipe Agapito >Priority: Major > Labels: beginner, newbie > > With KIP-247 we added public TopologyTestDriver. We should rewrite out own > test to use this new test driver and remove the two classes > ProcessorTopoogyTestDriver and KStreamTestDriver. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (KAFKA-6474) Rewrite test to use new public TopologyTestDriver
[ https://issues.apache.org/jira/browse/KAFKA-6474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16410621#comment-16410621 ] Guozhang Wang commented on KAFKA-6474: -- Filipe would like you to take a look at https://github.com/apache/kafka/pull/4760. > Rewrite test to use new public TopologyTestDriver > - > > Key: KAFKA-6474 > URL: https://issues.apache.org/jira/browse/KAFKA-6474 > Project: Kafka > Issue Type: Improvement > Components: streams, unit tests >Affects Versions: 1.1.0 >Reporter: Matthias J. Sax >Assignee: Filipe Agapito >Priority: Major > Labels: beginner, newbie > > With KIP-247 we added public TopologyTestDriver. We should rewrite out own > test to use this new test driver and remove the two classes > ProcessorTopoogyTestDriver and KStreamTestDriver. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (KAFKA-6474) Rewrite test to use new public TopologyTestDriver
[
https://issues.apache.org/jira/browse/KAFKA-6474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16410162#comment-16410162
]
John Roesler commented on KAFKA-6474:
-
Yeah, thanks [~h314to]! I had tried it with just half of that change, and it
obviously didn't work. I'm really glad you figured that out!
FYI, there are other sub-projects in kafka that are going to follow this
pattern, so I'm going to start a dev mailing list discussion to share your
pattern.
Also, for completeness, I had to augment your build.gradle patch slightly to
get './gradlew streams:test-utils:test' to pass:
{noformat}
diff --git a/build.gradle b/build.gradle
index 5e4c35643..2f38bf28b 100644
--- a/build.gradle
+++ b/build.gradle
@@ -921,6 +921,7 @@ project(':streams') {
testCompile project(':clients').sourceSets.test.output
testCompile project(':core')
testCompile project(':core').sourceSets.test.output
+ testCompile project(':streams:test-utils').sourceSets.main.output
testCompile libs.junit
testCompile libs.easymock
testCompile libs.bcpkix
@@ -965,11 +966,12 @@ project(':streams:test-utils') {
archivesBaseName = "kafka-streams-test-utils"
dependencies {
- compile project(':streams')
+ compile project(':streams').sourceSets.main.output
compile project(':clients')
testCompile project(':clients').sourceSets.test.output
testCompile libs.junit
+ testCompile libs.rocksDBJni
testRuntime libs.slf4jlog4j
}{noformat}
The reason is that we are skipping :streams:copyDependantLibs during
test-utils:compile now (to avoid the circular dependency), so we have to
explictly depend in testCompile on any libs that would have been transitively
pulled in from :streams (and are in the test's run-time code path).
In case you run into a similar error when you run the full test suite, the
error I saw was:
{noformat}
java.lang.ClassNotFoundException: org.rocksdb.RocksDBException{noformat}
> Rewrite test to use new public TopologyTestDriver
> -
>
> Key: KAFKA-6474
> URL: https://issues.apache.org/jira/browse/KAFKA-6474
> Project: Kafka
> Issue Type: Improvement
> Components: streams, unit tests
>Affects Versions: 1.1.0
>Reporter: Matthias J. Sax
>Assignee: Filipe Agapito
>Priority: Major
> Labels: beginner, newbie
>
> With KIP-247 we added public TopologyTestDriver. We should rewrite out own
> test to use this new test driver and remove the two classes
> ProcessorTopoogyTestDriver and KStreamTestDriver.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (KAFKA-6474) Rewrite test to use new public TopologyTestDriver
[
https://issues.apache.org/jira/browse/KAFKA-6474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16402326#comment-16402326
]
Filipe Agapito commented on KAFKA-6474:
---
Hi John,
I also ran into the cyclic dependency issue. For instance, running:
{code:java}
./gradlew streams:test --tests
org.apache.kafka.streams.processor.TopologyBuilderTest
{code}
resulted in:
{code:java}
Task :streams:compileTestJava FAILED
warning: [options] bootstrap class path not set in conjunction with -source 1.7
/Users/agapito/Development/apache/kafka/streams/src/test/java/org/apache/kafka/streams/kstream/internals/GlobalKTableJoinsTest.java:24:
error: cannot find symbol
import org.apache.kafka.streams.TopologyTestDriver;
{code}
I added :streams:test-utils as a dependency to :streams in build gradle, which
then led to:
{code:java}
FAILURE: Build failed with an exception.
* What went wrong:
Circular dependency between the following tasks:
:streams:copyDependantLibs
+--- :streams:jar
|\--- :streams:copyDependantLibs (*)
\--- :streams:test-utils:jar
+--- :streams:test-utils:classes
|\--- :streams:test-utils:compileJava
| \--- :streams:jar (*)
\--- :streams:test-utils:copyDependantLibs
\--- :streams:jar (*)
(*) - details omitted (listed previously)
{code}
I got rid of it by adding the following changes to build.gradle:
{code:java}
diff --git a/build.gradle b/build.gradle
index 5e4c35643..17b49258d 100644
--- a/build.gradle
+++ b/build.gradle
@@ -921,6 +921,7 @@ project(':streams') {
testCompile project(':clients').sourceSets.test.output
testCompile project(':core')
testCompile project(':core').sourceSets.test.output
+testCompile project(':streams:test-utils').sourceSets.main.output
testCompile libs.junit
testCompile libs.easymock
testCompile libs.bcpkix
@@ -965,7 +966,7 @@ project(':streams:test-utils') {
archivesBaseName = "kafka-streams-test-utils"
dependencies {
-compile project(':streams')
+compile project(':streams').sourceSets.main.output
compile project(':clients')
testCompile project(':clients').sourceSets.test.output
{code}
This way both the tests and the jar build run cleanly. I'm not sure if this is
the best solution, but this issue entails a large refactor (not too complicated
but there are a lot of test classes to change) so it at least allows me to
continue refactoring and testing my changes without worrying about the cyclic
dependency. I only figured it out today. Previously I was just running the
tests in Intellij, where I added test-utils_main as a dependency of streams ->
streams_test module, so it was all working fine there. I hope this helps.
> Rewrite test to use new public TopologyTestDriver
> -
>
> Key: KAFKA-6474
> URL: https://issues.apache.org/jira/browse/KAFKA-6474
> Project: Kafka
> Issue Type: Improvement
> Components: streams, unit tests
>Affects Versions: 1.1.0
>Reporter: Matthias J. Sax
>Assignee: Filipe Agapito
>Priority: Major
> Labels: beginner, newbie
>
> With KIP-247 we added public TopologyTestDriver. We should rewrite out own
> test to use this new test driver and remove the two classes
> ProcessorTopoogyTestDriver and KStreamTestDriver.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (KAFKA-6474) Rewrite test to use new public TopologyTestDriver
[ https://issues.apache.org/jira/browse/KAFKA-6474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16400912#comment-16400912 ] John Roesler commented on KAFKA-6474: - Hi Filipe, I hope you're doing well. I'm trying to figure out why this is a problem for me but not for you... can you share the exact command you're using to run the tests? Thanks, -John > Rewrite test to use new public TopologyTestDriver > - > > Key: KAFKA-6474 > URL: https://issues.apache.org/jira/browse/KAFKA-6474 > Project: Kafka > Issue Type: Improvement > Components: streams, unit tests >Affects Versions: 1.1.0 >Reporter: Matthias J. Sax >Assignee: Filipe Agapito >Priority: Major > Labels: beginner, newbie > > With KIP-247 we added public TopologyTestDriver. We should rewrite out own > test to use this new test driver and remove the two classes > ProcessorTopoogyTestDriver and KStreamTestDriver. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (KAFKA-6474) Rewrite test to use new public TopologyTestDriver
[ https://issues.apache.org/jira/browse/KAFKA-6474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16395070#comment-16395070 ] Filipe Agapito commented on KAFKA-6474: --- Hi John, So far I've done most of the refactor and I haven't had any complaints from gradle (just did a clean compile). Now I'm refactoring the last class, KStreamTestDriver. Probably the circular dependency issue will show up now. It's nice to know it is a known issue. Thanks for the heads up! > Rewrite test to use new public TopologyTestDriver > - > > Key: KAFKA-6474 > URL: https://issues.apache.org/jira/browse/KAFKA-6474 > Project: Kafka > Issue Type: Improvement > Components: streams, unit tests >Affects Versions: 1.1.0 >Reporter: Matthias J. Sax >Assignee: Filipe Agapito >Priority: Major > Labels: beginner, newbie > > With KIP-247 we added public TopologyTestDriver. We should rewrite out own > test to use this new test driver and remove the two classes > ProcessorTopoogyTestDriver and KStreamTestDriver. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (KAFKA-6474) Rewrite test to use new public TopologyTestDriver
[
https://issues.apache.org/jira/browse/KAFKA-6474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16393444#comment-16393444
]
John Roesler commented on KAFKA-6474:
-
Also worth mentioning is [~guozhang]'s reply:
{quote}I think I agree with your proposed changes, in fact in order to not
scatter
the test classes in two places maybe it's better to move all of them to the
new module. One caveat is that it will make streams' project hierarchy
inconsistent with other projects where the unit test classes are maintained
inside the main artifact package, but I think it is a good cost to pay,
plus once we start publishing test-util artifacts for other projects like
client and connect, we may face the same issue and need to do this
refactoring as well.
{quote}
> Rewrite test to use new public TopologyTestDriver
> -
>
> Key: KAFKA-6474
> URL: https://issues.apache.org/jira/browse/KAFKA-6474
> Project: Kafka
> Issue Type: Improvement
> Components: streams, unit tests
>Affects Versions: 1.1.0
>Reporter: Matthias J. Sax
>Assignee: Filipe Agapito
>Priority: Major
> Labels: beginner, newbie
>
> With KIP-247 we added public TopologyTestDriver. We should rewrite out own
> test to use this new test driver and remove the two classes
> ProcessorTopoogyTestDriver and KStreamTestDriver.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (KAFKA-6474) Rewrite test to use new public TopologyTestDriver
[
https://issues.apache.org/jira/browse/KAFKA-6474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16393415#comment-16393415
]
John Roesler commented on KAFKA-6474:
-
Hi Filipe,
I have been working on a similar task (KAFKA-6473) and discovered something
that will probably be a problem for you.
This task requires the streams project to have a test dependency on the
stream:test-utils project, but streams:test-utils already has a compile
dependency on the streams project. I'm no Gradle expert, but as far as I can
tell, there's no way to break this circular dependency, at least without doing
something exotic in the gradle config.
We had a discussion in this mailing list thread: [[DISCUSS]
KIP-267|[http://mail-archives.apache.org/mod_mbox/kafka-dev/201803.mbox/%3CCAAyirGsovAzRMLa91nd6rzceQgEcNcLMt7ZrXVN7M1Psj4jCmQ%40mail.gmail.com%3E]|http://mail-archives.apache.org/mod_mbox/kafka-dev/201803.mbox/%3CCAAyirGsovAzRMLa91nd6rzceQgEcNcLMt7ZrXVN7M1Psj4jCmQ%40mail.gmail.com%3E].]
. Here's what we settled on:
{quote}I would propose we restructure the streams directory thusly:
streams/ (artifact name := "streams", the actual streams code lives here)
- test-utils/ (this is the current test-utils artifact, depends on "streams")
- tests/ (new module, depends on "streams" and "test-utils", *NO published
artifact*)
This gets us out of the circular dependency without having to engage in any
Gradle shenanigans while preserving "test-utils" as a separate artifact. This
is good because: 1) the test-utils don't need to be in production code, so it's
nice to have a separate artifact, 2) test-utils is already public in 1.1, and
it's a bummer to introduce users' code when we can so easily avoid it.{quote}
Another result of the discussion is that I'm actually going to side-step this
issue for KAFKA-6473, so I won't be doing any restructuring in the course of my
work. I'm just sharing these ideas with you for your context.
Hope you're well!
-John
> Rewrite test to use new public TopologyTestDriver
> -
>
> Key: KAFKA-6474
> URL: https://issues.apache.org/jira/browse/KAFKA-6474
> Project: Kafka
> Issue Type: Improvement
> Components: streams, unit tests
>Affects Versions: 1.1.0
>Reporter: Matthias J. Sax
>Assignee: Filipe Agapito
>Priority: Major
> Labels: beginner, newbie
>
> With KIP-247 we added public TopologyTestDriver. We should rewrite out own
> test to use this new test driver and remove the two classes
> ProcessorTopoogyTestDriver and KStreamTestDriver.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (KAFKA-6474) Rewrite test to use new public TopologyTestDriver
[ https://issues.apache.org/jira/browse/KAFKA-6474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16388735#comment-16388735 ] Filipe Agapito commented on KAFKA-6474: --- Hi. Yes, I'm still working on in. Got a few busy days so I couldn't finish it as soon as I'd like, but I won't take much longer. > Rewrite test to use new public TopologyTestDriver > - > > Key: KAFKA-6474 > URL: https://issues.apache.org/jira/browse/KAFKA-6474 > Project: Kafka > Issue Type: Improvement > Components: streams, unit tests >Affects Versions: 1.1.0 >Reporter: Matthias J. Sax >Assignee: Filipe Agapito >Priority: Major > Labels: beginner, newbie > > With KIP-247 we added public TopologyTestDriver. We should rewrite out own > test to use this new test driver and remove the two classes > ProcessorTopoogyTestDriver and KStreamTestDriver. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (KAFKA-6474) Rewrite test to use new public TopologyTestDriver
[ https://issues.apache.org/jira/browse/KAFKA-6474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16388216#comment-16388216 ] Guozhang Wang commented on KAFKA-6474: -- Hi Filipe, Are you still working on this? > Rewrite test to use new public TopologyTestDriver > - > > Key: KAFKA-6474 > URL: https://issues.apache.org/jira/browse/KAFKA-6474 > Project: Kafka > Issue Type: Improvement > Components: streams, unit tests >Affects Versions: 1.1.0 >Reporter: Matthias J. Sax >Assignee: Filipe Agapito >Priority: Major > Labels: beginner, newbie > > With KIP-247 we added public TopologyTestDriver. We should rewrite out own > test to use this new test driver and remove the two classes > ProcessorTopoogyTestDriver and KStreamTestDriver. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (KAFKA-6474) Rewrite test to use new public TopologyTestDriver
[ https://issues.apache.org/jira/browse/KAFKA-6474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16382069#comment-16382069 ] Filipe Agapito commented on KAFKA-6474: --- I'd like to work on this. I'm assigning the issue to myself. > Rewrite test to use new public TopologyTestDriver > - > > Key: KAFKA-6474 > URL: https://issues.apache.org/jira/browse/KAFKA-6474 > Project: Kafka > Issue Type: Improvement > Components: streams, unit tests >Affects Versions: 1.1.0 >Reporter: Matthias J. Sax >Priority: Major > Labels: beginner, newbie > > With KIP-247 we added public TopologyTestDriver. We should rewrite out own > test to use this new test driver and remove the two classes > ProcessorTopoogyTestDriver and KStreamTestDriver. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
