[Test] Cleanup tests with Files#createTempDir() #492
Conversation
integration-test/common/src/test/java/org/apache/uniffle/test/AssignmentWithTagsTest.java
Outdated
Show resolved
Hide resolved
| @@ -245,8 +246,6 @@ public static MockedShuffleServer createServer(int id) throws Exception { | |||
| shuffleServerConf.set(ShuffleServerConf.SERVER_APP_EXPIRED_WITHOUT_HEARTBEAT, 5000L); | |||
| shuffleServerConf.set(ShuffleServerConf.DISK_CAPACITY, 1000000L); | |||
| shuffleServerConf.setLong("rss.server.heartbeat.interval", 5000); | |||
| File tmpDir = Files.createTempDir(); | |||
There was a problem hiding this comment.
to be consistent with other modules, I think better ways are to pass a @TempDir in L71-L78's setupServers.
There was a problem hiding this comment.
This is consistent with QuorumTest. It will be a lot of changes if we pass tmpDir into setupServers.
I'm OK with both, though.
There was a problem hiding this comment.
It will be a lot of changes if we pass tmpDir into setupServers
How many changes are we talking about? If there's too many changes, I'm ok with current PR.
And also, why so many changes are required?
integration-test/common/src/test/java/org/apache/uniffle/test/QuorumTest.java
Show resolved
Hide resolved
Codecov Report
@@ Coverage Diff @@
## master #492 +/- ##
============================================
- Coverage 61.76% 58.81% -2.95%
- Complexity 1697 1706 +9
============================================
Files 200 206 +6
Lines 10297 11508 +1211
Branches 1021 1030 +9
============================================
+ Hits 6360 6769 +409
- Misses 3586 4324 +738
- Partials 351 415 +64
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Thanks @advancedxy for the review. There are a few more cases found, let me include them in this PR too. |
|
Thanks @advancedxy for reviewing. |
What changes were proposed in this pull request?
Replace
Files#createTempDir()with@TempDir.Cleanup some tests.
Why are the changes needed?
Files#createTempDir()is deprecated, it's better to use@TempDirfrom JUnit 5.This PR should closes #418
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Existing CI.