Test suite throws "org.junit.platform.commons.PreconditionViolationException: The discover() method for TestEngine with ID 'scalatest' returned a cyclic graph." error

[tribbloid]

When running the test for a well-defined scalatest package. The runner may sometimes yield the following error:

failed to execute tests

org.gradle.api.internal.tasks.testing.TestSuiteExecutionException: Could not complete execution for Gradle Test Executor 1.
	at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.stop(SuiteTestClassProcessor.java:63)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
	at org.gradle.internal.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:33)
	at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:94)
	at com.sun.proxy.$Proxy2.stop(Unknown Source)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker$3.run(TestWorker.java:193)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.executeAndMaintainThreadName(TestWorker.java:129)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:100)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:60)
	at org.gradle.process.internal.worker.child.ActionExecutionWorker.execute(ActionExecutionWorker.java:56)
	at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:133)
	at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:71)
	at worker.org.gradle.process.internal.worker.GradleWorkerMain.run(GradleWorkerMain.java:69)
	at worker.org.gradle.process.internal.worker.GradleWorkerMain.main(GradleWorkerMain.java:74)
Caused by: org.junit.platform.commons.JUnitException: TestEngine with ID 'scalatest' failed to discover tests
	at org.junit.platform.launcher.core.EngineDiscoveryOrchestrator.discoverEngineRoot(EngineDiscoveryOrchestrator.java:111)
	at org.junit.platform.launcher.core.EngineDiscoveryOrchestrator.discover(EngineDiscoveryOrchestrator.java:85)
	at org.junit.platform.launcher.core.DefaultLauncher.discover(DefaultLauncher.java:92)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:75)
	at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor$CollectAllTestClassesExecutor.processAllTestClasses(JUnitPlatformTestClassProcessor.java:99)
	at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor$CollectAllTestClassesExecutor.access$000(JUnitPlatformTestClassProcessor.java:79)
	at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor.stop(JUnitPlatformTestClassProcessor.java:75)
	at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.stop(SuiteTestClassProcessor.java:61)
	... 18 more
Caused by: org.junit.platform.commons.PreconditionViolationException: The discover() method for TestEngine with ID 'scalatest' returned a cyclic graph.
	at org.junit.platform.commons.util.Preconditions.condition(Preconditions.java:296)
	at org.junit.platform.launcher.core.EngineDiscoveryResultValidator.validate(EngineDiscoveryResultValidator.java:40)
	at org.junit.platform.launcher.core.EngineDiscoveryOrchestrator.discoverEngineRoot(EngineDiscoveryOrchestrator.java:104)
	... 25 more

A clear and concise description of what the bug is.

To Reproduce

• Check out https://github.com/tribbloid/spookystuff/tree/ddf2ef2603f73090c44f4b7b616425972eef4c12
• run ./gradlew clean test

Expected behavior

Should run all the tests

In worst case it should print which part of the graph is cyclic

Screenshots

Build/Test environment:

• Build tool [e.g. Gradle, Maven] and version : Gradle 7.4.2
• Scala version : 2.12.16
• Scalatest version: 3.2.10
• other relevant libraries/versions : JUnit 5.8.2

Additional context

[tribbloid]

Found the real cause: it can be triggered if a test Suite have defined nested suited which share the same name:

class DummySpec extends AnyFunSpec {

  case class Child() extends AnyFunSpec {
    it("test 1") {}
  }

  override val nestedSuites: immutable.IndexedSeq[Suite] = {

    immutable.IndexedSeq(Child(), Child())
  }
}

This gives the error:

org.gradle.api.internal.tasks.testing.TestSuiteExecutionException: Could not complete execution for Gradle Test Executor 13.
	at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.stop(SuiteTestClassProcessor.java:63)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
	at org.gradle.internal.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:33)
	at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:94)
	at com.sun.proxy.$Proxy2.stop(Unknown Source)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker$3.run(TestWorker.java:193)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.executeAndMaintainThreadName(TestWorker.java:129)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:100)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:60)
	at org.gradle.process.internal.worker.child.ActionExecutionWorker.execute(ActionExecutionWorker.java:56)
	at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:133)
	at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:71)
	at worker.org.gradle.process.internal.worker.GradleWorkerMain.run(GradleWorkerMain.java:69)
	at worker.org.gradle.process.internal.worker.GradleWorkerMain.main(GradleWorkerMain.java:74)
