smengcl edited a comment on pull request #3304:
URL: https://github.com/apache/hadoop/pull/3304#issuecomment-902345346


   Thanks @aajisaka for trying out the _rewrite_ plugin.
   
   Overall the tool seems to be doing a pretty good job.
   
   1. In the case of `TestHDFSContractPathHandle.java`, It somehow attempts to 
import `org.junit.jupiter.api.Assertions.super` and rearrange the argument 
order to `super()` call, which is kinda amusing.
   
   ```diff
   diff --git 
a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/contract/hdfs/TestHDFSContractPathHandle.java
 
b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/contract/hdfs/TestHDFSContractPathHandle.java
   index c65a60b18b1..a5ea66782cf 100644
   --- 
a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/contract/hdfs/TestHDFSContractPathHandle.java
   +++ 
b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/contract/hdfs/TestHDFSContractPathHandle.java
   @@ -21,11 +21,13 @@
   ...
   +import static org.junit.jupiter.api.Assertions.super;
   +
    /**
     * Verify HDFS compliance with {@link org.apache.hadoop.fs.PathHandle}
     * semantics.
   @@ -35,15 +37,15 @@
    
      public TestHDFSContractPathHandle(String testname, Options.HandleOpt[] 
opts,
          boolean serialized) {
   -    super(testname, opts, serialized);
   +      super(opts, serialized, testname);
      }
   ...
   ```
   
   2. In the case of `TestDiskBalancerRPC.java`, it does remove JUnit 4's 
`ExpectedException` variables, but it doesn't seem to rewrite the 
`ExpectedException.expect` usages.
   
   For instance,
   
   ```java
     @Rule
     public ExpectedException thrown = ExpectedException.none();
   ...
     @Test
     public void testSubmitPlanWithInvalidVersion() throws Exception {
       ...
       thrown.expect(DiskBalancerException.class);
       thrown.expect(new 
DiskBalancerResultVerifier(Result.INVALID_PLAN_VERSION));
       dataNode.submitDiskBalancerPlan(planHash, planVersion, PLAN_FILE,
           plan.toJson(), false);
   ```
   
   should have been rewritten into something like:
   
   ```java
       final DiskBalancerException thrown =
           Assertions.assertThrows(DiskBalancerException.class, () -> {
             dataNode.submitDiskBalancerPlan(planHash, planVersion, PLAN_FILE,
                 plan.toJson(), false);
           });
       Assertions.assertEquals(thrown.getResult(), Result.INVALID_PLAN_VERSION);
   ```
   
   ... and some other interesting changes by rewrite plugin.
   
   I have included a bunch of fixes in my fork of your branch that addresses 
all these rewrite errors I mentioned above and a few others here:
   
   
https://github.com/apache/hadoop/compare/aajisaka:rewrite-junit5-hdfs..smengcl:rewrite-junit5-hdfs?diff=split
   
   Compliation passed locally. I think I might file a PR to trigger the CI 
later.


-- 
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: common-issues-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

Reply via email to