From 29b8483fd6e80116b417ca883361ab11a6be47bc Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Tue, 16 May 2023 12:10:13 -0700 Subject: [PATCH] Use test fixtures instead of sourceSets.test.output This avoids the (often missing) evaluationDependsOn and fixes using results from other projects without propagating those through Configuration. It also reduces the number of useless classes pulled in by down-stream tests, reducing the probability of rebuilds. The expectation of fixtures is they help testing down-stream code that use the classes in main. That applies to all the classes here except for FakeClock and StaticTestingClassLoader. It would also apply to many internal classes in grpc-testing, but let's consider cleaning that up future work. --- alts/build.gradle | 4 +--- api/build.gradle | 11 ++++++++--- .../java/io/grpc/ForwardingTestUtil.java | 0 .../java/io/grpc/IntegerMarshaller.java | 0 .../grpc/ManagedChannelRegistryAccessor.java | 0 .../java/io/grpc/StringMarshaller.java | 0 authz/build.gradle | 2 +- binder/build.gradle | 18 ++---------------- census/build.gradle | 8 +++----- context/build.gradle | 8 ++++++++ .../java/io/grpc/StaticTestingClassLoader.java | 0 .../java/io/grpc/testing/DeadlineSubject.java | 0 core/build.gradle | 16 +++++++++++----- .../grpc/internal/AbstractTransportTest.java | 0 .../java/io/grpc/internal/FakeClock.java | 0 .../internal/NoopClientStreamListener.java | 0 .../grpc/internal/ReadableBufferTestBase.java | 0 .../java/io/grpc/internal/TestUtils.java | 0 .../WritableBufferAllocatorTestBase.java | 0 .../grpc/internal/WritableBufferTestBase.java | 0 gcp-observability/build.gradle | 2 +- googleapis/build.gradle | 2 +- grpclb/build.gradle | 4 +--- interop-testing/build.gradle | 10 +++------- istio-interop-testing/build.gradle | 8 +++----- netty/build.gradle | 6 ++---- okhttp/build.gradle | 6 ++---- rls/build.gradle | 4 +--- services/build.gradle | 4 +--- testing/build.gradle | 4 +--- xds/build.gradle | 4 +--- 31 files changed, 51 insertions(+), 70 deletions(-) rename api/src/{test => testFixtures}/java/io/grpc/ForwardingTestUtil.java (100%) rename api/src/{test => testFixtures}/java/io/grpc/IntegerMarshaller.java (100%) rename api/src/{test => testFixtures}/java/io/grpc/ManagedChannelRegistryAccessor.java (100%) rename api/src/{test => testFixtures}/java/io/grpc/StringMarshaller.java (100%) rename context/src/{test => testFixtures}/java/io/grpc/StaticTestingClassLoader.java (100%) rename context/src/{test => testFixtures}/java/io/grpc/testing/DeadlineSubject.java (100%) rename core/src/{test => testFixtures}/java/io/grpc/internal/AbstractTransportTest.java (100%) rename core/src/{test => testFixtures}/java/io/grpc/internal/FakeClock.java (100%) rename core/src/{test => testFixtures}/java/io/grpc/internal/NoopClientStreamListener.java (100%) rename core/src/{test => testFixtures}/java/io/grpc/internal/ReadableBufferTestBase.java (100%) rename core/src/{test => testFixtures}/java/io/grpc/internal/TestUtils.java (100%) rename core/src/{test => testFixtures}/java/io/grpc/internal/WritableBufferAllocatorTestBase.java (100%) rename core/src/{test => testFixtures}/java/io/grpc/internal/WritableBufferTestBase.java (100%) diff --git a/alts/build.gradle b/alts/build.gradle index 2b48403d91..82454bf903 100644 --- a/alts/build.gradle +++ b/alts/build.gradle @@ -9,8 +9,6 @@ plugins { description = "gRPC: ALTS" -evaluationDependsOn(project(':grpc-core').path) - dependencies { api project(':grpc-core') implementation project(':grpc-auth'), @@ -28,7 +26,7 @@ dependencies { shadow project(path: ':grpc-netty-shaded', configuration: 'shadow') testImplementation project(':grpc-testing'), - project(':grpc-core').sourceSets.test.output, + testFixtures(project(':grpc-core')), project(':grpc-testing-proto'), libraries.guava, libraries.junit, diff --git a/api/build.gradle b/api/build.gradle index 05cd80674c..e106cad6a4 100644 --- a/api/build.gradle +++ b/api/build.gradle @@ -1,5 +1,6 @@ plugins { id "java-library" + id "java-test-fixtures" id "maven-publish" id "me.champeau.jmh" @@ -8,15 +9,16 @@ plugins { description = 'gRPC: API' -evaluationDependsOn(project(':grpc-context').path) - dependencies { api project(':grpc-context'), libraries.jsr305, libraries.errorprone.annotations implementation libraries.guava - testImplementation project(':grpc-context').sourceSets.test.output, + testFixturesImplementation libraries.guava, + libraries.junit, + libraries.mockito.core + testImplementation testFixtures(project(':grpc-context')), project(':grpc-testing'), project(':grpc-grpclb') testImplementation (libraries.guava.testlib) { @@ -32,3 +34,6 @@ tasks.named("javadoc").configure { // We want io.grpc.Internal, but not io.grpc.Internal* exclude 'io/grpc/Internal?*.java' } + +components.java.withVariantsFromConfiguration(configurations.testFixturesApiElements) { skip() } +components.java.withVariantsFromConfiguration(configurations.testFixturesRuntimeElements) { skip() } diff --git a/api/src/test/java/io/grpc/ForwardingTestUtil.java b/api/src/testFixtures/java/io/grpc/ForwardingTestUtil.java similarity index 100% rename from api/src/test/java/io/grpc/ForwardingTestUtil.java rename to api/src/testFixtures/java/io/grpc/ForwardingTestUtil.java diff --git a/api/src/test/java/io/grpc/IntegerMarshaller.java b/api/src/testFixtures/java/io/grpc/IntegerMarshaller.java similarity index 100% rename from api/src/test/java/io/grpc/IntegerMarshaller.java rename to api/src/testFixtures/java/io/grpc/IntegerMarshaller.java diff --git a/api/src/test/java/io/grpc/ManagedChannelRegistryAccessor.java b/api/src/testFixtures/java/io/grpc/ManagedChannelRegistryAccessor.java similarity index 100% rename from api/src/test/java/io/grpc/ManagedChannelRegistryAccessor.java rename to api/src/testFixtures/java/io/grpc/ManagedChannelRegistryAccessor.java diff --git a/api/src/test/java/io/grpc/StringMarshaller.java b/api/src/testFixtures/java/io/grpc/StringMarshaller.java similarity index 100% rename from api/src/test/java/io/grpc/StringMarshaller.java rename to api/src/testFixtures/java/io/grpc/StringMarshaller.java diff --git a/authz/build.gradle b/authz/build.gradle index 2e6cfc11ec..1eb6c212d0 100644 --- a/authz/build.gradle +++ b/authz/build.gradle @@ -19,7 +19,7 @@ dependencies { testImplementation project(':grpc-testing'), project(':grpc-testing-proto'), - project(':grpc-core').sourceSets.test.output // for FakeClock + testFixtures(project(':grpc-core')) testImplementation (libraries.guava.testlib) { exclude group: 'junit', module: 'junit' } diff --git a/binder/build.gradle b/binder/build.gradle index 1ba46d9334..3751a67733 100644 --- a/binder/build.gradle +++ b/binder/build.gradle @@ -7,22 +7,6 @@ description = 'gRPC BinderChannel' android { namespace 'io.grpc.binder' - sourceSets { - test { - java { - srcDirs += "${projectDir}/../core/src/test/java/" - setIncludes(["io/grpc/internal/FakeClock.java", - "io/grpc/binder/**"]) - } - } - androidTest { - java { - srcDirs += "${projectDir}/../core/src/test/java/" - setIncludes(["io/grpc/internal/AbstractTransportTest.java", - "io/grpc/binder/**"]) - } - } - } compileSdkVersion 31 compileOptions { sourceCompatibility 1.8 @@ -72,6 +56,7 @@ dependencies { } testImplementation libraries.truth testImplementation project(':grpc-testing') + testImplementation testFixtures(project(':grpc-core')) androidTestAnnotationProcessor libraries.auto.value androidTestImplementation project(':grpc-testing') @@ -88,6 +73,7 @@ dependencies { androidTestImplementation (libraries.guava.testlib) { exclude group: 'junit', module: 'junit' } + androidTestImplementation testFixtures(project(':grpc-core')) } import net.ltgt.gradle.errorprone.CheckSeverity diff --git a/census/build.gradle b/census/build.gradle index 02b7e6395b..25948f17f6 100644 --- a/census/build.gradle +++ b/census/build.gradle @@ -7,17 +7,15 @@ plugins { description = 'gRPC: Census' -evaluationDependsOn(project(':grpc-api').path) - dependencies { api project(':grpc-api') implementation libraries.guava, libraries.opencensus.api, libraries.opencensus.contrib.grpc.metrics - testImplementation project(':grpc-api').sourceSets.test.output, - project(':grpc-context').sourceSets.test.output, - project(':grpc-core').sourceSets.test.output, + testImplementation testFixtures(project(':grpc-api')), + testFixtures(project(':grpc-context')), + testFixtures(project(':grpc-core')), project(':grpc-testing'), libraries.opencensus.impl diff --git a/context/build.gradle b/context/build.gradle index 5815374aeb..8e02271bff 100644 --- a/context/build.gradle +++ b/context/build.gradle @@ -1,5 +1,6 @@ plugins { id "java" + id "java-test-fixtures" id "maven-publish" id "me.champeau.gradle.japicmp" @@ -13,6 +14,10 @@ sourceCompatibility = 1.7 targetCompatibility = 1.7 dependencies { + testFixturesApi libraries.truth + // Explicitly choose the guava version to stay Java 7-compatible. + testFixturesImplementation 'com.google.guava:guava:30.1.1-android' + testFixturesImplementation libraries.jsr305 testImplementation libraries.jsr305 // Explicitly choose the guava version to stay Java 7-compatible. The rest of gRPC can move // forward to Java 8-requiring versions. This is also only used for testing, so is unlikely to @@ -23,3 +28,6 @@ dependencies { signature "org.codehaus.mojo.signature:java17:1.0@signature" signature "net.sf.androidscents.signature:android-api-level-14:4.0_r4@signature" } + +components.java.withVariantsFromConfiguration(configurations.testFixturesApiElements) { skip() } +components.java.withVariantsFromConfiguration(configurations.testFixturesRuntimeElements) { skip() } diff --git a/context/src/test/java/io/grpc/StaticTestingClassLoader.java b/context/src/testFixtures/java/io/grpc/StaticTestingClassLoader.java similarity index 100% rename from context/src/test/java/io/grpc/StaticTestingClassLoader.java rename to context/src/testFixtures/java/io/grpc/StaticTestingClassLoader.java diff --git a/context/src/test/java/io/grpc/testing/DeadlineSubject.java b/context/src/testFixtures/java/io/grpc/testing/DeadlineSubject.java similarity index 100% rename from context/src/test/java/io/grpc/testing/DeadlineSubject.java rename to context/src/testFixtures/java/io/grpc/testing/DeadlineSubject.java diff --git a/core/build.gradle b/core/build.gradle index 3e34037136..f97c80b753 100644 --- a/core/build.gradle +++ b/core/build.gradle @@ -6,6 +6,7 @@ buildscript { plugins { id "java-library" + id "java-test-fixtures" id "maven-publish" id "me.champeau.gradle.japicmp" @@ -19,9 +20,6 @@ import com.google.common.primitives.Bytes; description = 'gRPC: Core' -evaluationDependsOn(project(':grpc-context').path) -evaluationDependsOn(project(':grpc-api').path) - dependencies { api project(':grpc-api') implementation libraries.gson, @@ -30,8 +28,13 @@ dependencies { libraries.errorprone.annotations, libraries.guava, libraries.perfmark.api - testImplementation project(':grpc-context').sourceSets.test.output, - project(':grpc-api').sourceSets.test.output, + testFixturesApi libraries.junit + testFixturesImplementation libraries.guava, + libraries.mockito.core, + libraries.truth, + project(':grpc-testing') + testImplementation testFixtures(project(':grpc-context')), + testFixtures(project(':grpc-api')), project(':grpc-testing'), project(':grpc-grpclb') testImplementation (libraries.guava.testlib) { @@ -61,6 +64,9 @@ animalsniffer { ] } +components.java.withVariantsFromConfiguration(configurations.testFixturesApiElements) { skip() } +components.java.withVariantsFromConfiguration(configurations.testFixturesRuntimeElements) { skip() } + import net.ltgt.gradle.errorprone.CheckSeverity def replaceBytes(byte[] haystack, byte[] needle, byte[] replacement) { diff --git a/core/src/test/java/io/grpc/internal/AbstractTransportTest.java b/core/src/testFixtures/java/io/grpc/internal/AbstractTransportTest.java similarity index 100% rename from core/src/test/java/io/grpc/internal/AbstractTransportTest.java rename to core/src/testFixtures/java/io/grpc/internal/AbstractTransportTest.java diff --git a/core/src/test/java/io/grpc/internal/FakeClock.java b/core/src/testFixtures/java/io/grpc/internal/FakeClock.java similarity index 100% rename from core/src/test/java/io/grpc/internal/FakeClock.java rename to core/src/testFixtures/java/io/grpc/internal/FakeClock.java diff --git a/core/src/test/java/io/grpc/internal/NoopClientStreamListener.java b/core/src/testFixtures/java/io/grpc/internal/NoopClientStreamListener.java similarity index 100% rename from core/src/test/java/io/grpc/internal/NoopClientStreamListener.java rename to core/src/testFixtures/java/io/grpc/internal/NoopClientStreamListener.java diff --git a/core/src/test/java/io/grpc/internal/ReadableBufferTestBase.java b/core/src/testFixtures/java/io/grpc/internal/ReadableBufferTestBase.java similarity index 100% rename from core/src/test/java/io/grpc/internal/ReadableBufferTestBase.java rename to core/src/testFixtures/java/io/grpc/internal/ReadableBufferTestBase.java diff --git a/core/src/test/java/io/grpc/internal/TestUtils.java b/core/src/testFixtures/java/io/grpc/internal/TestUtils.java similarity index 100% rename from core/src/test/java/io/grpc/internal/TestUtils.java rename to core/src/testFixtures/java/io/grpc/internal/TestUtils.java diff --git a/core/src/test/java/io/grpc/internal/WritableBufferAllocatorTestBase.java b/core/src/testFixtures/java/io/grpc/internal/WritableBufferAllocatorTestBase.java similarity index 100% rename from core/src/test/java/io/grpc/internal/WritableBufferAllocatorTestBase.java rename to core/src/testFixtures/java/io/grpc/internal/WritableBufferAllocatorTestBase.java diff --git a/core/src/test/java/io/grpc/internal/WritableBufferTestBase.java b/core/src/testFixtures/java/io/grpc/internal/WritableBufferTestBase.java similarity index 100% rename from core/src/test/java/io/grpc/internal/WritableBufferTestBase.java rename to core/src/testFixtures/java/io/grpc/internal/WritableBufferTestBase.java diff --git a/gcp-observability/build.gradle b/gcp-observability/build.gradle index 265c0d682f..48312d7fbc 100644 --- a/gcp-observability/build.gradle +++ b/gcp-observability/build.gradle @@ -62,7 +62,7 @@ dependencies { ('io.opencensus:opencensus-api:0.31.1'), ('com.google.guava:guava:31.1-jre') - testImplementation project(':grpc-context').sourceSets.test.output, + testImplementation testFixtures(project(':grpc-context')), project(':grpc-testing'), project(':grpc-testing-proto') testImplementation (libraries.guava.testlib) { diff --git a/googleapis/build.gradle b/googleapis/build.gradle index 540011986c..72f5b97940 100644 --- a/googleapis/build.gradle +++ b/googleapis/build.gradle @@ -13,7 +13,7 @@ dependencies { project(':grpc-core'), project(path: ':grpc-xds', configuration: 'shadow'), libraries.guava.jre // JRE required by transitive protobuf-java-util - testImplementation project(':grpc-core').sourceSets.test.output + testImplementation testFixtures(project(':grpc-core')) signature libraries.signature.java } diff --git a/grpclb/build.gradle b/grpclb/build.gradle index 6fd64c8429..7aa2d62b0f 100644 --- a/grpclb/build.gradle +++ b/grpclb/build.gradle @@ -9,8 +9,6 @@ plugins { description = "gRPC: GRPCLB LoadBalancer plugin" -evaluationDependsOn(project(':grpc-core').path) - dependencies { implementation project(':grpc-core'), project(':grpc-protobuf'), @@ -21,7 +19,7 @@ dependencies { runtimeOnly libraries.errorprone.annotations compileOnly libraries.javax.annotation testImplementation libraries.truth, - project(':grpc-core').sourceSets.test.output + testFixtures(project(':grpc-core')) signature libraries.signature.java } diff --git a/interop-testing/build.gradle b/interop-testing/build.gradle index ad032d7399..96d4ca3614 100644 --- a/interop-testing/build.gradle +++ b/interop-testing/build.gradle @@ -13,10 +13,6 @@ configurations { alpnagent } -evaluationDependsOn(project(':grpc-core').path) -evaluationDependsOn(project(':grpc-context').path) -evaluationDependsOn(project(':grpc-api').path) - dependencies { implementation project(path: ':grpc-alts', configuration: 'shadow'), project(':grpc-auth'), @@ -49,9 +45,9 @@ dependencies { libraries.netty.tcnative.classes, project(':grpc-grpclb'), project(':grpc-rls') - testImplementation project(':grpc-context').sourceSets.test.output, - project(':grpc-api').sourceSets.test.output, - project(':grpc-core').sourceSets.test.output, + testImplementation testFixtures(project(':grpc-context')), + testFixtures(project(':grpc-api')), + testFixtures(project(':grpc-core')), libraries.mockito.core, libraries.okhttp alpnagent libraries.jetty.alpn.agent diff --git a/istio-interop-testing/build.gradle b/istio-interop-testing/build.gradle index 7ba342c688..b4495002af 100644 --- a/istio-interop-testing/build.gradle +++ b/istio-interop-testing/build.gradle @@ -13,8 +13,6 @@ configurations { alpnagent } -evaluationDependsOn(project(':grpc-context').path) - dependencies { implementation project(':grpc-core'), project(':grpc-netty'), @@ -28,9 +26,9 @@ dependencies { runtimeOnly libraries.netty.tcnative, libraries.netty.tcnative.classes - testImplementation project(':grpc-context').sourceSets.test.output, - project(':grpc-api').sourceSets.test.output, - project(':grpc-core').sourceSets.test.output, + testImplementation testFixtures(project(':grpc-context')), + testFixtures(project(':grpc-api')), + testFixtures(project(':grpc-core')), libraries.mockito.core, libraries.junit, libraries.truth diff --git a/netty/build.gradle b/netty/build.gradle index 16d3279528..0f22d27746 100644 --- a/netty/build.gradle +++ b/netty/build.gradle @@ -13,8 +13,6 @@ configurations { alpnagent } -evaluationDependsOn(project(':grpc-core').path) - dependencies { api project(':grpc-core'), libraries.netty.codec.http2 @@ -25,8 +23,8 @@ dependencies { libraries.netty.unix.common // Tests depend on base class defined by core module. - testImplementation project(':grpc-core').sourceSets.test.output, - project(':grpc-api').sourceSets.test.output, + testImplementation testFixtures(project(':grpc-core')), + testFixtures(project(':grpc-api')), project(':grpc-testing'), project(':grpc-testing-proto'), libraries.conscrypt, diff --git a/okhttp/build.gradle b/okhttp/build.gradle index 439abaa337..37d033350c 100644 --- a/okhttp/build.gradle +++ b/okhttp/build.gradle @@ -8,8 +8,6 @@ plugins { description = "gRPC: OkHttp" -evaluationDependsOn(project(':grpc-core').path) - dependencies { api project(':grpc-core') implementation libraries.okio, @@ -18,8 +16,8 @@ dependencies { // Make okhttp dependencies compile only compileOnly libraries.okhttp // Tests depend on base class defined by core module. - testImplementation project(':grpc-core').sourceSets.test.output, - project(':grpc-api').sourceSets.test.output, + testImplementation testFixtures(project(':grpc-core')), + testFixtures(project(':grpc-api')), project(':grpc-testing'), project(':grpc-testing-proto'), libraries.netty.codec.http2, diff --git a/rls/build.gradle b/rls/build.gradle index ddd8cb6587..453052addf 100644 --- a/rls/build.gradle +++ b/rls/build.gradle @@ -8,8 +8,6 @@ plugins { description = "gRPC: RouteLookupService Loadbalancing plugin" -evaluationDependsOn(project(':grpc-core').path) - dependencies { implementation project(':grpc-core'), project(':grpc-protobuf'), @@ -22,7 +20,7 @@ dependencies { project(':grpc-grpclb'), project(':grpc-testing'), project(':grpc-testing-proto'), - project(':grpc-core').sourceSets.test.output // for FakeClock + testFixtures(project(':grpc-core')) signature libraries.signature.java } diff --git a/services/build.gradle b/services/build.gradle index 4b445c2d1e..76a767dce8 100644 --- a/services/build.gradle +++ b/services/build.gradle @@ -16,8 +16,6 @@ tasks.named("compileJava").configure { ] } -evaluationDependsOn(project(':grpc-core').path) - dependencies { api project(':grpc-protobuf'), project(':grpc-stub'), @@ -30,7 +28,7 @@ dependencies { compileOnly libraries.javax.annotation testImplementation project(':grpc-testing'), libraries.netty.transport.epoll, // for DomainSocketAddress - project(':grpc-core').sourceSets.test.output // for FakeClock + testFixtures(project(':grpc-core')) testCompileOnly libraries.javax.annotation signature libraries.signature.java } diff --git a/testing/build.gradle b/testing/build.gradle index 35da61f11f..512388cd07 100644 --- a/testing/build.gradle +++ b/testing/build.gradle @@ -8,8 +8,6 @@ plugins { description = "gRPC: Testing" -evaluationDependsOn(project(':grpc-core').path) - dependencies { api project(':grpc-core'), project(':grpc-stub'), @@ -24,7 +22,7 @@ dependencies { } testImplementation project(':grpc-testing-proto'), - project(':grpc-core').sourceSets.test.output + testFixtures(project(':grpc-core')) signature libraries.signature.java signature libraries.signature.android diff --git a/xds/build.gradle b/xds/build.gradle index a33fa6e6d5..ade629f380 100644 --- a/xds/build.gradle +++ b/xds/build.gradle @@ -11,8 +11,6 @@ plugins { description = "gRPC: XDS plugin" -evaluationDependsOn(project(':grpc-core').path) - sourceSets { thirdparty { java { @@ -57,7 +55,7 @@ dependencies { def nettyDependency = implementation project(':grpc-netty') testImplementation project(':grpc-rls') - testImplementation project(':grpc-core').sourceSets.test.output + testImplementation testFixtures(project(':grpc-core')) annotationProcessor libraries.auto.value // At runtime use the epoll included in grpc-netty-shaded