Caused by: org.junit.platform.commons.JUnitException: TestEngine with ID 'scalatest' failed to discover tests
	at org.junit.platform.launcher.core.EngineDiscoveryOrchestrator.discoverEngineRoot(EngineDiscoveryOrchestrator.java:111)
	at org.junit.platform.launcher.core.EngineDiscoveryOrchestrator.discover(EngineDiscoveryOrchestrator.java:85)
	at org.junit.platform.launcher.core.DefaultLauncher.discover(DefaultLauncher.java:92)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:75)
	at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor$CollectAllTestClassesExecutor.processAllTestClasses(JUnitPlatformTestClassProcessor.java:99)
	at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor$CollectAllTestClassesExecutor.access$000(JUnitPlatformTestClassProcessor.java:79)
	at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor.stop(JUnitPlatformTestClassProcessor.java:75)
	at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.stop(SuiteTestClassProcessor.java:61)
	... 18 more
Caused by: org.junit.platform.commons.PreconditionViolationException: The discover() method for TestEngine with ID 'scalatest' returned a cyclic graph.
	at org.junit.platform.commons.util.Preconditions.condition(Preconditions.java:296)
	at org.junit.platform.launcher.core.EngineDiscoveryResultValidator.validate(EngineDiscoveryResultValidator.java:40)
	at org.junit.platform.launcher.core.EngineDiscoveryOrchestrator.discoverEngineRoot(EngineDiscoveryOrchestrator.java:104)
	... 25 more

By avoiding identical names, a slightly different suite won't trigger this error:

class DummySpec extends AnyFunSpec {

  case class Child() extends AnyFunSpec {
    it("test 1") {}
  }

  object C1 extends Child()
  object C2 extends Child()

  override val nestedSuites: immutable.IndexedSeq[Suite] = {

    immutable.IndexedSeq(C1, C2)
  }
}

So there are ways to circumvent it. But I still prefer it to be either patched softly or fail with a clear & actionable error message

[giurim]

Hi Peng Cheng, Thank you very much for the report and the root cause analysis! I will fix and release a new version ASAP. Please allow me a few days.

…

On Tue, 14 Jun 2022 at 20:17, Peng Cheng ***@***.***> wrote: Found the real cause: it can be triggered if a test Suite have defined nested suited which share the same name: class DummySpec extends AnyFunSpec { case class Child() extends AnyFunSpec { it("test 1") {} } override val nestedSuites: immutable.IndexedSeq[Suite] = { immutable.IndexedSeq(Child(), Child()) } } This gives the error: org.gradle.api.internal.tasks.testing.TestSuiteExecutionException: Could not complete execution for Gradle Test Executor 13. at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.stop(SuiteTestClassProcessor.java:63) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:498) at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36) at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24) at org.gradle.internal.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:33) at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:94) at com.sun.proxy.$Proxy2.stop(Unknown Source) at org.gradle.api.internal.tasks.testing.worker.TestWorker$3.run(TestWorker.java:193) at org.gradle.api.internal.tasks.testing.worker.TestWorker.executeAndMaintainThreadName(TestWorker.java:129) at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:100) at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:60) at org.gradle.process.internal.worker.child.ActionExecutionWorker.execute(ActionExecutionWorker.java:56) at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:133) at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:71) at worker.org.gradle.process.internal.worker.GradleWorkerMain.run(GradleWorkerMain.java:69) at worker.org.gradle.process.internal.worker.GradleWorkerMain.main(GradleWorkerMain.java:74) Caused by: org.junit.platform.commons.JUnitException: TestEngine with ID 'scalatest' failed to discover tests at org.junit.platform.launcher.core.EngineDiscoveryOrchestrator.discoverEngineRoot(EngineDiscoveryOrchestrator.java:111) at org.junit.platform.launcher.core.EngineDiscoveryOrchestrator.discover(EngineDiscoveryOrchestrator.java:85) at org.junit.platform.launcher.core.DefaultLauncher.discover(DefaultLauncher.java:92) at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:75) at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor$CollectAllTestClassesExecutor.processAllTestClasses(JUnitPlatformTestClassProcessor.java:99) at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor$CollectAllTestClassesExecutor.access$000(JUnitPlatformTestClassProcessor.java:79) at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor.stop(JUnitPlatformTestClassProcessor.java:75) at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.stop(SuiteTestClassProcessor.java:61) ... 18 more Caused by: org.junit.platform.commons.PreconditionViolationException: The discover() method for TestEngine with ID 'scalatest' returned a cyclic graph. at org.junit.platform.commons.util.Preconditions.condition(Preconditions.java:296) at org.junit.platform.launcher.core.EngineDiscoveryResultValidator.validate(EngineDiscoveryResultValidator.java:40) at org.junit.platform.launcher.core.EngineDiscoveryOrchestrator.discoverEngineRoot(EngineDiscoveryOrchestrator.java:104) ... 25 more By avoiding identical names, a slightly different suite won't trigger this error: class DummySpec extends AnyFunSpec { case class Child() extends AnyFunSpec { it("test 1") {} } object C1 extends Child() object C2 extends Child() override val nestedSuites: immutable.IndexedSeq[Suite] = { immutable.IndexedSeq(C1, C2) } } So there are ways to circumvent it. But I still prefer it to be either patched softly or fail with a clear & actionable error message — Reply to this email directly, view it on GitHub <#83 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADST4FLDF7LHQL5H7X3MCR3VPDEEDANCNFSM5YYMY44A> . You are receiving this because you were assigned.Message ID: ***@***.***>

