openrewrite / rewrite-testing-frameworks

OpenRewrite recipes that perform common Java testing migration tasks.
Apache License 2.0
73 stars 64 forks source link

Issue discovered with `src/test/java/com/microsoft/jenkins/appservice/test/FilePathUtilsTest.java` #242

Closed jkschneider closed 2 years ago

jkschneider commented 2 years ago

Problem

Describe the issue you are experiencing.

Expected behavior

Describe what you expected to see.

Example diff

 import hudson.FilePath;
 import org.apache.commons.io.FilenameUtils;
 import com.microsoft.jenkins.appservice.util.FilePathUtils;
-import org.junit.Assert;
 import org.junit.Rule;
-import org.junit.Test;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
 import org.junit.rules.TemporaryFolder;

 public class FilePathUtilsTest {
     public TemporaryFolder workspaceDir = new TemporaryFolder();

     @Test
-    public void trimDirectoryPrefix() throws Exception {
-        assertTrimDirectoryPrefix("a", "a/b/c.txt", "b/c.txt");
+    void trimDirectoryPrefix() throws Exception {
+        /*~~(java.lang.NullPointerException: Cannot invoke "org.openrewrite.java.tree.JavaType$FullyQualified.getFullyQualifiedName()" because the return value of "org.openrewrite.java.tree.JavaType$Method.getDeclaringType()" is null
+  org.openrewrite.java.testing.junit5.TemporaryFolderToTempDir$TemporaryFolderToTempDirVisitor.visitMethodInvocation(TemporaryFolderToTempDir.java:112)
+  org.openrewrite.java.testing.junit5.TemporaryFolderToTempDir$TemporaryFolderToTempDirVisitor.visitMethodInvocation(TemporaryFolderToTempDir.java:67)
+  org.openrewrite.java.tree.J$MethodInvocation.acceptJava(J.java:3409)
+  org.openrewrite.java.tree.J.accept(J.java:55)
+  org.openrewrite.TreeVisitor.visit(TreeVisitor.java:206)
+  org.openrewrite.TreeVisitor.visitAndCast(TreeVisitor.java:285)
+  org.openrewrite.java.JavaVisitor.visitRightPadded(JavaVisitor.java:1231)
+  org.openrewrite.java.JavaVisitor.lambda$visitBlock$4(JavaVisitor.java:373)
+  ...)~~>*/assertTrimDirectoryPrefix("a", "a/b/c.txt", "b/c.txt");
         assertTrimDirectoryPrefix("a", "a/b.txt", "b.txt");
         assertTrimDirectoryPrefix("a/", "a/b.txt", "b.txt");
         assertTrimDirectoryPrefix("", "c.txt", "c.txt");
         final FilePath file = workspace.child(filePath);

         final String remoteName = FilePathUtils.trimDirectoryPrefix(sourceDir, file);
-        Assert.assertEquals(expectedRemoteName, FilenameUtils.separatorsToUnix(remoteName));
+        Assertions.assertEquals(expectedRemoteName, FilenameUtils.separatorsToUnix(remoteName));
     }
 }

Recipes in example diff:

References:

pway99 commented 2 years ago

@jkschneider I was not able to reproduce this issue via unit tests; however, the code should have been using TypeUtils.

The original code had a null check on MethodType and MethodType.DeclaringType is a non-null attribute, I am thinking its either an issue with the AST or the stack trace is not associated with the correct element. The project was a bit challenging to build locally, I just updated the recipe so we can re-test on the SaaS.

String declaringType = mi.getMethodType() != null ? mi.getMethodType().getDeclaringType().getFullyQualifiedName() : null;
if ("org.junit.rules.TemporaryFolder".equals(declaringType) && mi.getSelect() != null)
pway99 commented 2 years ago

fixed in: 37fb56a149a25a2f45192688df699a93700ce8c2

Image