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

AsyncFunSuite and StepwiseNestedSuiteExecution do not work together #2181

Closed
jespermbyskov opened this issue Nov 14, 2022 · 5 comments
Closed

Comments

@jespermbyskov
Copy link

jespermbyskov commented Nov 14, 2022

I have a test suite that extends AsyncFunSuite. I would like to parallelise it, but I have some tests that need to run sequentially, and only after all parallel tests have run.

Now, I tried to split the suite in two and added a parent suite extending StepwiseNestedSuiteExecution with two nested AsyncFunSuites, the first of which extends ParallelTestExecution, see example code below. If I run this, either with or without the -P option, all the parallel tests are started, but right after the sequential ones are started, before the parallel once have finished. I believe that this is a bug in the callExecuteOnSuite method in StepwiseNestedSuiteExecution, since it always returns a SucceededStatus right after having invoked run on the nested suite, instead of returning the status returned by that call. If I create a copy of StepwiseNestedSuiteExecution where I replace the lines

          report(SuiteCompleted(tracker.nextOrdinal(), nestedSuite.suiteName, nestedSuite.suiteId, Some(suiteClassName), Some(duration), formatter, Some(TopOfClass(nestedSuite.getClass.getName)), nestedSuite.rerunner))
          SucceededStatus

with

          status.withAfterEffect {
            report(SuiteCompleted(tracker.nextOrdinal(), nestedSuite.suiteName, nestedSuite.suiteId, Some(suiteClassName), Some(duration), formatter, Some(TopOfClass(nestedSuite.getClass.getName)), nestedSuite.rerunner))
          }
          status

then it seems to work. I have not created a PR for this, as I do not know the code well enough to know, if this has unintended effects.

Even with the above changes to StepwiseNestedSuiteExecution, if I run the test without the -P option, I get the following error:

Reporter completed abruptly with an exception after receiving event: SuiteCompleted(Ordinal(1, 9),ParallelSuite$ParallelTest,org.scalatest.ParallelSuite$ParallelTest,Some(org.scalatest.ParallelSuite$ParallelTest),Some(38),Some(MotionToSuppress),Some(TopOfClass(org.scalatest.ParallelSuite$ParallelTest)),None,None,scala-execution-context-global-43,1668497451326).
java.lang.RuntimeException: unexpected event [TestStarting(Ordinal(1, 4),ParallelSuite$ParallelTest,org.scalatest.ParallelSuite$ParallelTest,Some(org.scalatest.ParallelSuite$ParallelTest),Test 2,Test 2,Some(MotionToSuppress),Some(LineInFile(41,ParallelSuite.scala,Some(Please set the environment variable SCALACTIC_FILL_FILE_PATHNAMES to yes at compile time to enable this feature.))),None,None,ScalaTest-main-running-ParallelSuite$ParallelTest,1668497450824)]
	at io.bazel.rules.scala.JUnitXmlReporter.unexpected(JUnitXmlReporter.scala:503)
	at io.bazel.rules.scala.JUnitXmlReporter.processTest(JUnitXmlReporter.scala:333)
	at io.bazel.rules.scala.JUnitXmlReporter.getTestsuite(JUnitXmlReporter.scala:174)
	at io.bazel.rules.scala.JUnitXmlReporter.apply(JUnitXmlReporter.scala:57)

which seems to be caused by no Distributor being sent to the runTests method in ParallelTestExecution. I believe, that this is another bug.

Example code:

package org.scalatest

import org.scalatest.funsuite.AsyncFunSuite

import java.util.concurrent.atomic.{ AtomicBoolean, AtomicInteger }
import scala.concurrent.{ ExecutionContext, Future }

class ParallelSuite extends AsyncFunSuite with BeforeAndAfterAll with StepwiseNestedSuiteExecution {

  override val nestedSuites: IndexedSeq[Suite] = IndexedSeq(new ParallelTest, new SequentialTest)

  val beforeRun = new AtomicBoolean(false)

  val testCount = new AtomicInteger()

  override def beforeAll(): Unit = {
    println("Before")
    assert(beforeRun.compareAndSet(false, true))
  }

  override def afterAll(): Unit = {
    println("After")
    assert(beforeRun.get())
    assert(testCount.get() == 4)
  }

  @DoNotDiscover
  class ParallelTest extends AsyncFunSuite with ParallelTestExecution {

    override implicit def executionContext: ExecutionContext = ExecutionContext.global

    testFn("Test 1", 500)

    testFn("Test 2", 100)

    testFn("Test 3", 300)

    private def testFn(testName: String, sleepTimeMillis: Int): Unit = {
      test(testName) {
        println(s"$testName started")
        val fut = Future {
          assert(beforeRun.get())
          Thread.sleep(sleepTimeMillis)
          testCount.incrementAndGet()
          Assertions.succeed
        }
        fut.onComplete { _ =>
          println(s"$testName finished")
        }
        fut
      }
    }

    override def newInstance: Suite with ParallelTestExecution = new ParallelTest
  }

  @DoNotDiscover
  class SequentialTest extends AsyncFunSuite {
    test("Last test") {
      println("Last test started")
      assert(beforeRun.get())
      assert(testCount.get() == 3)
      val fut = Future {
        Thread.sleep(50)
        testCount.incrementAndGet()
        Assertions.succeed
      }
      fut.onComplete { _ =>
        println("Last test finished")
      }
      fut
    }
  }
}
@jespermbyskov
Copy link
Author

jespermbyskov commented Nov 15, 2022

The second bug is probably related to item 2 in this comment.

@cheeseng
Copy link
Contributor

cheeseng commented Dec 5, 2022

@jespermbyskov Your first fix does look valid to me, I'll try and look into your detailed example code.

Cheers.

@cheeseng
Copy link
Contributor

cheeseng commented Dec 6, 2022

@jespermbyskov Based on the code you posted I have submitted the following PR:

#2211

Would you mind to verify if it will fix the problem that you are facing?

Cheers.

@jespermbyskov
Copy link
Author

@cheeseng I tried running my tests with the changes in the PR, and the test succeeds, also when not running with the -P option, so the fix is fine for me. Thanks!

@jespermbyskov
Copy link
Author

I updated to ScalaTest 3.2.15 and that fixes my problem, so I think that this issue can be closed.

@bvenners bvenners closed this as completed Jan 9, 2023
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

No branches or pull requests

3 participants