Skip to content

Conversation

@aajisaka
Copy link
Member

@aajisaka aajisaka commented Aug 14, 2021

Description of PR

Test rewrite-maven-plugin https://docs.openrewrite.org/tutorials/migrate-from-junit-4-to-junit-5

How was this patch tested?

Not tested

For code changes:

  • Does the title or this PR starts with the corresponding JIRA issue id (e.g. 'HADOOP-17799. Your PR title ...')?
  • Object storage: have the integration tests been executed and the endpoint declared according to the connector-specific documentation?
  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
  • If applicable, have you updated the LICENSE, LICENSE-binary, NOTICE-binary files?

@hadoop-yetus

This comment has been minimized.

@aajisaka
Copy link
Member Author

The plugin failed to convert some contract tests.

@smengcl
Copy link
Contributor

smengcl commented Aug 20, 2021

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 --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);
   }
...
  1. 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,

  @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:

    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.

@smengcl
Copy link
Contributor

smengcl commented Aug 20, 2021

Filed #3316 for CI on my branch.

@aajisaka
Copy link
Member Author

aajisaka commented Nov 17, 2025

The oiriginal issue has been resolved. Closing

@aajisaka aajisaka closed this Nov 17, 2025
@aajisaka aajisaka deleted the rewrite-junit5-hdfs branch November 17, 2025 05:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants