Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gradle Plugin: Expose finalizeCoroutines as Gradle task option #2759

Open
wants to merge 24 commits into
base: master
Choose a base branch
from

Conversation

aSemy
Copy link
Contributor

@aSemy aSemy commented Nov 28, 2022

  • Allows the finalizeCoroutines option to be configurable from Gradle tasks
  • fixes flakey tests in the Kotlinx Serialization and Kotlin Coroutines integration tests

This PR was triggered by an issue in #2740 (comment). Kotlinx Serialization and Kotlinx Coroutines requires finalizeCoroutines to be disabled, else Dokka generation fails

Example error: https://github.com/Kotlin/dokka/actions/runs/3567615789/jobs/5995523064#step:4:4032

 Caused by: java.util.concurrent.RejectedExecutionException: DefaultDispatcher was terminated

Currently the Kotlinx Serialization and Kotlin Coroutines integration tests are flakey because of this error.

But with this PR, the option can be configured:

tasks.withType<AbstractDokkaTask>().configureEach {
    finalizeCoroutines.set(false)
}

I've fixed the flakey tests by setting finalizeCoroutines to false with a Gradle init script.

Since I would like to see more idiomatic Gradle config #2700, the new code in this PR follows the Gradle API more closely than pre-existing code (for example, using abstract val for properties, and setting the default value using convention(), so getSafe() is not necessary).

This PR includes

@aSemy aSemy marked this pull request as draft November 30, 2022 16:30
@aSemy aSemy marked this pull request as ready for review November 30, 2022 18:50
@IgnatBeresnev IgnatBeresnev self-requested a review January 25, 2023 20:11
…' into feat/expose_finalizeCoroutines_in_gradle

# Conflicts:
#	runners/gradle-plugin/src/main/kotlin/org/jetbrains/dokka/gradle/AbstractDokkaTask.kt
@IgnatBeresnev
Copy link
Member

IgnatBeresnev commented Jan 29, 2023

An update on this: if we do start using Gradle workers from #2740, it looks like this configuration option won't be needed at all for the Gradle plugin users, can just set it to false by default.

Disabling it for coroutines and serialization does make the tests pass, but if you try to run Dokka with gradle workers on any other project - I think it'll also fail until you turn set finalizeCoroutines to false as well.

Let's wait until we get to #2740 though and decide and test it there

Note: it might still be relevant for the CLI runner, also needs to be tested. But it looks like it should be set to false by default if Gradle workers are used.

@IgnatBeresnev IgnatBeresnev added the runner: Gradle plugin An issue/PR related to Dokka's Gradle plugin label Jan 31, 2023
…sl_plugin

# Conflicts:
#	runners/gradle-plugin/src/main/kotlin/org/jetbrains/dokka/gradle/utils.kt
#	runners/gradle-plugin/src/test/kotlin/org/jetbrains/dokka/gradle/DokkaCollectorTaskTest.kt
#	runners/gradle-plugin/src/test/kotlin/org/jetbrains/dokka/gradle/DokkaConfigurationJsonTest.kt
#	runners/gradle-plugin/src/test/kotlin/org/jetbrains/dokka/gradle/DokkaConfigurationSerializableTest.kt
#	runners/gradle-plugin/src/test/kotlin/org/jetbrains/dokka/gradle/DokkaTaskTest.kt
#	runners/gradle-plugin/src/test/kotlin/org/jetbrains/dokka/gradle/GradleDokkaSourceSetBuilderTest.kt
#	runners/gradle-plugin/src/test/kotlin/org/jetbrains/dokka/gradle/KotlinDslDokkaTaskConfigurationTest.kt
…Coroutines_in_gradle

# Conflicts:
#	runners/gradle-plugin/src/test/kotlin/org/jetbrains/dokka/gradle/DokkaCollectorTaskTest.kt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
runner: Gradle plugin An issue/PR related to Dokka's Gradle plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants