line / centraldogma

Highly-available version-controlled service configuration repository based on Git, ZooKeeper and HTTP/2
https://line.github.io/centraldogma/
Apache License 2.0
598 stars 117 forks source link

`CentralDogmaExtension` after test listener not waiting server to stop #854

Open Dogacel opened 1 year ago

Dogacel commented 1 year ago

https://github.com/line/centraldogma/blob/69572e15427626a890c9a0870618d4c1908d1c72/testing/junit/src/main/java/com/linecorp/centraldogma/testing/junit/CentralDogmaExtension.java#L108-L117

I don't see a join() here. I am seeing a flaky test, when I try to use CentralDogmaExtension in two tests back-to-back, I am seeing a connection error (around 1 in 5 runs).

flagma.server.app.InitializerTest > app should initialize FAILED
    java.util.concurrent.CompletionException at CoroutineDebugging.kt:46
        Caused by: java.util.concurrent.CompletionException at CompletableFuture.java:332
            Caused by: com.linecorp.armeria.client.UnprocessedRequestException at UnprocessedRequestException.java:45
                Caused by: com.linecorp.armeria.client.endpoint.EndpointSelectionTimeoutException at EndpointSelectionTimeoutException.java:48
10423 [Thread-5] [DEBUG] c.l.armeria.client.ClientFactory -- Closing the default client factories

I feel like the join is forgotten here and this error can happen rarely.

minwoox commented 1 year ago

HI! The after method is called after all tests are executed, which means we don't need this extension anymore, so we didn't use join() intentionally because it will make the whole build slower. Could you provide your test code so that we can investigate what's going on?

Dogacel commented 1 year ago

HI! The after method is called after all tests are executed, which means we don't need this extension anymore, so we didn't use join() intentionally because it will make the whole build slower. Could you provide your test code so that we can investigate what's going on?

package flagma.server.app

import com.linecorp.centraldogma.client.CentralDogma
import com.linecorp.centraldogma.testing.junit.CentralDogmaExtension
import flagma.server.Config
import io.kotest.assertions.throwables.shouldNotThrowAny
import io.kotest.core.spec.style.FunSpec
import io.kotest.core.test.TestCaseOrder
import io.kotest.extensions.junit5.JUnitExtensionAdapter
import io.kotest.koin.KoinExtension
import io.kotest.matchers.collections.shouldBeEmpty
import io.kotest.matchers.collections.shouldContain
import io.kotest.matchers.collections.shouldExist
import org.koin.dsl.module
import org.koin.test.KoinTest

class InitializerTest : KoinTest, FunSpec({
    val centralDogmaExtension = CentralDogmaExtension()
    val koinTestExtension = KoinExtension(
        modules = listOf(
            module { single<CentralDogma> { centralDogmaExtension.client() } },
            Modules.repositoryModules,
        )
    )

    testOrder = TestCaseOrder.Sequential
    extensions(
        koinTestExtension,
        JUnitExtensionAdapter(centralDogmaExtension),
    )

    test("app should initialize") {
        // Dogma should be clean
        centralDogmaExtension.client()
            .listProjects().join()
            .shouldBeEmpty()

        Initializer.initializeProject(centralDogmaExtension.client())

        centralDogmaExtension.client()
            .listProjects().join()
            .shouldExist { it == Config.CentralDogma.PROJECT_NAME }
        centralDogmaExtension.client()
            .listRepositories(Config.CentralDogma.PROJECT_NAME).join()
            .keys.shouldContain(Config.CentralDogma.PROJECTS_REPOSITORY_NAME)
    }

    test("app should initialize if already initialized") {
        shouldNotThrowAny {
            Initializer.initializeProject(centralDogmaExtension.client())
        }
    }
})

Initializer.initializeProject basically calls listProjects, listRepositories and createProject with .join().

Maybe timeout for .join() is too slow? 3200ms for endpoint selection.

minwoox commented 1 year ago

Maybe timeout for .join() is too slow? 3200ms for endpoint selection.

This is a local test so I think it is enough. So the exception is raised in your test("app should initialize") {...} test when you call centralDogmaExtension.client() .listProjects()? One more question is you are using the latest version, right?

Dogacel commented 1 year ago

Should be the latest:

    testImplementation("com.linecorp.centraldogma:centraldogma-testing-junit:0.61.4")

Not sure where exactly the exception is realized, stack trace is messy. I will do some debug logging.

minwoox commented 1 year ago

Let us know if you find something. 👍 The exception can also be raised if we call Central Dogma API after closing the extension (by not closing the watcher or something else) but I'm not sure if it's the cause.