-- -- Móra György

[giurim]

I was trying to fix the issue but the fix will break other functionality. If I suffix the test to be unique I will basically change the name of the test and filtering/disabling might not work as expected.

I think the best now is to detect and throw an error in this case as this is not a main use-case anyways and I think should be avoided.

[tribbloid]

agreed, porting a niche feature of scalatest to junit takes too much effort to be justified

[tribbloid]

Which branch are you working on? I may be able to help in compiling and testing on my projects locally

[giurim]

I am afk, nothing opened yet, I will have some time tonight, feel free to open a PR if you want.

…

On Sat, Oct 1, 2022, 01:46 Peng Cheng ***@***.***> wrote: Which branch are you working on? I may be able to help in compiling and testing on my projects locally — Reply to this email directly, view it on GitHub <#83 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADST4FIUVOVWYRSYEY77QL3WA53VNANCNFSM5YYMY44A> . You are receiving this because you were assigned.Message ID: ***@***.***>

[tribbloid]

I've proposed a more general (?) patch to be integrated into JUnit5, Do you think it make more sense to submit there?

[mpkorstanje]

If I suffix the test to be unique I will basically change the name of the test and filtering/disabling might not work as expected.

In JUnit the name and ID are different concepts. So you can make the ID unique without changing the test name.

I've had to solve the same problem for Cucumber where Scenarios can all have the same name and we support filtering by scenario name. E.g:

Feature: Example Feature

  Scenario: Example Scenario
...
  Scenario: Example Scenario
...
  Scenario: Example Scenario
...

I've added some suggestions on how to fix your problem to the JUnit issue.

[giurim]

I went trough the Scalatest code again. Here is what I think:

• I use the scalatest provided "suiteId" which is suppesed to be unique but it is not enforced. If the suite classes are modified (like in the above case the nestedSuites is overridden) it is the test authors task to ensure all nested suite gets an unique Id. In the OP's code the suites themselves do not know they are nested and by default one class is instantiated for one test therefore the default unique id is the FQN of the class.
• If a test author will override all suite's suiteId to the same constant, it will fail like this.
• Also, if a parametric test won't override the suiteId properly and the same class would be instantiated multiple times, same error would happen.
• I think I should continue to use the ids provided by Scalatest so selectors/filters can be the same when users migrate to Junit5 from native Scalatest test running.
• This indeed shoudl be handled in the test engine and not inside Junit.

After reading this in the source of Suite:

This trait's
implementation of this method returns the fully qualified name of this object's class.
Each suite reported during a run will commonly be an instance of a different Suite class,
and in such cases, this default implementation of this method will suffice. However, in special cases
you may need to override this method to ensure it is unique for each reported suite. For example, if you write
a Suite subclass that reads in a file whose name is passed to its constructor and dynamically
creates a suite of tests based on the information in that file, you will likely need to override this method
in your Suite subclass, perhaps by appending the pathname of the file to the fully qualified class name.
That way if you run a suite of tests based on a directory full of these files, you'll have unique suite IDs for
each reported suite.

The suite ID is intended to be unique, because ScalaTest does not enforce that it is unique. If it is not
unique, then you may not be able to uniquely identify a particular test of a particular suite. This ability is used,
for example, to dynamically tag tests as having failed in the previous run when rerunning only failed tests.

I think, we should detect if ids are not unique and throw an error in the test Engine. Scalatest might tolerate when ids are not unique but it is not encouraged there either and Junit5 does not tolerate it. We should not try to fix issues introduced by custom Suites not providing ids in a way Scalatest prescribes.

This code snippet from above is one possible way to avoid the error:

class DummySpec extends AnyFunSpec {

  case class Child() extends AnyFunSpec {
    it("test 1") {}
  }

  object C1 extends Child()
  object C2 extends Child()

  override val nestedSuites: immutable.IndexedSeq[Suite] = {

    immutable.IndexedSeq(C1, C2)
  }
}

[mpkorstanje]

I think I should continue to use the ids provided by Scalatest so selectors/filters can be the same when users migrate to Junit5 from native Scalatest test running.

While JUnit 5 allows tests to be selected by ID, this is mostly for APIs rather then humans. The ability to select by ID is omitted from the Console Launcher as well as the Suite Launcher. So I think it would be reasonable to assume that users will never have a need to user JUnits ID's directly.