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]