Skip to content

Cleaning up tests in scala3doc #10170

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

Merged
merged 5 commits into from
Nov 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions scala3doc-testcases/src/tests/complexNames.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package tests

package complexNames

abstract class A:
def ++(other: A): A
def +:(other: Int): A
def :+(other: Int): A

// scala3doc has problems with names in backticks
// def `multi word name`: Int
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably need a issue for it

cc @nicolasstucki

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what are we missing here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Print a name that needs backticks in backticks, I believe.

This makes sense to include in Tasty-Reflect, since the general concept is being able to print a name in a form that corresponds to how it was entered in source. I'm not certain what's the best way to do that, though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will need to introduce a Name abstraction in the reflection API. A String loses too much information about the name. We cannot distinguish semantic names from simple names and we cannot know if they were backticked. That is on my TODO list.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably for now we can just fix that in scala3doc: lampepfl/scala3doc#235 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kordyjan I'd rather avoid the code duplication, but I suppose that the regex for Scala identifiers that don't need a backtick is unlikely to change anytime soon. Can you wrap the code for detecting whether an identifier needs a backtick in a function whose name starts with hack, so we can grep for it easily? We have a bunch of functions like that in the doctool already.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, we won't have that info. The scaladoc will need to figure it out.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@abgruszecki: #10199 is ready.

// def `*** name with arbitrary chars ^%`: Int
// def `mischievous(param:Int)`(otherParam: Int): String
// def withMischievousParams(`param: String, param2`: String): String

def complexName_^*(param: String): A

def `completelyUnnecessaryBackticks`: Int //expected: def completelyUnnecessaryBackticks: Int
def `+++:`(other: Int): A //expected: def +++:(other: Int): A
def `:+++`(other: Int): A //expected: def :+++(other: Int): A

def `abc_^^_&&`: A //expected: def abc_^^_&&: A
def `abc_def`: A //expected: def abc_def: A
def `abc_def_++`: A //expected: def abc_def_++: A
// def `++_abc`: A
// def `abc_++_--`: A
46 changes: 30 additions & 16 deletions scala3doc/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,22 +72,36 @@ You can also find the result of building the same sites for latest `master` at:

### Testing

Most tests rely on comparing signatures (of classes, methods, objects etc.) extracted from the generated documentation
to signatures found in source files. Such tests are defined using [MultipleFileTest](src/test/scala/dotty/dokka/MultipleFileTest.scala) class
and its subtypes (such as [SingleFileTest](src/test/scala/dotty/dokka/SingleFileTest.scala))

WARNING: As the classes mentioned above are likely to evolve, the description below might easily get out of date.
In case of any discrepancies rely on the source files instead.

