Re: [PR] KAFKA-15623: Migrate streams tests (part 1) module to JUnit 5 [kafka]

2024-06-13 Thread via GitHub 


chia7712 merged PR #16254:
URL: https://github.com/apache/kafka/pull/16254


-- 
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-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] KAFKA-15623: Migrate streams tests (part 1) module to JUnit 5 [kafka]

2024-06-12 Thread via GitHub 


FrankYang0529 commented on PR #16254:
URL: https://github.com/apache/kafka/pull/16254#issuecomment-2164633695

   Hi @chia7712, I addressed all comments. May you take a look? Thank you.


-- 
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-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] KAFKA-15623: Migrate streams tests (part 1) module to JUnit 5 [kafka]

2024-06-12 Thread via GitHub 


FrankYang0529 commented on code in PR #16254:
URL: https://github.com/apache/kafka/pull/16254#discussion_r1637632275


##
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamSlidingWindowAggregateTest.java:
##
@@ -87,48 +85,45 @@
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.not;
 import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
-@RunWith(Parameterized.class)
 public class KStreamSlidingWindowAggregateTest {
-
-@Parameterized.Parameters(name = "{0}_inorder:{1}_cache:{2}")
-public static Collection data() {
-return Arrays.asList(new Object[][] {
-{StrategyType.ON_WINDOW_UPDATE, true, true},
-{StrategyType.ON_WINDOW_UPDATE, true, false},
-{StrategyType.ON_WINDOW_UPDATE, false, true},
-{StrategyType.ON_WINDOW_UPDATE, false, false},
-{StrategyType.ON_WINDOW_CLOSE, true, true},
-{StrategyType.ON_WINDOW_CLOSE, true, false},
-{StrategyType.ON_WINDOW_CLOSE, false, true},
-{StrategyType.ON_WINDOW_CLOSE, false, false}
-});
+
+public static Stream data() {
+return Stream.of(
+Arguments.of(StrategyType.ON_WINDOW_UPDATE, true, true),
+Arguments.of(StrategyType.ON_WINDOW_UPDATE, true, false),
+Arguments.of(StrategyType.ON_WINDOW_UPDATE, false, true),
+Arguments.of(StrategyType.ON_WINDOW_UPDATE, false, false),
+Arguments.of(StrategyType.ON_WINDOW_CLOSE, true, true), 
+Arguments.of(StrategyType.ON_WINDOW_CLOSE, true, false),
+Arguments.of(StrategyType.ON_WINDOW_CLOSE, false, true),
+Arguments.of(StrategyType.ON_WINDOW_CLOSE, false, false)
+);
 }
-@Parameter
 public StrategyType type;
-
-@Parameter(1)
 public boolean inOrderIterator;
-
-@Parameter(2)
 public boolean withCache;
 
 private boolean emitFinal;
 private EmitStrategy emitStrategy;
 
 private final Properties props = 
StreamsTestUtils.getStreamsConfig(Serdes.String(), Serdes.String());
 private final String threadId = Thread.currentThread().getName();
-
-@Before
-public void before() {
+
+public void setup(final StrategyType inputType, final boolean 
inputInOrderIterator, final boolean inputWithCache) {
+type = inputType;
+inOrderIterator = inputInOrderIterator;
+withCache = inputWithCache;
 emitFinal = type.equals(StrategyType.ON_WINDOW_CLOSE);
 emitStrategy = StrategyType.forType(type);
 }
 
-@Test
-public void testAggregateSmallInput() {
+@ParameterizedTest
+@MethodSource("data")

Review Comment:
   This one can't just use `@EnumSource(EmitStrategy.StrategyType.class)`, 
because there are another two arguments.



-- 
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-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] KAFKA-15623: Migrate streams tests (part 1) module to JUnit 5 [kafka]

2024-06-11 Thread via GitHub 


chia7712 commented on code in PR #16254:
URL: https://github.com/apache/kafka/pull/16254#discussion_r1635384831


##
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamFlatTransformTest.java:
##
@@ -41,21 +42,22 @@
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
-@RunWith(MockitoJUnitRunner.StrictStubs.class)
+@ExtendWith(MockitoExtension.class)
+@MockitoSettings(strictness = Strictness.STRICT_STUBS)
 public class KStreamFlatTransformTest {
 
 private Number inputKey;
 private Number inputValue;
 
-@Mock
-private Transformer>> 
transformer;
-@Mock
-private InternalProcessorContext context;
+@SuppressWarnings("unchecked")

Review Comment:
   why we need this chagne? `@Mock` should work well with junit 5



##
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamSlidingWindowAggregateTest.java:
##
@@ -87,48 +85,45 @@
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.not;
 import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
-@RunWith(Parameterized.class)
 public class KStreamSlidingWindowAggregateTest {
-
-@Parameterized.Parameters(name = "{0}_inorder:{1}_cache:{2}")
-public static Collection data() {
-return Arrays.asList(new Object[][] {
-{StrategyType.ON_WINDOW_UPDATE, true, true},
-{StrategyType.ON_WINDOW_UPDATE, true, false},
-{StrategyType.ON_WINDOW_UPDATE, false, true},
-{StrategyType.ON_WINDOW_UPDATE, false, false},
-{StrategyType.ON_WINDOW_CLOSE, true, true},
-{StrategyType.ON_WINDOW_CLOSE, true, false},
-{StrategyType.ON_WINDOW_CLOSE, false, true},
-{StrategyType.ON_WINDOW_CLOSE, false, false}
-});
+
+public static Stream data() {
+return Stream.of(
+Arguments.of(StrategyType.ON_WINDOW_UPDATE, true, true),
+Arguments.of(StrategyType.ON_WINDOW_UPDATE, true, false),
+Arguments.of(StrategyType.ON_WINDOW_UPDATE, false, true),
+Arguments.of(StrategyType.ON_WINDOW_UPDATE, false, false),
+Arguments.of(StrategyType.ON_WINDOW_CLOSE, true, true), 
+Arguments.of(StrategyType.ON_WINDOW_CLOSE, true, false),
+Arguments.of(StrategyType.ON_WINDOW_CLOSE, false, true),
+Arguments.of(StrategyType.ON_WINDOW_CLOSE, false, false)
+);
 }
-@Parameter
 public StrategyType type;
-
-@Parameter(1)
 public boolean inOrderIterator;
-
-@Parameter(2)
 public boolean withCache;
 
 private boolean emitFinal;
 private EmitStrategy emitStrategy;
 
 private final Properties props = 
StreamsTestUtils.getStreamsConfig(Serdes.String(), Serdes.String());
 private final String threadId = Thread.currentThread().getName();
-
-@Before
-public void before() {
+
+public void setup(final StrategyType inputType, final boolean 
inputInOrderIterator, final boolean inputWithCache) {
+type = inputType;
+inOrderIterator = inputInOrderIterator;
+withCache = inputWithCache;
 emitFinal = type.equals(StrategyType.ON_WINDOW_CLOSE);
 emitStrategy = StrategyType.forType(type);
 }
 
-@Test
-public void testAggregateSmallInput() {
+@ParameterizedTest
+@MethodSource("data")

Review Comment:
   how about using `@EnumSource(EmitStrategy.StrategyType.class)`?



##
streams/src/test/java/org/apache/kafka/streams/kstream/internals/SessionWindowedKStreamImplTest.java:
##
@@ -99,14 +95,18 @@ public void before() {
 .emitStrategy(emitStrategy);
 }
 
-@Test
-public void shouldCountSessionWindowedWithCachingDisabled() {
+@ParameterizedTest
+@MethodSource("data")

Review Comment:
   ditto



##
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamTransformValuesTest.java:
##
@@ -34,24 +34,26 @@
 import org.apache.kafka.test.MockProcessorSupplier;
 import org.apache.kafka.test.NoOpValueTransformerWithKeySupplier;
 import org.apache.kafka.test.StreamsTestUtils;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.Mock;
-import org.mockito.junit.MockitoJUnitRunner;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.junit.jupiter.MockitoExtension;
+import org.mockito.junit.jupiter.MockitoSettings;
+import org.mockito.quality.Strictness;
 
 import java.util.Properties;
 
 import static org.hamcrest.CoreMatchers.isA;
 import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.Assert.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertArrayEq

[PR] KAFKA-15623: Migrate streams tests (part 1) module to JUnit 5 [kafka]

2024-06-09 Thread via GitHub 


FrankYang0529 opened a new pull request, #16254:
URL: https://github.com/apache/kafka/pull/16254

   *More detailed description of your change,
   if necessary. The PR title and PR message become
   the squashed commit message, so use a separate
   comment to ping reviewers.*
   
   *Summary of testing strategy (including rationale)
   for the feature or bug fix. Unit and/or integration
   tests are expected for any behaviour change and
   system tests should be considered for larger changes.*
   
   ### 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-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org