Uh oh!
There was an error while loading. Please reload this page.
TEZ-4719: Move to Junit6 - #502
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
| import org.junit.Assert; | ||
| import org.junit.Test; | ||
| import org.junit.jupiter.api.*; |
There was a problem hiding this comment.
Which is the preferred way?
- Using single line separte import
*imports
This comment was marked as outdated.
This comment was marked as outdated.
Aggarwal-Raghav
commented
Jun 2, 2026
UT has passed, which is good thing, few indendations and java formatting is left that will be handled once final rebase with #505 is done. |
5fb400c to
5f401beCompare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
tez-yetus
commented
Jun 28, 2026
💔 -1 overall
This message was automatically generated. |
Aggarwal-Raghav
commented
Jun 29, 2026
@abstractdog , this is ready for review. |
abstractdog
left a comment
There was a problem hiding this comment.
this is huge so far, thanks @Aggarwal-Raghav ! left some comments
| Environment.PWD.$() | ||
| + File.pathSeparator | ||
| + "USER_PATH" | ||
| + File.pathSeparator | ||
| + "DEFAULT_PATH", |
There was a problem hiding this comment.
it's a matter of taste, but I don't think indenting a string concatenation this way makes the code better, a single line is more compact and still pretty much readable
There was a problem hiding this comment.
i was using google java formatter in intelliJ, now I have moved to tez project checkstyle present at tez-build-tools/src/main/resources/checkstyle/checkstyle.xml and updated the whole PR manually file by file. Fixed line length >120 in this PR. I hope it should help in checkstyle/spotbugs as well.
There was a problem hiding this comment.
thanks for clarifying!
I believe we can start soon a community discussion about which formatter to accept, and then we can enforce it by spotless: that's going to be another huge patch, not a problem :)
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
dd8e0a0 to
6374968Compare
This comment was marked as outdated.
This comment was marked as outdated.
Aggarwal-Raghav
commented
Jun 30, 2026
CI has failed because of some artifactory issue, have re-triggered it |
This comment was marked as outdated.
This comment was marked as outdated.
Giving 1 more try, there is some infra issue i guess, puppycrawler is going OOM, and different issue in checkstyle , sporbugs. then will play aroung Jenkinsfile. I know there are few checkstyle issues still because of this PR, will handle once CI start passing. |
Aggarwal-Raghav
commented
Jul 1, 2026
Failing with OOM. |
Aggarwal-Raghav
commented
Jul 1, 2026
@abstractdog , is it because of some jenkins node problem? |
Aggarwal-Raghav
commented
Jul 1, 2026
rebasing as there are merge conflict. The older run is stuck, neither failing nor proceeding. |
abstractdog
commented
Jul 1, 2026
yeah, please be aware of a recent unit test, which might need to be migrated: 8fde353 |
Aggarwal-Raghav
commented
Jul 1, 2026
yes, doing that only :-) |
Aggarwal-Raghav
commented
Jul 1, 2026
For |
tez-yetus
commented
Jul 1, 2026
💔 -1 overall
This message was automatically generated. |
Aggarwal-Raghav
commented
Jul 1, 2026
CI has passed @abstractdog , I think the checkstyle especially in javac can be handled separately? |
abstractdog
commented
Jul 2, 2026
agree! those problems were not introduced by this patch, and this is already huge, it's worth moving on before further conflicts :) |
Uh oh!
There was an error while loading. Please reload this page.
Update Assertions:
org.junit.Assert.* → org.junit.jupiter.api.Assertions.*Removed the transitive dependency on
Hamcrestmatchers.Migrated all
@Test(timeout = ...)parameters to the@Timeoutannotation while maintianing the Millisecond timeunitReplaced
@Rule TemporaryFolderwith JUnit Jupiter's@TempDirNote: In JUnit 4, the custom assertion message was the first parameter. In JUnit 6, it is the last parameter.
In JUnit 4:
assertEquals("Values should match", expected, actual);In JUnit 6:
assertEquals(expected, actual, "Values should match");