TestsShouldNotBePublic should not reduce visibility if test classes are extended

[timtebeek]

TestsShouldNotBePublic should not reduce visibility of classes used in subsequent test classes, even if those are in Groovy. As seen in openrewrite/rewrite#2800.

Compilation fails as JavaTypeSignatureBuilderTest and JavaTypeMappingTest should not have reduced visibility for Groovy.

/home/tim/Documents/workspace/rewrite/rewrite-groovy/src/test/java/org/openrewrite/groovy/GroovyTypeSignatureBuilderTest.java:24: error: JavaTypeSignatureBuilderTest is not public in org.openrewrite.java; cannot be accessed from outside package
import org.openrewrite.java.JavaTypeSignatureBuilderTest;
                           ^
/home/tim/Documents/workspace/rewrite/rewrite-groovy/src/test/java/org/openrewrite/groovy/GroovyTypeSignatureBuilderTest.java:35: error: JavaTypeSignatureBuilderTest is not public in org.openrewrite.java; cannot be accessed from outside package
public class GroovyTypeSignatureBuilderTest implements JavaTypeSignatureBuilderTest {
                                                       ^
/home/tim/Documents/workspace/rewrite/rewrite-groovy/src/test/java/org/openrewrite/groovy/GroovyTypeMappingTest.java:23: error: JavaTypeMappingTest is not public in org.openrewrite.java; cannot be accessed from outside package
import org.openrewrite.java.JavaTypeMappingTest;
                           ^
/home/tim/Documents/workspace/rewrite/rewrite-groovy/src/test/java/org/openrewrite/groovy/GroovyTypeMappingTest.java:34: error: JavaTypeMappingTest is not public in org.openrewrite.java; cannot be accessed from outside package
class GroovyTypeMappingTest implements JavaTypeMappingTest {
                                       ^
warning: unknown enum constant When.MAYBE
  reason: class file for javax.annotation.meta.When not found
4 errors
4 warnings

Might be fairly niche; as such I would not consider it high priority for now.

[timtebeek]

Also occurs for Java classes that are extended: openrewrite/rewrite-kubernetes#41

[timtebeek]

And also for method access in GradleWrapperTest: openrewrite/rewrite#2948

[mkonietzka]

Reducing visibilty also breaks constructs like

import com.example.SomeGatewayIT;

import org.junit.platform.suite.api.SelectClasses;
import org.junit.platform.suite.api.Suite;

@Suite
@SelectClasses({SomeGatewayIT.class})
public class BranchITSuite {
}

SomeGatewayIT cannot be referenced when it becomes non public.

[timo-abele]

I think I found another one, someone called their @AfterEach annotated method "finalize", (unwittingly?) overriding Object.finalize. As overriding must not reduce visibility, this breaks the build.

[mbruggmann]

We ran into this as well. Here is a testcase to reproduce it:

  @Test
  void baseclassForTestsNeedsToStayPublic() {
    rewriteRun(
        spec -> spec.recipe(new TestsShouldNotBePublic(true)),
        java(
            // base class for tests should stay public
            """
            package com.hello;

            import org.junit.jupiter.api.BeforeEach;

            public class MyTestBase {
              @BeforeEach
              void setUp() {
              }
            }
            """),
        java(
            // test class extends base class from another package
            """
            package com.world;

            import com.hello.MyTestBase;
            import org.junit.jupiter.api.Test;

            class MyTest extends MyTestBase {
              @Test
              void isWorking() {
              }
            }
            """));
  }

[timtebeek]

Thanks for the helpful runnable test; I've created this PR which addresses the issue and welcome a review

• Do not reduce visibility of extended classes in TestsShouldNotBePublic #524

[mbruggmann]

@timtebeek Awesome, that looks like a good fix, thanks for implementing it 🎉