Closes #3: Should not reinitialize Resources parameter for beforeAll …

…or for static method
asarkar · Aug 11, 2020 · 60a8a3c · 60a8a3c
2 parents c2e128d + d92255d
commit 60a8a3c
Show file tree

Hide file tree

Showing 9 changed files with 126 additions and 34 deletions.
diff --git a/build.gradle.kts b/build.gradle.kts
@@ -29,6 +29,7 @@ dependencies {
     implementation("org.junit.jupiter:junit-jupiter-api")
     implementation("io.grpc:grpc-api")
     testImplementation("org.junit.jupiter:junit-jupiter")
+    testImplementation("io.grpc:grpc-core")
     testImplementation("org.mockito:mockito-core:$mockitoVersion")
     testImplementation("org.mockito:mockito-junit-jupiter:$mockitoVersion")
     testImplementation("org.junit.platform:junit-platform-testkit:$jUnitTestkitVersion")

diff --git a/gradle.properties b/gradle.properties
@@ -10,7 +10,7 @@ ktlintVersion=9.3.0
 bintrayPluginVersion=1.8.5
 
 projectGroup=com.asarkar.grpc
-projectVersion=1.0.1
+projectVersion=1.0.2
 projectDescription=JUnit5 Extension that can automatically release gRPC resources at the end of the test
 licenseName=Apache-2.0
 licenseUrl=http://www.apache.org/licenses/LICENSE-2.0.txt

diff --git a/src/main/kotlin/com/asarkar/grpc/test/GrpcCleanupExtension.kt b/src/main/kotlin/com/asarkar/grpc/test/GrpcCleanupExtension.kt
@@ -1,5 +1,6 @@
 package com.asarkar.grpc.test
 
+import org.junit.jupiter.api.BeforeAll
 import org.junit.jupiter.api.TestInstance
 import org.junit.jupiter.api.extension.AfterAllCallback
 import org.junit.jupiter.api.extension.AfterEachCallback
@@ -29,7 +30,7 @@ import kotlin.reflect.KFunction1
  */
 class GrpcCleanupExtension :
     BeforeEachCallback, AfterEachCallback, ParameterResolver, BeforeAllCallback, AfterAllCallback {
-    private var resources: Resources? = null
+    private val resources = mutableMapOf<Boolean, MutableList<Resources>>()
     private var resourcesField: Field? = null
 
     override fun beforeEach(ctx: ExtensionContext) {
@@ -51,30 +52,34 @@ class GrpcCleanupExtension :
 
     override fun afterEach(ctx: ExtensionContext) {
         var successful = true
-        this.resources?.also {
+        this.resources[false]?.forEach {
             getCleanUpMethod(ctx)(it)
             successful = it.awaitReleased()
         }
 
         if (shouldAccessResourcesField(ctx)) {
             tryGet(ctx.requiredTestInstance)?.also {
                 getCleanUpMethod(ctx)(it)
-                if (!it.awaitReleased()) {
-                    if (!successful) throw PostconditionViolationException("$resources and $it couldn't be released")
-                    else throw PostconditionViolationException("$it couldn't be released")
-                }
+                successful = it.awaitReleased()
+
                 trySet(ctx.requiredTestInstance, null)
             }
         }
-        if (!successful) throw PostconditionViolationException("$resources couldn't be released")
+        if (!successful) throw PostconditionViolationException("One or more Resources couldn't be released")
     }
 
     override fun supportsParameter(parameterContext: ParameterContext, extensionContext: ExtensionContext): Boolean {
         return parameterContext.parameter.type == Resources::class.java
     }
 
-    override fun resolveParameter(parameterContext: ParameterContext, extensionContext: ExtensionContext): Any {
-        return Resources().also { resources = it }
+    override fun resolveParameter(parameterCtx: ParameterContext, extensionCtx: ExtensionContext): Any {
+        val once = !parameterCtx.target.isPresent ||
+            (
+                extensionCtx.testInstanceLifecycle.orElse(null) == TestInstance.Lifecycle.PER_CLASS &&
+                    parameterCtx.declaringExecutable.isAnnotationPresent(BeforeAll::class.java)
+                )
+
+        return Resources().also { resources.getOrPut(once, { mutableListOf() }).add(it) }
     }
 
     override fun beforeAll(ctx: ExtensionContext) {
@@ -94,10 +99,16 @@ class GrpcCleanupExtension :
     }
 
     override fun afterAll(ctx: ExtensionContext) {
+        var successful = true
         tryGet(ctx.testInstance.orElse(null))?.also {
             getCleanUpMethod(ctx)(it)
-            if (!it.awaitReleased()) throw PostconditionViolationException("$it couldn't be released")
+            successful = it.awaitReleased()
+        }
+        this.resources[true]?.forEach {
+            getCleanUpMethod(ctx)(it)
+            successful = it.awaitReleased()
         }
+        if (!successful) throw PostconditionViolationException("One or more Resources couldn't be released")
     }
 
     private fun shouldAccessResourcesField(ctx: ExtensionContext): Boolean {

diff --git a/src/test/kotlin/com/asarkar/grpc/test/ExampleTestCase2.kt b/src/test/kotlin/com/asarkar/grpc/test/ExampleTestCase2.kt
@@ -1,27 +1,37 @@
 package com.asarkar.grpc.test
 
+import io.grpc.inprocess.InProcessChannelBuilder
+import org.assertj.core.api.Assertions.assertThat
 import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.extension.ExtendWith
 import org.mockito.Mockito
+import kotlin.random.Random
 
 @ExtendWith(GrpcCleanupExtension::class)
 class ExampleTestCase2 {
     private lateinit var resources: Resources
-    private val setOfResources: Set<Resources?> = mutableSetOf()
+    private val setOfResources: MutableSet<Resources?> = mutableSetOf()
     private val set = Mockito.spy(setOfResources)
 
     @Test
     fun test1() {
-        setOfResources.plus(resources)
+        resources.register(randomChannel())
+        assertThat(setOfResources.add(resources)).isTrue()
     }
 
     @Test
     fun test2() {
-        setOfResources.plus(resources)
+        resources.register(randomChannel())
+        assertThat(setOfResources.add(resources)).isTrue()
     }
 
     @Test
     fun test3() {
-        setOfResources.plus(resources)
+        resources.register(randomChannel())
+        assertThat(setOfResources.add(resources)).isTrue()
     }
+
+    private fun randomChannel() = InProcessChannelBuilder
+        .forName((1..4).map { ('a' + Random.nextInt(0, 26)) }.joinToString(""))
+        .build()
 }
diff --git a/src/test/kotlin/com/asarkar/grpc/test/ExampleTestCase3.kt b/src/test/kotlin/com/asarkar/grpc/test/ExampleTestCase3.kt
@@ -9,16 +9,16 @@ import org.mockito.Mockito
 @TestInstance(TestInstance.Lifecycle.PER_CLASS)
 class ExampleTestCase3 {
     private var resources: Resources? = null
-    private val setOfResources: Set<Resources?> = mutableSetOf()
+    private val setOfResources: MutableSet<Resources?> = mutableSetOf()
     private val set = Mockito.spy(setOfResources)
 
     @Test
     fun test1() {
-        setOfResources.plus(resources)
+        setOfResources.add(resources)
     }
 
     @Test
     fun test2() {
-        setOfResources.plus(resources)
+        setOfResources.add(resources)
     }
 }
diff --git a/src/test/kotlin/com/asarkar/grpc/test/ExampleTestCase5.kt b/src/test/kotlin/com/asarkar/grpc/test/ExampleTestCase5.kt
@@ -1,5 +1,6 @@
 package com.asarkar.grpc.test
 
+import org.assertj.core.api.Assertions.assertThat
 import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.extension.ExtendWith
 import org.mockito.Mockito
@@ -10,11 +11,11 @@ open class BaseTestCase {
 
 @ExtendWith(GrpcCleanupExtension::class)
 class ExampleTestCase5 : BaseTestCase() {
-    private val setOfResources: Set<Resources?> = mutableSetOf()
+    private val setOfResources: MutableSet<Resources?> = mutableSetOf()
     private val set = Mockito.spy(setOfResources)
 
     @Test
     fun testInheritResource() {
-        setOfResources.plus(super.resources)
+        assertThat(setOfResources.add(super.resources)).isTrue()
     }
 }
diff --git a/src/test/kotlin/com/asarkar/grpc/test/ExampleTestCase7.kt b/src/test/kotlin/com/asarkar/grpc/test/ExampleTestCase7.kt
@@ -9,16 +9,16 @@ import org.mockito.Mockito
 @TestInstance(TestInstance.Lifecycle.PER_CLASS)
 class ExampleTestCase7 {
     private var resources = Resources()
-    private val setOfResources: Set<Resources?> = mutableSetOf()
+    private val setOfResources: MutableSet<Resources?> = mutableSetOf()
     private val set = Mockito.spy(setOfResources)
 
     @Test
     fun test1() {
-        setOfResources.plus(resources)
+        setOfResources.add(resources)
     }
 
     @Test
     fun test2() {
-        setOfResources.plus(resources)
+        setOfResources.add(resources)
     }
 }
diff --git a/src/test/kotlin/com/asarkar/grpc/test/ExampleTestCase9.kt b/src/test/kotlin/com/asarkar/grpc/test/ExampleTestCase9.kt
@@ -0,0 +1,46 @@
+package com.asarkar.grpc.test
+
+import io.grpc.ManagedChannel
+import io.grpc.inprocess.InProcessChannelBuilder
+import org.assertj.core.api.Assertions.assertThat
+import org.junit.jupiter.api.BeforeAll
+import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.TestInstance
+import org.junit.jupiter.api.extension.ExtendWith
+import org.mockito.Mockito
+import kotlin.random.Random
+
+@ExtendWith(GrpcCleanupExtension::class)
+@TestInstance(TestInstance.Lifecycle.PER_CLASS)
+class ExampleTestCase9 {
+    private lateinit var channel: ManagedChannel
+    private val setOfResources: MutableSet<Resources?> = mutableSetOf()
+    private val set = Mockito.spy(setOfResources)
+
+    @BeforeAll
+    fun beforeAll(resources: Resources) {
+        channel = randomChannel()
+        resources.register(channel)
+        assertThat(setOfResources.add(resources)).isTrue()
+    }
+
+    @Test
+    fun test1(resources: Resources) {
+        assertThat(channel.isShutdown).isFalse()
+        assertThat(channel.isTerminated).isFalse()
+        resources.register(randomChannel())
+        assertThat(setOfResources.add(resources)).isTrue()
+    }
+
+    @Test
+    fun test2(resources: Resources) {
+        assertThat(channel.isShutdown).isFalse()
+        assertThat(channel.isTerminated).isFalse()
+        resources.register(randomChannel())
+        assertThat(setOfResources.add(resources)).isTrue()
+    }
+
+    private fun randomChannel() = InProcessChannelBuilder
+        .forName((1..4).map { ('a' + Random.nextInt(0, 26)) }.joinToString(""))
+        .build()
+}
diff --git a/src/test/kotlin/com/asarkar/grpc/test/GrpcCleanupExtensionIntegrationTests.kt b/src/test/kotlin/com/asarkar/grpc/test/GrpcCleanupExtensionIntegrationTests.kt
@@ -154,7 +154,7 @@ class GrpcCleanupExtensionIntegrationTests {
     }
 
     @Test
-    fun testReinitializeInstanceVariableForEachTest() {
+    fun testReinitializeInstanceField() {
         val mocks = mutableListOf<Any>()
         val listener = MockCreationListener { mock, _ -> mocks.add(mock) }
         Mockito.framework().addListener(listener)
@@ -171,11 +171,11 @@ class GrpcCleanupExtensionIntegrationTests {
             )
 
         assertThat(mocks).allMatch { it is Set<*> }
-        assertThat(mocks.map { it as Set<*> }.toSet()).hasSize(3)
+        assertThat(mocks.flatMap { it as Set<*> }.toSet()).hasSize(3)
     }
 
     @Test
-    fun testInitializeInstanceVariableOnlyOnce() {
+    fun testInitializeInstanceFieldOnlyOnce() {
         val mocks = mutableListOf<Any>()
         val listener = MockCreationListener { mock, _ -> mocks.add(mock) }
         Mockito.framework().addListener(listener)
@@ -192,11 +192,11 @@ class GrpcCleanupExtensionIntegrationTests {
             )
 
         assertThat(mocks).allMatch { it is Set<*> }
-        assertThat(mocks.map { it as Set<*> }.toSet()).hasSize(1)
+        assertThat(mocks.flatMap { it as Set<*> }.toSet()).hasSize(1)
     }
 
     @Test
-    fun testInitializeStaticVariableOnlyOnce() {
+    fun testInitializeStaticFieldOnlyOnce() {
         EngineTestKit.engine("junit-jupiter")
             .selectors(
                 selectClass(ExampleTestCase4::class.java)
@@ -230,11 +230,11 @@ class GrpcCleanupExtensionIntegrationTests {
             )
 
         assertThat(mocks).allMatch { it is Set<*> }
-        assertThat(mocks.map { it as Set<*> }.toSet()).hasSize(1)
+        assertThat(mocks.flatMap { it as Set<*> }.toSet()).hasSize(1)
     }
 
     @Test
-    fun testMultipleParameters() {
+    fun testMultipleParametersError() {
         EngineTestKit.engine("junit-jupiter")
             .selectors(
                 selectMethod(
@@ -254,7 +254,7 @@ class GrpcCleanupExtensionIntegrationTests {
     }
 
     @Test
-    fun testMultipleInstances() {
+    fun testMultipleInstanceFieldsError() {
         EngineTestKit.engine("junit-jupiter")
             .selectors(
                 selectClass(
@@ -271,7 +271,7 @@ class GrpcCleanupExtensionIntegrationTests {
     }
 
     @Test
-    fun testAlreadyInitialized() {
+    fun testAlreadyInitializedField() {
         val mocks = mutableListOf<Any>()
         val listener = MockCreationListener { mock, _ -> mocks.add(mock) }
         Mockito.framework().addListener(listener)
@@ -288,11 +288,11 @@ class GrpcCleanupExtensionIntegrationTests {
             )
 
         assertThat(mocks).allMatch { it is Set<*> }
-        assertThat(mocks.map { it as Set<*> }.toSet()).hasSize(1)
+        assertThat(mocks.flatMap { it as Set<*> }.toSet()).hasSize(1)
     }
 
     @Test
-    fun testCannotRetainAlreadyInitialized() {
+    fun testAlreadyInitializedFieldError() {
         EngineTestKit.engine("junit-jupiter")
             .selectors(
                 selectClass(ExampleTestCase8::class.java)
@@ -305,4 +305,27 @@ class GrpcCleanupExtensionIntegrationTests {
                 event(finishedWithFailure(instanceOf(PreconditionViolationException::class.java)))
             )
     }
+
+    @Test
+    fun testInitializeParameterOnlyOnce() {
+        val mocks = mutableListOf<Any>()
+        val listener = MockCreationListener { mock, _ -> mocks.add(mock) }
+        Mockito.framework().addListener(listener)
+        EngineTestKit.engine("junit-jupiter")
+            .selectors(
+                selectClass(ExampleTestCase9::class.java)
+            )
+            .execute()
+            .testEvents()
+            .assertThatEvents()
+            .haveExactly(
+                2,
+                event(
+                    finishedSuccessfully()
+                )
+            )
+
+        assertThat(mocks).allMatch { it is Set<*> }
+        assertThat(mocks.flatMap { it as Set<*> }.toSet()).hasSize(3)
+    }
 }