You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "m1a2st (via GitHub)" <gi...@apache.org> on 2024/06/15 15:39:41 UTC
[PR] KAFKA-16921: Migrate test of connect module to Junit5 (Stream state) [kafka]
m1a2st opened a new pull request, #16356:
URL: https://github.com/apache/kafka/pull/16356
Migrate test of stream module to Junit5
Committer Checklist (excluded from commit message)
Verify design and implementation
Verify test coverage and CI build status
Verify documentation (including upgrade notes)
--
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-16921: Migrate test of Stream module to Junit5 (Stream state) [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #16356:
URL: https://github.com/apache/kafka/pull/16356#issuecomment-2175742237
@m1a2st Could you please update the jira number? It should be KAFKA-15623, right?
--
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-16921: Migrate test of Stream module to Junit5 (Stream state) [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on code in PR #16356:
URL: https://github.com/apache/kafka/pull/16356#discussion_r1643586369
##########
streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBVersionedStoreSegmentValueFormatterTest.java:
##########
@@ -18,61 +18,49 @@
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.Assert.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import java.util.ArrayList;
import java.util.Arrays;
-import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.stream.Stream;
+
import org.apache.kafka.streams.state.internals.RocksDBVersionedStoreSegmentValueFormatter.SegmentValue;
import org.apache.kafka.streams.state.internals.RocksDBVersionedStoreSegmentValueFormatter.SegmentValue.SegmentSearchResult;
-import org.junit.Test;
-import org.junit.experimental.runners.Enclosed;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
-@RunWith(Enclosed.class)
public class RocksDBVersionedStoreSegmentValueFormatterTest {
/**
* Non-exceptional scenarios which are expected to occur during regular store operation.
*/
- @RunWith(Parameterized.class)
public static class ExpectedCasesTest {
Review Comment:
@chia7712, Thanks for your comment, I don't really understand that I should extract which test case? Could you talk more details, Thanks for your patience.
--
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-16921: Migrate test of Stream module to Junit5 (Stream state) [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on PR #16356:
URL: https://github.com/apache/kafka/pull/16356#issuecomment-2171061086
@C0urante, Thanks for your comment, It's my mistake, I change title to stream 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-15623, Migrate test of Stream module to Junit5 (Stream state) [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on code in PR #16356:
URL: https://github.com/apache/kafka/pull/16356#discussion_r1647508440
##########
streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractRocksDBSessionStoreTest.java:
##########
@@ -0,0 +1,33 @@
+/*
+ * 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.state.internals;
+
+public abstract class AbstractRocksDBSessionStoreTest extends AbstractSessionBytesStoreTest {
+
+ @Override
+ String getStoreName() {
+ return "rocksDB session store";
+ }
+
+ abstract StoreType storeType();
Review Comment:
@chia7712, Thanks for your comment, I think it' a good idea.
--
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-16921: Migrate test of connect module to Junit5 (Stream state) [kafka]
Posted by "C0urante (via GitHub)" <gi...@apache.org>.
C0urante commented on PR #16356:
URL: https://github.com/apache/kafka/pull/16356#issuecomment-2171043633
This doesn't touch on Connect, right? Might want to change the title 😅
--
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-15623, Migrate test of Stream module to Junit5 (Stream state) [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on PR #16356:
URL: https://github.com/apache/kafka/pull/16356#issuecomment-2175916646
@chia7712, Thanks for your comments, PTAL
--
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-16921: Migrate test of connect module to Junit5 (Stream state) [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #16356:
URL: https://github.com/apache/kafka/pull/16356#discussion_r1641586674
##########
streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBWindowStoreTest.java:
##########
@@ -18,42 +18,87 @@
import java.io.File;
import java.util.ArrayList;
-import java.util.Collection;
import java.util.Collections;
+import java.util.HashMap;
import java.util.List;
import java.util.HashSet;
+import java.util.Map;
+import java.util.Properties;
import java.util.Set;
import org.apache.kafka.clients.producer.ProducerRecord;
+import org.apache.kafka.common.Metric;
+import org.apache.kafka.common.MetricName;
import org.apache.kafka.common.header.internals.RecordHeaders;
+import org.apache.kafka.common.metrics.Metrics;
import org.apache.kafka.common.serialization.Serde;
import org.apache.kafka.common.serialization.Serdes;
import org.apache.kafka.common.utils.Bytes;
+import org.apache.kafka.common.utils.LogCaptureAppender;
+import org.apache.kafka.common.utils.LogContext;
+import org.apache.kafka.common.utils.Time;
import org.apache.kafka.common.utils.Utils;
import org.apache.kafka.streams.KeyValue;
+import org.apache.kafka.streams.StreamsConfig;
+import org.apache.kafka.streams.kstream.Windowed;
import org.apache.kafka.streams.processor.StateStoreContext;
+import org.apache.kafka.streams.processor.internals.MockStreamsMetrics;
import org.apache.kafka.streams.query.Position;
import org.apache.kafka.streams.processor.internals.ProcessorRecordContext;
+import org.apache.kafka.streams.state.KeyValueIterator;
+import org.apache.kafka.streams.state.StateSerdes;
import org.apache.kafka.streams.state.Stores;
import org.apache.kafka.streams.state.WindowStore;
import org.apache.kafka.streams.state.WindowStoreIterator;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
-import org.junit.runners.Parameterized.Parameter;
+import org.apache.kafka.test.InternalMockProcessorContext;
+import org.apache.kafka.test.MockRecordCollector;
+import org.apache.kafka.test.StreamsTestUtils;
+import org.apache.kafka.test.TestUtils;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.EnumSource;
import static java.time.Duration.ofMillis;
import static java.time.Instant.ofEpochMilli;
import static java.util.Arrays.asList;
import static java.util.Objects.requireNonNull;
import static org.apache.kafka.common.utils.Utils.mkEntry;
import static org.apache.kafka.common.utils.Utils.mkMap;
+import static org.apache.kafka.test.StreamsTestUtils.toList;
+import static org.apache.kafka.test.StreamsTestUtils.toSet;
import static org.apache.kafka.test.StreamsTestUtils.valuesToSet;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.CoreMatchers.hasItem;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+
+public class RocksDBWindowStoreTest {
Review Comment:
Could you please rewrite the parameter by making it be abstract and then add related sub classes? for example:
[RocksDBWindowStoreTest.txt](https://github.com/user-attachments/files/15855803/RocksDBWindowStoreTest.txt)
--
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-15623, Migrate test of Stream module to Junit5 (Stream state) [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on PR #16356:
URL: https://github.com/apache/kafka/pull/16356#issuecomment-2180588283
@chia7712, Thanks for your comment, PTAL
--
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-16921: Migrate test of Stream module to Junit5 (Stream state) [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #16356:
URL: https://github.com/apache/kafka/pull/16356#discussion_r1644229011
##########
streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBVersionedStoreSegmentValueFormatterTest.java:
##########
@@ -18,61 +18,49 @@
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.Assert.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import java.util.ArrayList;
import java.util.Arrays;
-import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.stream.Stream;
+
import org.apache.kafka.streams.state.internals.RocksDBVersionedStoreSegmentValueFormatter.SegmentValue;
import org.apache.kafka.streams.state.internals.RocksDBVersionedStoreSegmentValueFormatter.SegmentValue.SegmentSearchResult;
-import org.junit.Test;
-import org.junit.experimental.runners.Enclosed;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
-@RunWith(Enclosed.class)
public class RocksDBVersionedStoreSegmentValueFormatterTest {
/**
* Non-exceptional scenarios which are expected to occur during regular store operation.
*/
- @RunWith(Parameterized.class)
public static class ExpectedCasesTest {
Review Comment:
I means - please do a bit refactor to avoid nested 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-16921: Migrate test of Stream module to Junit5 (Stream state) [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #16356:
URL: https://github.com/apache/kafka/pull/16356#discussion_r1642920606
##########
streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractRocksDBSessionStoreTest.java:
##########
@@ -0,0 +1,75 @@
+/*
+ * 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.state.internals;
+
+import org.apache.kafka.common.serialization.Serde;
+import org.apache.kafka.streams.state.SessionStore;
+import org.apache.kafka.streams.state.Stores;
+
+import static java.time.Duration.ofMillis;
+
+public abstract class AbstractRocksDBSessionStoreTest extends AbstractSessionBytesStoreTest {
+
+ private static final String STORE_NAME = "rocksDB session store";
+
+ public abstract StoreType storeType();
Review Comment:
please remove `public`
##########
streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBTimeOrderedSessionStoreWithIndexTest.java:
##########
@@ -0,0 +1,25 @@
+/*
+ * 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.state.internals;
+
+public class RocksDBTimeOrderedSessionStoreWithIndexTest extends AbstractRocksDBSessionStoreTest {
+
+ @Override
+ public StoreType storeType() {
Review Comment:
please remove `public`
##########
streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBTimeOrderedWindowSchemaWithIndexSegmentedBytesStoreTest.java:
##########
@@ -0,0 +1,30 @@
+/*
+ * 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.state.internals;
+
+public class RocksDBTimeOrderedWindowSchemaWithIndexSegmentedBytesStoreTest extends AbstractRocksDBTimeOrderedWindowSegmentedBytesStoreTest {
+
+ @Override
+ protected boolean hasIndex() {
+ return true;
+ }
+
+ @Override
+ protected AbstractRocksDBTimeOrderedWindowSegmentedBytesStoreTest.SchemaType schemaType() {
Review Comment:
please remove "protected"
##########
streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractRocksDBTimeOrderedWindowSegmentedBytesStoreTest.java:
##########
@@ -16,59 +16,37 @@
*/
package org.apache.kafka.streams.state.internals;
-import static java.util.Arrays.asList;
-
-import java.util.Collection;
import org.apache.kafka.streams.state.internals.PrefixedSessionKeySchemas.KeyFirstSessionKeySchema;
import org.apache.kafka.streams.state.internals.PrefixedSessionKeySchemas.TimeFirstSessionKeySchema;
import org.apache.kafka.streams.state.internals.PrefixedWindowKeySchemas.KeyFirstWindowKeySchema;
import org.apache.kafka.streams.state.internals.PrefixedWindowKeySchemas.TimeFirstWindowKeySchema;
import org.apache.kafka.streams.state.internals.SegmentedBytesStore.KeySchema;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
-@RunWith(Parameterized.class)
-public class RocksDBTimeOrderedWindowSegmentedBytesStoreTest
+public abstract class AbstractRocksDBTimeOrderedWindowSegmentedBytesStoreTest
extends AbstractDualSchemaRocksDBSegmentedBytesStoreTest<KeyValueSegment> {
private static final String METRICS_SCOPE = "metrics-scope";
- private enum SchemaType {
+ protected enum SchemaType {
Review Comment:
please use package-private
##########
streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBTimeOrderedSessionSchemaWithoutIndexSegmentedBytesStoreTest.java:
##########
@@ -0,0 +1,30 @@
+/*
+ * 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.state.internals;
+
+public class RocksDBTimeOrderedSessionSchemaWithoutIndexSegmentedBytesStoreTest extends AbstractRocksDBTimeOrderedWindowSegmentedBytesStoreTest {
+
+ @Override
+ protected boolean hasIndex() {
Review Comment:
please remove "protected"
##########
streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBSessionStoreTest.java:
##########
@@ -16,78 +16,10 @@
*/
package org.apache.kafka.streams.state.internals;
-import java.util.Collection;
-import org.apache.kafka.common.serialization.Serde;
-import org.apache.kafka.streams.state.SessionStore;
-import org.apache.kafka.streams.state.Stores;
-
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
-import org.junit.runners.Parameterized.Parameter;
-
-import static java.time.Duration.ofMillis;
-import static java.util.Arrays.asList;
-
-@RunWith(Parameterized.class)
-public class RocksDBSessionStoreTest extends AbstractSessionBytesStoreTest {
-
- private static final String STORE_NAME = "rocksDB session store";
-
- @Parameter
- public StoreType storeType;
-
- @Parameterized.Parameters(name = "{0}")
- public static Collection<Object[]> getParamStoreType() {
- return asList(new Object[][] {
- {StoreType.RocksDBSessionStore},
- {StoreType.RocksDBTimeOrderedSessionStoreWithIndex},
- {StoreType.RocksDBTimeOrderedSessionStoreWithoutIndex}
- });
- }
+public class RocksDBSessionStoreTest extends AbstractRocksDBSessionStoreTest {
@Override
- StoreType getStoreType() {
- return storeType;
+ public StoreType storeType() {
Review Comment:
please remove `public`
##########
streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBTimeOrderedSessionStoreWithoutIndexTest.java:
##########
@@ -0,0 +1,25 @@
+/*
+ * 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.state.internals;
+
+public class RocksDBTimeOrderedSessionStoreWithoutIndexTest extends AbstractRocksDBSessionStoreTest {
+
+ @Override
+ public StoreType storeType() {
Review Comment:
please remove `public`
##########
streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractRocksDBTimeOrderedWindowSegmentedBytesStoreTest.java:
##########
@@ -16,59 +16,37 @@
*/
package org.apache.kafka.streams.state.internals;
-import static java.util.Arrays.asList;
-
-import java.util.Collection;
import org.apache.kafka.streams.state.internals.PrefixedSessionKeySchemas.KeyFirstSessionKeySchema;
import org.apache.kafka.streams.state.internals.PrefixedSessionKeySchemas.TimeFirstSessionKeySchema;
import org.apache.kafka.streams.state.internals.PrefixedWindowKeySchemas.KeyFirstWindowKeySchema;
import org.apache.kafka.streams.state.internals.PrefixedWindowKeySchemas.TimeFirstWindowKeySchema;
import org.apache.kafka.streams.state.internals.SegmentedBytesStore.KeySchema;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
-@RunWith(Parameterized.class)
-public class RocksDBTimeOrderedWindowSegmentedBytesStoreTest
+public abstract class AbstractRocksDBTimeOrderedWindowSegmentedBytesStoreTest
extends AbstractDualSchemaRocksDBSegmentedBytesStoreTest<KeyValueSegment> {
private static final String METRICS_SCOPE = "metrics-scope";
- private enum SchemaType {
+ protected enum SchemaType {
WindowSchemaWithIndex,
WindowSchemaWithoutIndex,
SessionSchemaWithIndex,
SessionSchemaWithoutIndex
}
- private boolean hasIndex;
- private SchemaType schemaType;
-
- @Parameterized.Parameters(name = "{0}")
- public static Collection<Object[]> getKeySchema() {
- return asList(new Object[][] {
- {SchemaType.WindowSchemaWithIndex, true},
- {SchemaType.WindowSchemaWithoutIndex, false},
- {SchemaType.SessionSchemaWithIndex, true},
- {SchemaType.SessionSchemaWithoutIndex, false}
- });
- }
-
- public RocksDBTimeOrderedWindowSegmentedBytesStoreTest(final SchemaType schemaType, final boolean hasIndex) {
- this.schemaType = schemaType;
- this.hasIndex = hasIndex;
- }
-
+ protected abstract boolean hasIndex();
+ protected abstract SchemaType schemaType();
Review Comment:
please use package-private
##########
streams/src/test/java/org/apache/kafka/streams/state/internals/KeyValueStoreBuilderTest.java:
##########
@@ -47,27 +49,32 @@ public class KeyValueStoreBuilderTest {
private KeyValueStore<Bytes, byte[]> inner;
private KeyValueStoreBuilder<String, String> builder;
- @Before
- public void setUp() {
- when(supplier.get()).thenReturn(inner);
+ public void setUpWithoutInner() {
when(supplier.name()).thenReturn("name");
when(supplier.metricsScope()).thenReturn("metricScope");
builder = new KeyValueStoreBuilder<>(
- supplier,
- Serdes.String(),
- Serdes.String(),
- new MockTime()
+ supplier,
Review Comment:
please revert those unrelated changes
##########
streams/src/test/java/org/apache/kafka/streams/state/internals/MeteredTimestampedKeyValueStoreTest.java:
##########
@@ -106,35 +108,37 @@ public class MeteredTimestampedKeyValueStoreTest {
private final Metrics metrics = new Metrics();
private Map<String, String> tags;
- @Before
- public void before() {
+ private void setUpWithoutContext() {
mockTime = new MockTime();
metered = new MeteredTimestampedKeyValueStore<>(
- inner,
- "scope",
- mockTime,
- Serdes.String(),
- new ValueAndTimestampSerde<>(Serdes.String())
+ inner,
Review Comment:
please revert those changes
##########
streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBTimeOrderedSessionSchemaWithoutIndexSegmentedBytesStoreTest.java:
##########
@@ -0,0 +1,30 @@
+/*
+ * 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.state.internals;
+
+public class RocksDBTimeOrderedSessionSchemaWithoutIndexSegmentedBytesStoreTest extends AbstractRocksDBTimeOrderedWindowSegmentedBytesStoreTest {
+
+ @Override
+ protected boolean hasIndex() {
+ return false;
+ }
+
+ @Override
+ protected SchemaType schemaType() {
Review Comment:
please remove "protected"
##########
streams/src/test/java/org/apache/kafka/streams/state/internals/AbstractRocksDBTimeOrderedWindowSegmentedBytesStoreTest.java:
##########
@@ -16,59 +16,37 @@
*/
package org.apache.kafka.streams.state.internals;
-import static java.util.Arrays.asList;
-
-import java.util.Collection;
import org.apache.kafka.streams.state.internals.PrefixedSessionKeySchemas.KeyFirstSessionKeySchema;
import org.apache.kafka.streams.state.internals.PrefixedSessionKeySchemas.TimeFirstSessionKeySchema;
import org.apache.kafka.streams.state.internals.PrefixedWindowKeySchemas.KeyFirstWindowKeySchema;
import org.apache.kafka.streams.state.internals.PrefixedWindowKeySchemas.TimeFirstWindowKeySchema;
import org.apache.kafka.streams.state.internals.SegmentedBytesStore.KeySchema;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
-@RunWith(Parameterized.class)
-public class RocksDBTimeOrderedWindowSegmentedBytesStoreTest
+public abstract class AbstractRocksDBTimeOrderedWindowSegmentedBytesStoreTest
extends AbstractDualSchemaRocksDBSegmentedBytesStoreTest<KeyValueSegment> {
private static final String METRICS_SCOPE = "metrics-scope";
- private enum SchemaType {
+ protected enum SchemaType {
WindowSchemaWithIndex,
WindowSchemaWithoutIndex,
SessionSchemaWithIndex,
SessionSchemaWithoutIndex
}
- private boolean hasIndex;
- private SchemaType schemaType;
-
- @Parameterized.Parameters(name = "{0}")
- public static Collection<Object[]> getKeySchema() {
- return asList(new Object[][] {
- {SchemaType.WindowSchemaWithIndex, true},
- {SchemaType.WindowSchemaWithoutIndex, false},
- {SchemaType.SessionSchemaWithIndex, true},
- {SchemaType.SessionSchemaWithoutIndex, false}
- });
- }
-
- public RocksDBTimeOrderedWindowSegmentedBytesStoreTest(final SchemaType schemaType, final boolean hasIndex) {
- this.schemaType = schemaType;
- this.hasIndex = hasIndex;
- }
-
+ protected abstract boolean hasIndex();
Review Comment:
please use package-private
##########
streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBTimeOrderedWindowSchemaWithoutIndexSegmentedBytesStoreTest.java:
##########
@@ -0,0 +1,30 @@
+/*
+ * 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.state.internals;
+
+public class RocksDBTimeOrderedWindowSchemaWithoutIndexSegmentedBytesStoreTest extends AbstractRocksDBTimeOrderedWindowSegmentedBytesStoreTest {
+
+ @Override
+ protected boolean hasIndex() {
+ return false;
+ }
+
+ @Override
+ protected SchemaType schemaType() {
Review Comment:
please remove "protected"
##########
streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBTimeOrderedWindowSchemaWithIndexSegmentedBytesStoreTest.java:
##########
@@ -0,0 +1,30 @@
+/*
+ * 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.state.internals;
+
+public class RocksDBTimeOrderedWindowSchemaWithIndexSegmentedBytesStoreTest extends AbstractRocksDBTimeOrderedWindowSegmentedBytesStoreTest {
+
+ @Override
+ protected boolean hasIndex() {
Review Comment:
please remove "protected"
##########
streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBVersionedStoreSegmentValueFormatterTest.java:
##########
@@ -18,61 +18,49 @@
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.Assert.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import java.util.ArrayList;
import java.util.Arrays;
-import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.stream.Stream;
+
import org.apache.kafka.streams.state.internals.RocksDBVersionedStoreSegmentValueFormatter.SegmentValue;
import org.apache.kafka.streams.state.internals.RocksDBVersionedStoreSegmentValueFormatter.SegmentValue.SegmentSearchResult;
-import org.junit.Test;
-import org.junit.experimental.runners.Enclosed;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
-@RunWith(Enclosed.class)
public class RocksDBVersionedStoreSegmentValueFormatterTest {
/**
* Non-exceptional scenarios which are expected to occur during regular store operation.
*/
- @RunWith(Parameterized.class)
public static class ExpectedCasesTest {
Review Comment:
Could you please extract all test cases?
##########
streams/src/test/java/org/apache/kafka/streams/state/internals/TimestampedKeyValueStoreBuilderTest.java:
##########
@@ -46,28 +48,33 @@ public class TimestampedKeyValueStoreBuilderTest {
private RocksDBTimestampedStore inner;
private TimestampedKeyValueStoreBuilder<String, String> builder;
- @Before
- public void setUp() {
- when(supplier.get()).thenReturn(inner);
+ private void setUpWithoutInner() {
when(supplier.name()).thenReturn("name");
when(supplier.metricsScope()).thenReturn("metricScope");
builder = new TimestampedKeyValueStoreBuilder<>(
- supplier,
- Serdes.String(),
- Serdes.String(),
- new MockTime()
+ supplier,
Review Comment:
please revert those unrelated changes
##########
streams/src/test/java/org/apache/kafka/streams/state/internals/VersionedKeyValueStoreBuilderTest.java:
##########
@@ -48,29 +50,34 @@ public class VersionedKeyValueStoreBuilderTest {
private VersionedKeyValueStoreBuilder<String, String> builder;
- @Before
- public void setUp() {
- when(supplier.get()).thenReturn(inner);
+ private void setUpWithoutInner() {
when(supplier.name()).thenReturn(STORE_NAME);
when(supplier.metricsScope()).thenReturn(METRICS_SCOPE);
builder = new VersionedKeyValueStoreBuilder<>(
- supplier,
- Serdes.String(),
- Serdes.String(),
- new MockTime()
+ supplier,
Review Comment:
please revert those unrelated changes
##########
streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBTimeOrderedWindowSchemaWithoutIndexSegmentedBytesStoreTest.java:
##########
@@ -0,0 +1,30 @@
+/*
+ * 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.state.internals;
+
+public class RocksDBTimeOrderedWindowSchemaWithoutIndexSegmentedBytesStoreTest extends AbstractRocksDBTimeOrderedWindowSegmentedBytesStoreTest {
+
+ @Override
+ protected boolean hasIndex() {
Review Comment:
please remove "protected"
##########
streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBVersionedStoreSegmentValueFormatterTest.java:
##########
@@ -280,38 +276,27 @@ public void shouldCreateNewWithRecord() {
* inconsistency among segments. These tests verify that the store is able to recover
* gracefully even if this happens.
*/
- @RunWith(Parameterized.class)
public static class ExceptionalCasesTest {
Review Comment:
Could you please extract all test cases?
--
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