[
https://issues.apache.org/jira/browse/FLINK-6670?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16586099#comment-16586099
]
ASF GitHub Bot commented on FLINK-6670:
---------------------------------------
zentol closed pull request #6581: [FLINK-6670][tests] Remove
CommonTestUtils#createTempDirectory
URL: https://github.com/apache/flink/pull/6581
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/flink-connectors/flink-connector-cassandra/src/test/java/org/apache/flink/streaming/connectors/cassandra/CassandraConnectorITCase.java
b/flink-connectors/flink-connector-cassandra/src/test/java/org/apache/flink/streaming/connectors/cassandra/CassandraConnectorITCase.java
index cb5767a0173..a2a77777ebf 100644
---
a/flink-connectors/flink-connector-cassandra/src/test/java/org/apache/flink/streaming/connectors/cassandra/CassandraConnectorITCase.java
+++
b/flink-connectors/flink-connector-cassandra/src/test/java/org/apache/flink/streaming/connectors/cassandra/CassandraConnectorITCase.java
@@ -35,7 +35,6 @@
import org.apache.flink.batch.connectors.cassandra.CassandraTupleOutputFormat;
import org.apache.flink.configuration.Configuration;
import org.apache.flink.core.io.InputSplit;
-import org.apache.flink.runtime.testutils.CommonTestUtils;
import org.apache.flink.streaming.api.datastream.DataStream;
import org.apache.flink.streaming.api.datastream.DataStreamSource;
import org.apache.flink.streaming.api.environment.StreamExecutionEnvironment;
@@ -54,7 +53,9 @@
import org.junit.Assert;
import org.junit.Before;
import org.junit.BeforeClass;
+import org.junit.ClassRule;
import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -82,7 +83,6 @@
public class CassandraConnectorITCase extends
WriteAheadSinkTestBase<Tuple3<String, Integer, Integer>,
CassandraTupleWriteAheadSink<Tuple3<String, Integer, Integer>>> {
private static final Logger LOG =
LoggerFactory.getLogger(CassandraConnectorITCase.class);
- private static File tmpDir;
private static final boolean EMBEDDED = true;
@@ -119,7 +119,6 @@ protected Cluster buildCluster(Cluster.Builder builder) {
private static final ArrayList<Tuple3<String, Integer, Integer>>
collection = new ArrayList<>(20);
private static final ArrayList<Row> rowCollection = new ArrayList<>(20);
- private static final String[] FIELD_NAMES = {"id", "counter",
"batch_id"};
private static final TypeInformation[] FIELD_TYPES = {
BasicTypeInfo.STRING_TYPE_INFO, BasicTypeInfo.INT_TYPE_INFO,
BasicTypeInfo.INT_TYPE_INFO};
@@ -144,16 +143,15 @@ public void stop() {
}
}
+ @ClassRule
+ public static final TemporaryFolder TEMPORARY_FOLDER = new
TemporaryFolder();
+
@BeforeClass
public static void startCassandra() throws IOException {
- // generate temporary files
- tmpDir = CommonTestUtils.createTempDirectory();
ClassLoader classLoader =
CassandraConnectorITCase.class.getClassLoader();
File file = new
File(classLoader.getResource("cassandra.yaml").getFile());
- File tmp = new File(tmpDir.getAbsolutePath() + File.separator +
"cassandra.yaml");
-
- assertTrue(tmp.createNewFile());
+ File tmp = TEMPORARY_FOLDER.newFile("cassandra.yaml");
try (
BufferedWriter b = new BufferedWriter(new
FileWriter(tmp));
@@ -221,11 +219,6 @@ public static void closeCassandra() {
if (cassandra != null) {
cassandra.stop();
}
-
- if (tmpDir != null) {
- //noinspection ResultOfMethodCallIgnored
- tmpDir.delete();
- }
}
//
------------------------------------------------------------------------
diff --git
a/flink-runtime/src/test/java/org/apache/flink/runtime/testutils/CommonTestUtils.java
b/flink-runtime/src/test/java/org/apache/flink/runtime/testutils/CommonTestUtils.java
index d857a19f810..13e4c8d9a1c 100644
---
a/flink-runtime/src/test/java/org/apache/flink/runtime/testutils/CommonTestUtils.java
+++
b/flink-runtime/src/test/java/org/apache/flink/runtime/testutils/CommonTestUtils.java
@@ -29,7 +29,6 @@
import java.io.StringWriter;
import java.lang.management.ManagementFactory;
import java.lang.management.RuntimeMXBean;
-import java.util.UUID;
/**
* This class contains auxiliary methods for unit tests.
@@ -150,19 +149,6 @@ public static void printLog4jDebugConfig(File file) throws
IOException {
}
}
- public static File createTempDirectory() throws IOException {
- File tempDir = new File(System.getProperty("java.io.tmpdir"));
-
- for (int i = 0; i < 10; i++) {
- File dir = new File(tempDir,
UUID.randomUUID().toString());
- if (!dir.exists() && dir.mkdirs()) {
- return dir;
- }
- }
-
- throw new IOException("Could not create temporary file
directory");
- }
-
/**
* Utility class to read the output of a process stream and forward it
into a StringWriter.
*/
diff --git
a/flink-tests/src/test/java/org/apache/flink/test/recovery/AbstractTaskManagerProcessFailureRecoveryTest.java
b/flink-tests/src/test/java/org/apache/flink/test/recovery/AbstractTaskManagerProcessFailureRecoveryTest.java
index 4144f46b495..56327adaae0 100644
---
a/flink-tests/src/test/java/org/apache/flink/test/recovery/AbstractTaskManagerProcessFailureRecoveryTest.java
+++
b/flink-tests/src/test/java/org/apache/flink/test/recovery/AbstractTaskManagerProcessFailureRecoveryTest.java
@@ -41,8 +41,9 @@
import akka.actor.ActorSystem;
import akka.pattern.Patterns;
import akka.util.Timeout;
-import org.apache.commons.io.FileUtils;
+import org.junit.Rule;
import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -87,6 +88,9 @@
protected static final int PARALLELISM = 4;
+ @Rule
+ public final TemporaryFolder temporaryFolder = new TemporaryFolder();
+
@Test
public void testTaskManagerProcessFailure() throws Exception {
@@ -117,7 +121,7 @@ public void testTaskManagerProcessFailure() throws
Exception {
CommonTestUtils.printLog4jDebugConfig(tempLogFile);
// coordination between the processes goes through a
directory
- coordinateTempDir =
CommonTestUtils.createTempDirectory();
+ coordinateTempDir = temporaryFolder.newFolder();
// find a free port to start the JobManager
final int jobManagerPort = NetUtils.getAvailablePort();
@@ -268,14 +272,6 @@ public void run() {
if (jmActorSystem != null) {
jmActorSystem.shutdown();
}
- if (coordinateTempDir != null) {
- try {
-
FileUtils.deleteDirectory(coordinateTempDir);
- }
- catch (Throwable t) {
- // we can ignore this
- }
- }
if (highAvailabilityServices != null) {
highAvailabilityServices.closeAndCleanupAllData();
diff --git
a/flink-tests/src/test/java/org/apache/flink/test/recovery/JobManagerHACheckpointRecoveryITCase.java
b/flink-tests/src/test/java/org/apache/flink/test/recovery/JobManagerHACheckpointRecoveryITCase.java
index a22a8a8c930..ebe45578a67 100644
---
a/flink-tests/src/test/java/org/apache/flink/test/recovery/JobManagerHACheckpointRecoveryITCase.java
+++
b/flink-tests/src/test/java/org/apache/flink/test/recovery/JobManagerHACheckpointRecoveryITCase.java
@@ -52,7 +52,6 @@
import org.apache.flink.runtime.taskmanager.TaskManager;
import org.apache.flink.runtime.testingUtils.TestingUtils;
import org.apache.flink.runtime.testtasks.BlockingNoOpInvokable;
-import org.apache.flink.runtime.testutils.CommonTestUtils;
import org.apache.flink.runtime.testutils.JobManagerActorTestUtils;
import org.apache.flink.runtime.testutils.JobManagerProcess;
import org.apache.flink.runtime.testutils.ZooKeeperTestUtils;
@@ -82,7 +81,6 @@
import org.slf4j.LoggerFactory;
import java.io.File;
-import java.io.IOException;
import java.util.Collections;
import java.util.List;
import java.util.UUID;
@@ -113,42 +111,23 @@
@Rule
public RetryRule retryRule = new RetryRule();
+ @Rule
+ public final TemporaryFolder temporaryFolder = new TemporaryFolder();
+
private static final ZooKeeperTestEnvironment ZooKeeper = new
ZooKeeperTestEnvironment(1);
private static final FiniteDuration TestTimeOut = new FiniteDuration(5,
TimeUnit.MINUTES);
- private static final File FileStateBackendBasePath;
-
- static {
- try {
- FileStateBackendBasePath =
CommonTestUtils.createTempDirectory();
- }
- catch (IOException e) {
- throw new RuntimeException("Error in test setup. Could
not create directory.", e);
- }
- }
-
@AfterClass
- public static void tearDown() throws Exception {
+ public static void tearDown() {
try {
ZooKeeper.shutdown();
} catch (Exception ignored) {
}
-
- try {
- if (FileStateBackendBasePath != null) {
-
FileUtils.deleteDirectory(FileStateBackendBasePath);
- }
- } catch (IOException ignored) {
- }
}
@Before
public void cleanUp() throws Exception {
- if (FileStateBackendBasePath != null &&
FileStateBackendBasePath.exists()) {
- FileUtils.cleanDirectory(FileStateBackendBasePath);
- }
-
ZooKeeper.deleteAll();
}
@@ -179,7 +158,7 @@ public void cleanUp() throws Exception {
public void testCheckpointRecoveryFailure() throws Exception {
final Deadline testDeadline = TestTimeOut.fromNow();
final String zooKeeperQuorum = ZooKeeper.getConnectString();
- final String fileStateBackendPath =
FileStateBackendBasePath.getAbsoluteFile().toString();
+ final String fileStateBackendPath =
temporaryFolder.newFolder().toString();
Configuration config =
ZooKeeperTestUtils.createZooKeeperHAConfig(
zooKeeperQuorum,
@@ -266,7 +245,7 @@ public void testCheckpointRecoveryFailure() throws
Exception {
testDeadline.timeLeft());
// Remove all files
- FileUtils.deleteDirectory(FileStateBackendBasePath);
+ FileUtils.deleteDirectory(new
File(fileStateBackendPath));
// Kill the leader
leadingJobManagerProcess.destroy();
@@ -340,7 +319,7 @@ public void testCheckpointRecoveryFailure() throws
Exception {
public void testCheckpointedStreamingProgramIncrementalRocksDB() throws
Exception {
testCheckpointedStreamingProgram(
new RocksDBStateBackend(
- new
FsStateBackend(FileStateBackendBasePath.getAbsoluteFile().toURI(), 16),
+ new
FsStateBackend(temporaryFolder.newFolder().getAbsoluteFile().toURI(), 16),
true));
}
diff --git
a/flink-tests/src/test/java/org/apache/flink/test/recovery/JobManagerHAProcessFailureBatchRecoveryITCase.java
b/flink-tests/src/test/java/org/apache/flink/test/recovery/JobManagerHAProcessFailureBatchRecoveryITCase.java
index 7beb9273c2a..d3accffcbf8 100644
---
a/flink-tests/src/test/java/org/apache/flink/test/recovery/JobManagerHAProcessFailureBatchRecoveryITCase.java
+++
b/flink-tests/src/test/java/org/apache/flink/test/recovery/JobManagerHAProcessFailureBatchRecoveryITCase.java
@@ -40,7 +40,6 @@
import org.apache.flink.runtime.metrics.NoOpMetricRegistry;
import org.apache.flink.runtime.taskmanager.TaskManager;
import org.apache.flink.runtime.testingUtils.TestingUtils;
-import org.apache.flink.runtime.testutils.CommonTestUtils;
import org.apache.flink.runtime.testutils.JobManagerActorTestUtils;
import org.apache.flink.runtime.testutils.JobManagerProcess;
import org.apache.flink.runtime.testutils.ZooKeeperTestUtils;
@@ -53,12 +52,13 @@
import org.apache.commons.io.FileUtils;
import org.junit.AfterClass;
import org.junit.Before;
+import org.junit.Rule;
import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import java.io.File;
-import java.io.IOException;
import java.util.Arrays;
import java.util.Collection;
import java.util.UUID;
@@ -68,7 +68,6 @@
import scala.concurrent.duration.Deadline;
import scala.concurrent.duration.FiniteDuration;
-import static
org.apache.flink.runtime.testutils.CommonTestUtils.createTempDirectory;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.fail;
@@ -95,33 +94,19 @@
private static final FiniteDuration TestTimeOut = new FiniteDuration(5,
TimeUnit.MINUTES);
- private static final File FileStateBackendBasePath;
-
- static {
- try {
- FileStateBackendBasePath =
CommonTestUtils.createTempDirectory();
- }
- catch (IOException e) {
- throw new RuntimeException("Error in test setup. Could
not create directory.", e);
- }
- }
+ @Rule
+ public final TemporaryFolder temporaryFolder = new TemporaryFolder();
@AfterClass
public static void tearDown() throws Exception {
if (ZooKeeper != null) {
ZooKeeper.shutdown();
}
-
- if (FileStateBackendBasePath != null) {
- FileUtils.deleteDirectory(FileStateBackendBasePath);
- }
}
@Before
public void cleanUp() throws Exception {
ZooKeeper.deleteAll();
-
- FileUtils.cleanDirectory(FileStateBackendBasePath);
}
protected static final String READY_MARKER_FILE_PREFIX = "ready_";
@@ -154,12 +139,12 @@ public
JobManagerHAProcessFailureBatchRecoveryITCase(ExecutionMode executionMode
* @param coordinateDir Coordination directory
* @throws Exception
*/
- public void testJobManagerFailure(String zkQuorum, final File
coordinateDir) throws Exception {
+ private void testJobManagerFailure(String zkQuorum, final File
coordinateDir, final File zookeeperStoragePath) throws Exception {
Configuration config = new Configuration();
config.setString(CoreOptions.MODE, CoreOptions.LEGACY_MODE);
config.setString(HighAvailabilityOptions.HA_MODE, "ZOOKEEPER");
config.setString(HighAvailabilityOptions.HA_ZOOKEEPER_QUORUM,
zkQuorum);
- config.setString(HighAvailabilityOptions.HA_STORAGE_PATH,
FileStateBackendBasePath.getAbsolutePath());
+ config.setString(HighAvailabilityOptions.HA_STORAGE_PATH,
zookeeperStoragePath.getAbsolutePath());
ExecutionEnvironment env =
ExecutionEnvironment.createRemoteEnvironment(
"leader", 1, config);
@@ -228,6 +213,8 @@ public void flatMap(Long value, Collector<Long> out) throws
Exception {
@Test
public void testJobManagerProcessFailure() throws Exception {
+ final File zookeeperStoragePath = temporaryFolder.newFolder();
+
// Config
final int numberOfJobManagers = 2;
final int numberOfTaskManagers = 2;
@@ -256,11 +243,11 @@ public void testJobManagerProcessFailure() throws
Exception {
final Deadline deadline = TestTimeOut.fromNow();
// Coordination directory
- coordinateTempDir = createTempDirectory();
+ coordinateTempDir = temporaryFolder.newFolder();
// Job Managers
Configuration config =
ZooKeeperTestUtils.createZooKeeperHAConfig(
- ZooKeeper.getConnectString(),
FileStateBackendBasePath.getPath());
+ ZooKeeper.getConnectString(),
zookeeperStoragePath.getPath());
// Start first process
jmProcess[0] = new JobManagerProcess(0, config);
@@ -322,7 +309,7 @@ public void testJobManagerProcessFailure() throws Exception
{
@Override
public void run() {
try {
-
testJobManagerFailure(ZooKeeper.getConnectString(), coordinateDirClosure);
+
testJobManagerFailure(ZooKeeper.getConnectString(), coordinateDirClosure,
zookeeperStoragePath);
}
catch (Throwable t) {
t.printStackTrace();
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> remove CommonTestUtils.createTempDirectory()
> --------------------------------------------
>
> Key: FLINK-6670
> URL: https://issues.apache.org/jira/browse/FLINK-6670
> Project: Flink
> Issue Type: Bug
> Components: Tests
> Reporter: Nico Kruber
> Assignee: Chesnay Schepler
> Priority: Minor
> Labels: pull-request-available
> Fix For: 1.7.0
>
>
> {{CommonTestUtils.createTempDirectory()}} encourages a dangerous design
> pattern with potential concurrency issues in the unit tests as well as the
> need to cleanup the created directories.
> Instead, it should be solved by using the following pattern:
> {code:java}
> @Rule
> public TemporaryFolder tempFolder = new TemporaryFolder();
> {code}
> We should therefore remove {{CommonTestUtils.createTempDirectory()}}.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
