You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by GitBox <gi...@apache.org> on 2020/02/05 01:24:56 UTC

[GitHub] [incubator-hudi] nbalajee opened a new pull request #1307: [HUDI-570] - Improve test coverage for FSUtils.java

nbalajee opened a new pull request #1307: [HUDI-570] - Improve test coverage for FSUtils.java
URL: https://github.com/apache/incubator-hudi/pull/1307
 
 
   
   ## What is the purpose of the pull request
   Increase the test coverage for FSUtils.java under hudi-common.
   
   ## Brief change log
   Added test cases to increase coverage.
   
   ## Verify this pull request
   This change added tests and can be verified as follows:
   - testDeleteOlderRollbackFiles
   - testDeleteOlderCleanMetaFiles
   - testFileNameRelatedFunctions
   
   ## Committer checklist
   
    - [x ] Has a corresponding JIRA in PR title & commit
    
    - [x ] Commit message is descriptive of the change
    
    - [x ] CI is green
   
    - [ ] Necessary doc changes done or have another open PR
          
    - [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-hudi] n3nash commented on a change in pull request #1307: [HUDI-570] - Improve test coverage for FSUtils.java

Posted by GitBox <gi...@apache.org>.
n3nash commented on a change in pull request #1307: [HUDI-570] - Improve test coverage for FSUtils.java
URL: https://github.com/apache/incubator-hudi/pull/1307#discussion_r375038980
 
 

 ##########
 File path: hudi-common/src/test/java/org/apache/hudi/common/util/TestFSUtils.java
 ##########
 @@ -43,11 +49,33 @@
 import java.util.stream.Stream;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 /**
  * Tests file system utils.
  */
 public class TestFSUtils extends HoodieCommonTestHarness {
+  private long minRollbackToKeep = 10;
+  private long minCleanToKeep = 10;
+  protected transient FileSystem fs;
+  protected String basePath = null;
+
+  @Before
 
 Review comment:
   You don't need this init, the HoodieTestHarness actually creates a DFS based file system which works for log records and should not run into the exception for which you might be creating the local file system 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-hudi] nbalajee commented on a change in pull request #1307: [HUDI-570] Improve test coverage for FSUtils.java

Posted by GitBox <gi...@apache.org>.
nbalajee commented on a change in pull request #1307: [HUDI-570] Improve test coverage for FSUtils.java
URL: https://github.com/apache/incubator-hudi/pull/1307#discussion_r375520095
 
 

 ##########
 File path: hudi-common/src/test/java/org/apache/hudi/common/util/TestFSUtils.java
 ##########
 @@ -43,11 +49,33 @@
 import java.util.stream.Stream;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 /**
  * Tests file system utils.
  */
 public class TestFSUtils extends HoodieCommonTestHarness {
+  private long minRollbackToKeep = 10;
+  private long minCleanToKeep = 10;
+  protected transient FileSystem fs;
+  protected String basePath = null;
+
+  @Before
 
 Review comment:
   incorporated the comments.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-hudi] n3nash commented on a change in pull request #1307: [HUDI-570] - Improve test coverage for FSUtils.java

Posted by GitBox <gi...@apache.org>.
n3nash commented on a change in pull request #1307: [HUDI-570] - Improve test coverage for FSUtils.java
URL: https://github.com/apache/incubator-hudi/pull/1307#discussion_r375038467
 
 

 ##########
 File path: hudi-common/src/test/java/org/apache/hudi/common/util/TestFSUtils.java
 ##########
 @@ -43,11 +49,33 @@
 import java.util.stream.Stream;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 /**
  * Tests file system utils.
  */
 public class TestFSUtils extends HoodieCommonTestHarness {
+  private long minRollbackToKeep = 10;
 
 Review comment:
   We shouldn't have counter variables as members of a test class, each test might mutate it and eventually break other tests

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-hudi] n3nash commented on a change in pull request #1307: [HUDI-570] - Improve test coverage for FSUtils.java

Posted by GitBox <gi...@apache.org>.
n3nash commented on a change in pull request #1307: [HUDI-570] - Improve test coverage for FSUtils.java
URL: https://github.com/apache/incubator-hudi/pull/1307#discussion_r375039117
 
 

 ##########
 File path: hudi-common/src/test/java/org/apache/hudi/common/util/TestFSUtils.java
 ##########
 @@ -261,4 +289,87 @@ public void testLogFilesComparison() {
   public static String makeOldLogFileName(String fileId, String logFileExtension, String baseCommitTime, int version) {
     return "." + String.format("%s_%s%s.%d", fileId, baseCommitTime, logFileExtension, version);
   }
+
+  private void cleanupFiles(File[] cleanupFiles) {
 
 Review comment:
   Once you use the basePath from the DFS started by the TestHarness and create files underneath, when the test dies, it will automatically clean those files. You shouldn't need to clean it here

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-hudi] nbalajee commented on a change in pull request #1307: [HUDI-570] Improve test coverage for FSUtils.java

Posted by GitBox <gi...@apache.org>.
nbalajee commented on a change in pull request #1307: [HUDI-570] Improve test coverage for FSUtils.java
URL: https://github.com/apache/incubator-hudi/pull/1307#discussion_r375520024
 
 

 ##########
 File path: hudi-common/src/test/java/org/apache/hudi/common/util/TestFSUtils.java
 ##########
 @@ -43,11 +49,33 @@
 import java.util.stream.Stream;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 /**
  * Tests file system utils.
  */
 public class TestFSUtils extends HoodieCommonTestHarness {
+  private long minRollbackToKeep = 10;
 
 Review comment:
   incorporated the comments.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-hudi] n3nash merged pull request #1307: [HUDI-570] Improve test coverage for FSUtils.java

Posted by GitBox <gi...@apache.org>.
n3nash merged pull request #1307: [HUDI-570] Improve test coverage for FSUtils.java
URL: https://github.com/apache/incubator-hudi/pull/1307
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services