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

sbt ScalaCheckRunner: loss of test selection fidelity #1105

Open
dubinsky opened this issue Mar 16, 2025 · 3 comments · May be fixed by #1107
Open

sbt ScalaCheckRunner: loss of test selection fidelity #1105

dubinsky opened this issue Mar 16, 2025 · 3 comments · May be fixed by #1107

Comments

@dubinsky
Copy link

The only code that looks at the test selectors supplied via the sbt.testing.TaskDef is ScalaCheckRunner.checkPropTask(); it recognizes sbt.testing.TestSelectors and selects the properties with the exact names supplied; it should also recognize sbt.testing.TestWildcardSelectors and select the properties with the names containing the wildcard supplied.

Also, ScalaCheckRunner.deserializeTask() should probably count sbt.testing.TestWildcardSelectors towards countTestSelectors and thus dispatch to ScalaCheckRunner.checkPropTask() and not to ScalaCheckRunner.rootTask() if any are supplied.

Finally, whatever test selection functionality ScalaCheck provides only kicks in in very specific use cases:

  • when ScalaCheckRunner.deserializeTask() is called or
  • when class name of the sbt.testing.Fingerprint used contains "ForkMain".

When running ScalaCheck the "official" sbt.testing way, by calling ScalaCheckRunner.tasks() with the fingerprints returned from ScalaCheckFramework.fingerprints() (none of which contain "ForkMain" in their class names), ScalaCheckRunner.checkPropTask() is never called, and no test selection is applied.

Context: Scala.js Gradle plugin integrates with sbt.testing-enabled test frameworks, and ScalaCheck is among them. Plugin does its own TaskDef serialization/deserialization in a way that is not specific to the test framework used, but works with Gradle. As a result of the above idiosyncrasies in the ScalaCheck's test selection functionality, plugin can not access any of it :(

@satorg
Copy link
Contributor

satorg commented Apr 1, 2025

Hi @dubinsky , thank you for reporting the issue.
I apologize in advance if this question isn’t fully thought through,
but do you think this issue and PR #1031 could be connected somehow?

@dubinsky
Copy link
Author

dubinsky commented Apr 4, 2025

Hi @satorg!

do you think this issue and PR #1031 could be connected somehow?

Indeed they are connected! That PR closes a part of the gap that I need closed ;)

To clarify: this is not about any issues with ScalaCheck itself; it is about driving it via the SBT Test Interface. Invented by the SBT developers, that interface allows plugging test frameworks into SBT without requiring SBT to know the details of the framework's API. Now that every test framework has an implementation of this interface, it can be used to integrate them with tools other than SBT (in my case - Gradle).

The protocol is as follows:

  • obtain an instance of the framework-specific implementation of sbt.testing.Framework (for ScalaCheck, that is org.scalacheck.ScalaCheckFramework);
  • retrieve Framework.fingerprints - description of what classes/objects the framework knows how to run (for ScalaCheck, it is "class or object derived from either org.scalacheck.Properties or org.scalacheck.Prop");
  • using fingerprints, do your own test discovery, without involving the test framework at all;
  • obtain one Framework.runner that will be used to run all the test classes/objects one-by-one;
  • construct a sbt.testing.TaskDef for each class/object you want to run;
  • retrieve Runner.tasks for each of the TaskDefs;
  • invoke task.execute(EventHandler) for each task;
  • in addition to emitting events via the EventHandler, task.execute can (and in ScalaCheck - does) return nested tasks that also need to be executed.

When obtaining the Runner, it is possible to supply framework-specific command-line arguments like -propFilter, but since those arguments apply to all classes/objects that need to be run, this approach is not in general flexible enough to express desired filtering (consider two objects, X and Y, both with properties a and b, where we need to run X.a and Y.b - and nothing else). SBT Test Interface, as @Duhemm mentioned, does provide a facility to express the desired filtering - the issue is that ScalaCheck's implementation of this facility is, sadly, lacking.

To describe to the framework what needs to be run in a class/object, a sbt.testing.TaskDef is used; it has the following data:

  • fullyQualifiedName: String identifies the class/object to run;
  • fingerprint: Fingerprint associates one of the framework's fingerprints with it (and may affect how it is run);
  • explicitlySpecified: Boolean, if set to true, means that any included classes/objects should not run;
  • selectors: Array[Selector]: specifies what members of the class/object being run should actually run.

Coming from the code driving the framework via the SBT Test Interface, selectors array can contain:

  • exactly one SuiteSelector, indicating that the class/object should be run in its entirety; or
  • any number of TestSelectors and TestWildcardSelectors, indicating which tests belonging to the class/object should be run using their exact names or sub-strings that those names contain respectively.

The problem is that currently ScalaCheck's implementation of the SBT Test Interface completely ignores both selectors and explicitlySpecified fields of the TaskDef. This should be reasonably easy to fix - should you accept that this issue needs fixing ;)

Thank you!

Here is a little test that demonstrates (a part of) this issue:

package org.scalacheck

import sbt.testing.{Event, EventHandler, Framework, Runner, Selector, SuiteSelector, Task, TaskDef, TestSelector}

object SbtFixture extends Properties("SbtFixture") {
  property("success") = Prop.passed
}

object SbtSpecification extends Properties("Sbt") {
  property("runAll") = {
    val ran: List[String] = SbtDriver.run(
      fullyQualifiedName = "org.scalacheck.SbtFixture",
      selectors = Array(new SuiteSelector)
    )

    ran.length == 1 && ran.head == "SbtFixture.success"
  }

  property("runNonExistent") = {
    val ran: List[String] = SbtDriver.run(
      fullyQualifiedName = "org.scalacheck.SbtFixture",
      selectors = Array(new TestSelector("nonexistent"))
    )

    // Since we are asking to run a property that does not exist, result should be:
    ran.isEmpty
    // but because ScalaCheckFramework ignores taskDef.selectors, it runs everything instead:
//    ran.length == 1 && ran.head == "SbtFixture.success"
  }
}

object SbtDriver {
  private class StoringEventHandler extends EventHandler {
    private var ran: List[String] = List.empty
    def getRan: List[String] = ran
    override def handle(event: Event): Unit = ran = ran.appended(event.selector.asInstanceOf[TestSelector].testName)
  }

  def run(
    fullyQualifiedName: String,
    explicitlySpecified: Boolean = false,
    selectors: Array[Selector]
  ): List[String] = {
    val framework: Framework = new ScalaCheckFramework
    val runner: Runner = framework.runner(Array.empty, Array.empty, getClass.getClassLoader)
    val taskDef: TaskDef = new TaskDef(
      fullyQualifiedName,
      framework.fingerprints()(2), // object ... extends org.scalacheck.Properties
      explicitlySpecified,
      selectors
    )
    val eventHandler: StoringEventHandler = new StoringEventHandler
    def execute(task: Task): Unit = task.execute(eventHandler, Array.empty).foreach(execute)
    runner.tasks(Array(taskDef)).foreach(execute)
    eventHandler.getRan
  }
}

dubinsky added a commit to dubinsky/scalacheck that referenced this issue Apr 4, 2025
…dcardSelector`s, filter properties to run by matching their names against the `Selector`s;

- match a property by both its short and full name;
- added a test that actually runs ScalaCheck via the `SBT Test Interface` and demonstrates the (now correct) treatment of the `Selector`s;
- test also demonstrates two unfixable infidelities in the treatment of the nested properties;
- thankfully, ScalaCheck's implementation of `sbt.testing.Framework` is, unlike in some other test frameworks, shared between the platforms (JVM, Scala.js, Scala Native), so the fixes do not have to be replicated for each platform, but:
- the test needs to supply a `testClassLoader: ClassLoader` parameter when calling `sbt.testing.Framework.runner()`; on platforms other than JVM, `getClass.getClassLoader` is not available, so `Platform.getClassLoader: ClassLoader` method was added to every `Platform`; on platforms other than the JVM, it returns `null`, which is fine since on those platforms `sbt.testing.Framework.runner()` ignores the `testClassLoader` parameter anyway.

fixes typelevel#1105
@dubinsky
Copy link
Author

dubinsky commented Apr 4, 2025

Hi @satorg! #1107 is my attempt to:

  • fix what can be fixed;
  • test the fix;
  • demonstrate behavior that can not be fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants