github-workflows-kt icon indicating copy to clipboard operation
github-workflows-kt copied to clipboard

Add tests to verify the documentation examples are valid

Open aSemy opened this issue 2 years ago • 6 comments

At the moment one of the examples in the docs doesn't compile

https://github.com/krzema12/github-actions-kotlin-dsl/blob/32576f647c533f087eed07a93564a56f418d78cd/docs/user-guide/getting_started.md?plain=1#L35-L45

job(name = "test_job", runsOn = UbuntuLatest) {
// Error: No value passed for parameter 'id'

I'd like to use Knit to solve the problem of the docs getting out of sync.

Recently I've been using Knit to verify that the documentation examples are correct. It provides a simple way of testing any source code that's in the docs. The tests are then run as part of the usual test suite. If the API changes, then the tests in the docs fail.

A good example project that uses Knit is Kotlinx Serialization: https://github.com/Kotlin/kotlinx.serialization/blob/master/docs/basic-serialization.md#json-encoding

I'll make a quite demo PR (I'll base it off the 'update Gradle' PR, but it's not necessary - either can be done independently )

aSemy avatar Apr 17 '22 10:04 aSemy

I've attempted this but it's much more difficult than I anticipated, because Knit isn't really set up for running .kts files.

Here's my progress so far: https://github.com/aSemy/github-actions-kotlin-dsl/pull/1/. If I evaluate the script in a main function, it works as expected...

fun main() {
    val file = Path("./docs/code/example/example-guide-getting-started-01.main.kts").toFile()

    val res = evalFile(file)

    res.reports.forEach {
        if (it.severity > ScriptDiagnostic.Severity.DEBUG) {
            println(it.prettyPrint())
        }
    }
}

But when the test executes, it fails:


java.lang.NoSuchMethodError: 'void org.jetbrains.kotlin.cli.jvm.compiler.KotlinCliJavaFileManagerImpl.<init>(org.jetbrains.kotlin.com.intellij.psi.PsiManager)'
	at org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreProjectEnvironment.createCoreFileManager(KotlinCoreProjectEnvironment.kt:28)
	at org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreProjectEnvironment.createCoreFileManager(KotlinCoreProjectEnvironment.kt:24)
	at org.jetbrains.kotlin.com.intellij.core.JavaCoreProjectEnvironment.<init>(JavaCoreProjectEnvironment.java:51)
	at org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreProjectEnvironment.<init>(KotlinCoreProjectEnvironment.kt:27)
	at org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment$ProjectEnvironment.<init>(KotlinCoreEnvironment.kt:122)
	at org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment$Companion.createForProduction(KotlinCoreEnvironment.kt:474)
	at org.jetbrains.kotlin.scripting.compiler.plugin.impl.CompilationContextKt.createIsolatedCompilationContext(compilationContext.kt:74)
	at org.jetbrains.kotlin.scripting.compiler.plugin.impl.ScriptJvmCompilerIsolated$compile$1$1.invoke(ScriptJvmCompilerImpls.kt:50)
	at org.jetbrains.kotlin.scripting.compiler.plugin.impl.ScriptJvmCompilerIsolated$compile$1$1.invoke(ScriptJvmCompilerImpls.kt:45)
	at org.jetbrains.kotlin.scripting.compiler.plugin.impl.ScriptJvmCompilerImplsKt.withScriptCompilationCache(ScriptJvmCompilerImpls.kt:99)
	at org.jetbrains.kotlin.scripting.compiler.plugin.impl.ScriptJvmCompilerImplsKt.access$withScriptCompilationCache(ScriptJvmCompilerImpls.kt:1)
	at org.jetbrains.kotlin.scripting.compiler.plugin.impl.ScriptJvmCompilerIsolated.compile(ScriptJvmCompilerImpls.kt:45)
	at kotlin.script.experimental.jvmhost.JvmScriptCompiler.invoke$suspendImpl(jvmScriptCompilation.kt:30)
	at kotlin.script.experimental.jvmhost.JvmScriptCompiler.invoke(jvmScriptCompilation.kt)
	at kotlin.script.experimental.host.BasicScriptingHost$eval$1.invokeSuspend(BasicScriptingHost.kt:47)
	at kotlin.script.experimental.host.BasicScriptingHost$eval$1.invoke(BasicScriptingHost.kt)
	at kotlin.script.experimental.host.BasicScriptingHost$eval$1.invoke(BasicScriptingHost.kt)
	at kotlin.script.experimental.host.BasicScriptingHost$runInCoroutineContext$1.invokeSuspend(BasicScriptingHost.kt:36)
	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
	at kotlin.coroutines.ContinuationKt.startCoroutine(Continuation.kt:115)
	at kotlin.script.experimental.impl.RunSuspendKt.internalScriptingRunSuspend(runSuspend.kt:19)
	at kotlin.script.experimental.host.BasicScriptingHost.runInCoroutineContext(BasicScriptingHost.kt:36)
	at kotlin.script.experimental.host.BasicScriptingHost.eval(BasicScriptingHost.kt:46)
	at util.ScriptingHostKt.evalFile(scriptingHost.kt:86)

aSemy avatar Apr 17 '22 21:04 aSemy

Hi @aSemy, thanks for this idea! Originally I planned to do it within #30, but resolved it since the ROI didn't seem high back then. Now when the issue appeared and you proposed a first-party library from JetBrains that would help us achieve it, I'm totally for giving it a shot.

Regarding missing support for KTS files, as a workaround I'd suggest treating the examples as pure Kotlin code, the scripting context is not absolutely needed to be used while running these tests. It won't be ideal as some scripting-specific features won't be tested and reflected, but it's better than nothing we have now. Maybe this approach will lead you somewhere. I also encourage you to talk to the team that maintains Knit to learn what's our chances to get proper KTS support.

Fixing the immediate issue with the docs: I extracted it to #198.

krzema12 avatar Apr 19 '22 10:04 krzema12

I've found the issue with running the scripts, it's something to do with kotlinx-knit https://github.com/Kotlin/kotlinx-knit/issues/41

aSemy avatar Apr 23 '22 13:04 aSemy

Number of times the docs weren't updated to reflect code changes:

  • several times in the past, no details
  • the reason of this ticket
  • breaking change: targetFile -> targetFileName: https://github.com/krzema12/github-actions-kotlin-dsl/commit/9245ae5dcdcbd590c02106bdcb1c566e71825e98
  • https://github.com/krzema12/github-workflows-kt/commit/a5b6242b65bae0199f59ebe24f61a4cecd504b82

krzema12 avatar Jun 14 '22 07:06 krzema12

@jmfayard do you know if it's possible with Mkdocs to have source code in a separate file and mark which pieces should not be included in the rendered docs (context)? Then this whole problem becomes simple: let's extract code to separate files, with context so that it compiles, and then let's compile it with Kotlin compiler in GitHub workflow.

krzema12 avatar Jul 02 '22 09:07 krzema12

@krzema12 mkdocs has a list of plugins here https://github.com/mkdocs/mkdocs/wiki/MkDocs-Plugins

https://github.com/rnorth/mkdocs-codeinclude-plugin seems to be what you are looking for

<rant> It's one of the things that everyone needs but is not in markdown. Like for table of contents. So everyone is reinventing the wheel in an incompatible way. Asciidoc does this by default https://docs.asciidoctor.org/asciidoc/latest/directives/include-tagged-regions/ </rant>

jmfayard avatar Jul 02 '22 09:07 jmfayard

@aSemy do you have capacity to try onboarding Knit once again, and just skipping all snippets that are coupled to scripting? I think there's just one such snippet, in "Getting started".

krzema12 avatar Feb 16 '23 05:02 krzema12

The issue he referenced is not solved yet. And imho extracting code from docs and trying to execute it is not the right way. I personally prefer the other way around, including snippets from source files into docs. That way they could for example also be target of refactorings and usage searches, can be run as part of normal tests and so on.

The plugin @jmfayard mentioned actually looks a bit ugly to me usage-wise, does not work properly, and is very inflexible (e.g. no way to use hl_lines.

But to the rescue, the pymdown extensions that are included in mkdocs-material have a proper snippets extension which works quite well except for built-in unindentation.

Vampire avatar Feb 16 '23 09:02 Vampire

@Vampire if we ignore one code snippet in the docs, I think Knit can be used even without having https://github.com/Kotlin/kotlinx-knit/issues/41 solved.

krzema12 avatar Feb 16 '23 09:02 krzema12

Well, I'm still not a big fan of it, due to the mentioned things. And additionally because it generates additional files you then check into VCS which I'm a bit allergic against. :-D Besides that I already halfway did the extraction to the built-in snippets extension.

Vampire avatar Feb 16 '23 09:02 Vampire