[ 
https://issues.apache.org/jira/browse/FLINK-6670?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16585667#comment-16585667
 ] 

ASF GitHub Bot commented on FLINK-6670:
---------------------------------------

NicoK commented on a change in pull request #6581: [FLINK-6670][tests] Remove 
CommonTestUtils#createTempDirectory 
URL: https://github.com/apache/flink/pull/6581#discussion_r211189607
 
 

 ##########
 File path: 
flink-tests/src/test/java/org/apache/flink/test/recovery/JobManagerHAProcessFailureBatchRecoveryITCase.java
 ##########
 @@ -95,33 +94,21 @@
 
        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();
+       
+       private File zookeeperStoragePath;
 
 Review comment:
   instead of making this a member of 
`JobManagerHAProcessFailureBatchRecoveryITCase`, you should make this a 
parameter to `testJobManagerFailure()` since this is the only place using the 
variable

----------------------------------------------------------------
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:
us...@infra.apache.org


> 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
>
> {{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)

Reply via email to