`MultipleFileTest` requires that you specify the names of the files used to extract signatures,
the names of directories containing corresponding TASTY files
and the kinds of signatures from source files (corresponding to keywords used to declare them like `def`, `class`, `object` etc.)
whose presence in the generated documentation will be checked (other signatures, when missing, will be ignored).
The mentioned source files should be located directly inside `src/main/scala/tests` directory
but the file names passed as parameters should contain neither this path prefix nor `.scala` suffix.
The TASTY folders are expected to be located in `target/${dottyVersion}/classes/tests` (after successful compilation of the project)
and similarly only their names relative to this path should be provided as tests' parameters.
For `SingleFileTest` the name of the source file and the TASTY folder are expected to be the same.
To implement integration tests that inspects state of the model on different stages of
documentation generation one can use [ScaladocTest](src/test/scala/dotty/dokka/ScaladocTest.scala#L15).
This class requires providing the name of the test and a the list of assertions.

Name of the test is used to extract symbols that should be included in given test run from
the TASTY files. All test data is located in [scala3doc-testcases module](../scala3doc-testcases/src).
It is compiled together with the rest of modules. This solves lots of potential problems with
invoking the compiler in a separate process such as mismatching versions. Every test is using
only symbols defined in the package with the same name as the test located inside top level `tests`
package (including subpackages). This restriction may be relaxed in the future.

Assertions of each test are defined as list of [Assertion enum](src/test/scala/dotty/dokka/ScaladocTest.scala#L63)
instances. Each of them represents a callback that is invoked in corresponding phase of
documentation generation. Those callback can inspect the model, log information and raise errors
using `reportError` method.

#### Signature tests

Some of the tests rely on comparing signatures (of classes, methods, objects etc.) extracted from
the generated documentation to signatures found in source files. Such tests are defined using
[SignatureTest](src/test/scala/dotty/dokka/SignatureTest.scala#L16) class, that is a subclass of
`ScaladocTest` and uses exactly tha same mechanism to find input symbols in the TASTY files.

Signature tests by default assume that sources that were used to generate used TASTY files are
located in the file with the same name as the name of the test. If this is not the case optional
parameter `sourceFiles` can be used to pass list of source file names (without `.scala` extension).

Those tests also requires specifying kinds of ignatures from source files (corresponding to
keywords used to declare them like `def`, `class`, `object` etc.) whose presence in the generated
documentation will be checked (other signatures, when missing, will be ignored).

By default it's expected that all signatures from the source files will be present in the documentation
but not vice versa (because the documentation can contain also inherited signatures).
Expand Down
126 changes: 0 additions & 126 deletions scala3doc/test/dotty/dokka/DottyTestRunner.scala

This file was deleted.

92 changes: 0 additions & 92 deletions scala3doc/test/dotty/dokka/MultipleFileTest.scala

This file was deleted.

95 changes: 95 additions & 0 deletions scala3doc/test/dotty/dokka/ScaladocTest.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
package dotty.dokka

import org.jetbrains.dokka.plugability.DokkaContext
import org.jetbrains.dokka.model.{DModule, WithChildren}
import org.jetbrains.dokka.pages.RootPageNode
import org.jetbrains.dokka.testApi.testRunner.{DokkaTestGenerator, TestMethods}
import org.jetbrains.dokka.testApi.logger.TestLogger
import org.jetbrains.dokka.utilities.DokkaConsoleLogger
import org.jetbrains.dokka.DokkaConfiguration
import scala.jdk.CollectionConverters.{ListHasAsScala, SeqHasAsJava}
import org.junit.{Test, Rule}
import org.junit.rules.{TemporaryFolder, ErrorCollector}
import java.io.File

abstract class ScaladocTest(val name: String):
def assertions: Seq[Assertion]

private def getTempDir() : TemporaryFolder =
val folder = new TemporaryFolder()
folder.create()
folder

private def args = Args(
name = "test",
tastyRoots = Nil ,
classpath = System.getProperty("java.class.path"),
None,
output = getTempDir().getRoot,
projectVersion = "1.0",
projectTitle = None,
projectLogo = None,
defaultSyntax = None,
sourceLinks = Nil
)

private def tastyFiles =
def listFilesSafe(dir: File) = Option(dir.listFiles).getOrElse {
throw AssertionError(s"$dir not found. The test name is incorrect or scala3doc-testcases were not recompiled.")
}
def collectFiles(dir: File): List[String] = listFilesSafe(dir).toList.flatMap {
case f if f.isDirectory => collectFiles(f)
case f if f.getName endsWith ".tasty" => f.getAbsolutePath :: Nil
case _ => Nil
}
collectFiles(File(s"${BuildInfo.test_testcasesOutputDir}/tests/$name"))

@Rule
def collector = _collector
private val _collector = new ErrorCollector();
def reportError(msg: String) = collector.addError(new AssertionError(msg))

@Test
def executeTest =
DokkaTestGenerator(
DottyDokkaConfig(DocConfiguration.Standalone(args, tastyFiles, Nil)),
TestLogger(DokkaConsoleLogger.INSTANCE),
assertions.asTestMethods,
Nil.asJava
).generate()

end ScaladocTest

type Validator = () => Unit

/**
* Those assertions map 1-1 to their dokka counterparts. Some of them may be irrelevant in scala3doc.
*/
enum Assertion:
case AfterPluginSetup(fn: DokkaContext => Unit)
case DuringValidation(fn: Validator => Unit)
case AfterDocumentablesCreation(fn: Seq[DModule] => Unit)
case AfterPreMergeDocumentablesTransformation(fn: Seq[DModule] => Unit)
case AfterDocumentablesMerge(fn: DModule => Unit)
case AfterDocumentablesTransformation(fn: DModule => Unit)
case AfterPagesGeneration(fn: RootPageNode => Unit)
case AfterPagesTransformation(fn: RootPageNode => Unit)
case AfterRendering(fn: (RootPageNode, DokkaContext) => Unit)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should turn RootPageNode => Unit and (RootPageNode, DokkaContext) => Unit into traits and provide helper methods, e.g. option to inspect all pages or output of rendering or location of rendered page


extension (s: Seq[Assertion]):
def asTestMethods: TestMethods =
import Assertion._
TestMethods(
(context => s.collect { case AfterPluginSetup(fn) => fn(context) }.kUnit),
(validator => s.collect { case DuringValidation(fn) => fn(() => validator.invoke()) }.kUnit),
(modules => s.collect { case AfterDocumentablesCreation(fn) => fn(modules.asScala.toSeq) }.kUnit),
(modules => s.collect { case AfterPreMergeDocumentablesTransformation(fn) => fn(modules.asScala.toSeq) }.kUnit),
(module => s.collect { case AfterDocumentablesMerge(fn) => fn(module)}.kUnit),
(module => s.collect { case AfterDocumentablesTransformation(fn) => fn(module) }.kUnit),
(root => s.collect { case AfterPagesGeneration(fn) => fn(root) }.kUnit),
(root => s.collect { case AfterPagesTransformation(fn) => fn(root) }.kUnit),
((root, context) => s.collect { case AfterRendering(fn) => fn(root, context)}.kUnit)
)

extension [T] (s: T):
private def kUnit = kotlin.Unit.INSTANCE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one, probably we can unified the kotlin.Unit handling in whole codebase after this PR is merged.

Loading