test: convert tests in druid-processing to junit5 pt. 9#19601
test: convert tests in druid-processing to junit5 pt. 9#19601clintropolis wants to merge 5 commits into
Conversation
clintropolis
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| 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()); |
FrankChen021
left a comment
FrankChen021
left a comment
There was a problem hiding this comment.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 1 |
| P2 | 0 |
| P3 | 2 |
| Total | 3 |
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 1 |
| P2 | 0 |
| P3 | 2 |
| Total | 3 |
Reviewed 49 of 49 changed files.
This is an automated review by Codex GPT-5.5
| import org.junit.Test; | ||
| import org.junit.jupiter.api.Assertions; | ||
| import org.junit.jupiter.api.BeforeEach; | ||
| import org.junit.jupiter.api.Test; |
There was a problem hiding this 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.
| expectedException.expectMessage(StringUtils.format("Unable to parse header [%s]", header)); | ||
|
|
||
| PARSER_FACTORY.get(format, header); | ||
| Assertions.assertThrows( |
There was a problem hiding this 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.
|
|
||
| // Workaround so "withChildren" isn't flagged as unused in the DataSource interface. | ||
| ((DataSource) listDataSource).withChildren(ImmutableList.of(new TableDataSource("foo"))); | ||
| Assertions.assertThrows( |
There was a problem hiding this 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.
FrankChen021
left a comment
FrankChen021
left a comment
There was a problem hiding this comment.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 0 |
| P2 | 0 |
| P3 | 1 |
| Total | 1 |
Reviewed 49 of 49 changed files.
This is an automated review by Codex GPT-5.5
| concat(format, "hello", "world", "foo") | ||
| }; | ||
| parser.parseToMap(body[0]); | ||
| Assertions.assertThrows( |
There was a problem hiding this comment.
[P3] Restore the expected exception message assertion
The JUnit 4 version of this test checked that the UnsupportedOperationException message contained hasHeaderRow or maxSkipHeaderRows is not supported..., but the Jupiter conversion now only asserts the exception type. A regression that throws the same exception for the wrong reason or drops the user-facing diagnostic would pass this test. Capture the thrown exception and assert the message, as done for testDuplicatedColumnName.
3 participants
next batch
previous: