Pass in correct explicitlySpecified and selectors when creating TestDefinitions

[antoine-mulet]

Simply something that has never been implemented it seems. For example Scalatest relies on these two fields to implement the tasks run logic so it's a shame they are currently hardcoded.

sbt/main-actions/src/main/scala/sbt/Tests.scala

Lines 570 to 575 in 62c6cad

	// TODO: To pass in correct explicitlySpecified and selectors
	val tests =
	for {
	(df, di) <- discovered
	fingerprint <- toFingerprints(di)
	} yield new TestDefinition(df.name, fingerprint, false, Array(new SuiteSelector: Selector))

This is the first time I take a look at the sbt code but if someone can provide me with some guidance I would be happy to contribute.

[eed3si9n]

@antoine-mulet Thanks for the report. Could you provide reproduction steps so we can reproduce your problem on our computers?

[antoine-mulet]

@eed3si9n sorry if something wasn't clear. I didn't think a proper repro was necessary here because we are talking about an interface that has been partially implemented using hardcoded default values.

To give a bit more details, here is what I understand about the issue:

sbt TestDefinition

sbt/testing/src/main/scala/sbt/TestFramework.scala

Lines 82 to 87 in 975e3db

	final class TestDefinition(
	val name: String,
	val fingerprint: Fingerprint,
	val explicitlySpecified: Boolean,
	val selectors: Array[Selector]
	) {

maps to test-interface TaskDef

https://github.com/harrah/test-interface/blob/17a94641942546e21f4c6b300a3360be2d2888f6/src/main/java/sbt/testing/TaskDef.java

In there it is very clear what explicitlySpecified is supposed to be

https://github.com/harrah/test-interface/blob/17a94641942546e21f4c6b300a3360be2d2888f6/src/main/java/sbt/testing/TaskDef.java#L15-L43

and same for selectors

https://github.com/harrah/test-interface/blob/17a94641942546e21f4c6b300a3360be2d2888f6/src/main/java/sbt/testing/Selector.java

However explicitlySpecified and selectors are simply not implemented on the sbt side. So down the line when test frameworks like ScalaTest rely on those fields, it leads to bugs like scalatest/scalatest#417 for example.

That being said, to answer the original question, here is how you can reproduce the issue with ScalaTest:

1. Define a Scalatest suite

import org.scalatest.DoNotDiscover
import org.scalatest.flatspec.AnyFlatSpec
import org.scalatest.matchers.should.Matchers

@DoNotDiscover
class FooTestSuite extends AnyFlatSpec with Matchers {

  it should "run" in {
    1 + 1 shouldBe 2
  }
}

2. Run it with sbt

sbt "testOnly com.foo.test.FooTestSuite"

3. The output shows that no tests were run

[info] ScalaTest
[info] Run completed in 23 minutes, 27 seconds.
[info] Total number of tests run: 0
[info] Suites: completed 0, aborted 0
[info] Tests: succeeded 0, failed 0, canceled 0, ignored 0, pending 0
[info] No tests were executed.

And this is because explicitlySpecified was set to false by sbt (as always in the current implementation) while it should have been set to true to let the suite be discovered properly. The same way any logic relying on the correct values to be set for selectors would potentially lead to bugs.

Another way to double check the issue is to run the same sbt command in debug mode with breakpoints in the sbt / ScalaTest code to inspect the values of explicitlySpecified and selectors.

I hope everything is clearer with the explanations above, if still not please let me know.

[antoine-mulet]

Hi @eed3si9n, do you have any comments? Is everything above clear?

Thanks,
Antoine

[eed3si9n]

Thanks for unpacking this. @DoNotDiscover sounds like a good repro step for testing.