FrankChen021 commented on code in PR #19601:
URL: https://github.com/apache/druid/pull/19601#discussion_r3442411076


##########
processing/src/test/java/org/apache/druid/segment/join/lookup/LookupJoinableTest.java:
##########
@@ -29,10 +29,10 @@
 import org.apache.druid.segment.column.ValueType;
 import org.apache.druid.segment.join.Joinable;
 import org.apache.druid.testing.InitializedNullHandlingTest;
-import org.junit.Assert;
-import org.junit.Before;
 import org.junit.Ignore;
-import org.junit.Test;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;

Review Comment:
   [P1] Convert the remaining @Ignore to @Disabled
   
   This file now imports Jupiter's Test annotation, but it still imports 
org.junit.Ignore and leaves the skipped method annotated with @Ignore. Jupiter 
does not honor that JUnit 4 skip annotation here, so the test described as 
known-failing will start running and can break the test suite. Replace the 
import and annotation with org.junit.jupiter.api.Disabled.



##########
processing/src/test/java/org/apache/druid/query/InlineDataSourceTest.java:
##########
@@ -163,73 +158,73 @@ public void test_rowAdapter()
     final RowAdapter<Object[]> adapter = listDataSource.rowAdapter();
     final Object[] row = rows.get(1);
 
-    Assert.assertEquals(DateTimes.of("2000").getMillis(), 
adapter.timestampFunction().applyAsLong(row));
-    Assert.assertEquals("bar", adapter.columnFunction("str").apply(row));
-    Assert.assertEquals(1d, adapter.columnFunction("double").apply(row));
-    Assert.assertEquals(ImmutableMap.of("n", "1"), 
adapter.columnFunction("complex").apply(row));
-    Assert.assertEquals(ImmutableList.of(2.0, 4.0), 
adapter.columnFunction("double_array").apply(row));
+    Assertions.assertEquals(DateTimes.of("2000").getMillis(), 
adapter.timestampFunction().applyAsLong(row));
+    Assertions.assertEquals("bar", adapter.columnFunction("str").apply(row));
+    Assertions.assertEquals(1d, adapter.columnFunction("double").apply(row));
+    Assertions.assertEquals(ImmutableMap.of("n", "1"), 
adapter.columnFunction("complex").apply(row));
+    Assertions.assertEquals(ImmutableList.of(2.0, 4.0), 
adapter.columnFunction("double_array").apply(row));
   }
 
   @Test
   public void test_getRows_list()
   {
-    Assert.assertSame(this.rows, listDataSource.getRowsAsList());
+    Assertions.assertSame(this.rows, listDataSource.getRowsAsList());
   }
 
   @Test
   public void test_getRows_iterable()
   {
     final Iterable<Object[]> iterable = iterableDataSource.getRows();
-    Assert.assertNotSame(this.rows, iterable);
+    Assertions.assertNotSame(this.rows, iterable);
 
     // No iteration yet.
-    Assert.assertEquals(0, iterationCounter.get());
+    Assertions.assertEquals(0, iterationCounter.get());
 
     assertRowsEqual(this.rows, ImmutableList.copyOf(iterable));
 
     // OK, now we've iterated.
-    Assert.assertEquals(1, iterationCounter.get());
+    Assertions.assertEquals(1, iterationCounter.get());
 
     // Read again, we should iterate again.
     //noinspection MismatchedQueryAndUpdateOfCollection
     final List<Object[]> ignored = Lists.newArrayList(iterable);
-    Assert.assertEquals(2, iterationCounter.get());
+    Assertions.assertEquals(2, iterationCounter.get());
   }
 
   @Test
   public void test_getRowsAsList_list()
   {
-    Assert.assertSame(this.rows, listDataSource.getRowsAsList());
+    Assertions.assertSame(this.rows, listDataSource.getRowsAsList());
   }
 
   @Test
   public void test_getRowsAsList_iterable()
   {
     final List<Object[]> list = iterableDataSource.getRowsAsList();
 
-    Assert.assertEquals(1, iterationCounter.get());
+    Assertions.assertEquals(1, iterationCounter.get());
     assertRowsEqual(this.rows, list);
 
     // Read again, we should *not* iterate again (in contrast to 
"test_getRows_iterable").
     //noinspection MismatchedQueryAndUpdateOfCollection
     final List<Object[]> ignored = Lists.newArrayList(list);
-    Assert.assertEquals(1, iterationCounter.get());
+    Assertions.assertEquals(1, iterationCounter.get());
   }
 
   @Test
   public void test_withChildren_empty()
   {
-    Assert.assertSame(listDataSource, 
listDataSource.withChildren(Collections.emptyList()));
+    Assertions.assertSame(listDataSource, 
listDataSource.withChildren(Collections.emptyList()));
   }
 
   @Test
   public void test_withChildren_nonEmpty()
   {
-    expectedException.expect(IllegalArgumentException.class);
-    expectedException.expectMessage("Cannot accept children");
-
-    // Workaround so "withChildren" isn't flagged as unused in the DataSource 
interface.
-    ((DataSource) listDataSource).withChildren(ImmutableList.of(new 
TableDataSource("foo")));
+    Assertions.assertThrows(

Review Comment:
   [P3] Keep checking the withChildren error message
   
   The JUnit 4 version asserted both the IllegalArgumentException type and the 
message "Cannot accept children". After the migration this only checks the 
type, so a regression in the DataSource contract text would no longer be 
caught. Capture the thrown exception and assert the message.



##########
processing/src/test/java/org/apache/druid/java/util/common/parsers/FlatTextFormatParserTest.java:
##########
@@ -25,60 +25,52 @@
 import org.apache.druid.java.util.common.StringUtils;
 import 
org.apache.druid.java.util.common.parsers.AbstractFlatTextFormatParser.FlatTextFormat;
 import org.apache.druid.testing.InitializedNullHandlingTest;
-import org.junit.Assert;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.ExpectedException;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
-import org.junit.runners.Parameterized.Parameters;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.Parameter;
+import org.junit.jupiter.params.ParameterizedClass;
+import org.junit.jupiter.params.provider.MethodSource;
 
 import java.util.Arrays;
-import java.util.Collection;
 import java.util.Map;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
-@RunWith(Parameterized.class)
+@ParameterizedClass
+@MethodSource("constructorFeeder")
 public class FlatTextFormatParserTest extends InitializedNullHandlingTest
 {
-  @Parameters(name = "{0}")
-  public static Collection<Object[]> constructorFeeder()
+  public static Stream<Object[]> constructorFeeder()
   {
     return ImmutableList.of(
         new Object[]{FlatTextFormat.CSV},
         new Object[]{FlatTextFormat.DELIMITED}
-    );
+    ).stream();
   }
 
   private static final FlatTextFormatParserFactory PARSER_FACTORY = new 
FlatTextFormatParserFactory();
 
-  @Rule
-  public ExpectedException expectedException = ExpectedException.none();
-
-  private final FlatTextFormat format;
-
-  public FlatTextFormatParserTest(FlatTextFormat format)
-  {
-    this.format = format;
-  }
+  @Parameter(0)
+  public FlatTextFormat format;
 
   @Test
   public void testValidHeader()
   {
     final String header = concat(format, "time", "value1", "value2");
     final Parser<String, Object> parser = PARSER_FACTORY.get(format, header);
-    Assert.assertEquals(ImmutableList.of("time", "value1", "value2"), 
parser.getFieldNames());
+    Assertions.assertEquals(ImmutableList.of("time", "value1", "value2"), 
parser.getFieldNames());
   }
 
   @Test
   public void testDuplicatedColumnName()
   {
     final String header = concat(format, "time", "value1", "value2", "value2");
 
-    expectedException.expect(ParseException.class);
-    expectedException.expectMessage(StringUtils.format("Unable to parse header 
[%s]", header));
-
-    PARSER_FACTORY.get(format, header);
+    Assertions.assertThrows(

Review Comment:
   [P3] Preserve the exception message assertions
   
   The old ExpectedException checks verified the thrown message, but the 
converted assertThrows call uses the expected text as the optional assertion 
failure message instead. That means testDuplicatedColumnName no longer verifies 
the ParseException message, and testWithoutStartFileFromBeginning below only 
checks the exception type. Capture the exception returned by assertThrows and 
assert its message.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to