advancedxy commented on code in PR #492:
URL: https://github.com/apache/incubator-uniffle/pull/492#discussion_r1071192561
##########
integration-test/common/src/test/java/org/apache/uniffle/test/AssignmentWithTagsTest.java:
##########
@@ -112,7 +109,7 @@ private static void
createAndStartShuffleServerWithTags(Set<String> tags) throws
}
@BeforeAll
- public static void setupServers() throws Exception {
+ public static void setupServers(@TempDir File dir1, @TempDir File dir2,
@TempDir File dir3) throws Exception {
Review Comment:
One `TempDir` should be enough?
The three tempdir could be sub-dirs of the TempDir.
##########
integration-test/common/src/test/java/org/apache/uniffle/test/QuorumTest.java:
##########
@@ -146,36 +153,14 @@ public static void cleanCluster() throws Exception {
coordinators = Lists.newArrayList();
}
- @BeforeEach
- public void initEnv() throws Exception {
- // spark.rss.data.replica=3
- // spark.rss.data.replica.write=2
- // spark.rss.data.replica.read=2
- ((ShuffleServerGrpcClient)ShuffleServerClientFactory
- .getInstance().getShuffleServerClient("GRPC",
shuffleServerInfo0)).adjustTimeout(200);
- ((ShuffleServerGrpcClient)ShuffleServerClientFactory
- .getInstance().getShuffleServerClient("GRPC",
shuffleServerInfo1)).adjustTimeout(200);
- ((ShuffleServerGrpcClient)ShuffleServerClientFactory
- .getInstance().getShuffleServerClient("GRPC",
shuffleServerInfo2)).adjustTimeout(200);
- }
-
@AfterEach
public void cleanEnv() throws Exception {
if (shuffleWriteClientImpl != null) {
shuffleWriteClientImpl.close();
}
cleanCluster();
- initCluster();
- // we need recovery `rpcTime`, or some unit tests may fail
- ((ShuffleServerGrpcClient)ShuffleServerClientFactory
Review Comment:
this should be rolled back?
##########
integration-test/common/src/test/java/org/apache/uniffle/test/ShuffleServerFaultToleranceTest.java:
##########
@@ -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();
Review Comment:
to be consistent with other modules, I think better ways are to pass a
`@TempDir` in L71-L78's `setupServers`.
--
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]
