Search code examples
scalavariablesscalatestscalacheckorder-of-execution

Perplexing unit test code execution order when using ScalaCheck / ScalaTest clauses


I am facing the following perplexing behaviour when unit testing classes with variables.

For the sake of a simple example, let's assume I have the following class:

// Case classes are not an alternative in my use case.
final class C(var i: Int = 0) {
  def add(that: C): Unit = {
    i += that.i
  }

  override def toString: String = {
    s"C($i)"
  }
}

For which I concocted the below trivial and seemingly harmless unit test:

import org.junit.runner.RunWith
import org.scalacheck.Gen
import org.scalatest.junit.JUnitRunner
import org.scalatest.prop.GeneratorDrivenPropertyChecks
import org.scalatest.{MustMatchers, WordSpec}

@RunWith(classOf[JUnitRunner])
class CUnitTest extends WordSpec with MustMatchers with GeneratorDrivenPropertyChecks {
  private val c: C = new C()

  forAll (Gen.choose(1, 100).map(new C(_))) { x =>
    s"Adding $x to $c" must {
      val expectedI = c.i + x.i

      c.add(x)

      s"result in its .i property becoming $expectedI" in {
        c.i mustBe expectedI
      }
    }
  }
}

Where all test cases except the last fail:

enter image description here

For example, the first three test cases fail with the below results:

org.scalatest.exceptions.TestFailedException: 414 was not equal to 68
org.scalatest.exceptions.TestFailedException: 414 was not equal to 89
org.scalatest.exceptions.TestFailedException: 414 was not equal to 151

Now, playing around the unit test and moving the c.add(x) part inside the in clause:

import org.junit.runner.RunWith
import org.scalacheck.Gen
import org.scalatest.junit.JUnitRunner
import org.scalatest.prop.GeneratorDrivenPropertyChecks
import org.scalatest.{MustMatchers, WordSpec}

@RunWith(classOf[JUnitRunner])
class CUnitTest extends WordSpec with MustMatchers with GeneratorDrivenPropertyChecks {
  private val c: C = new C()

  forAll (Gen.choose(1, 100).map(new C(_))) { x =>
    s"Adding $x to $c" must {
      val expectedI = c.i + x.i

      s"result in its .i property becoming $expectedI" in {
        c.add(x)

        c.i mustBe expectedI
      }
    }
  }
}

And all test cases except the first fail:

enter image description here

For example, the second and the third test cases fail with the following messages:

org.scalatest.exceptions.TestFailedException: 46 was not equal to 44
org.scalatest.exceptions.TestFailedException: 114 was not equal to 68

In addition, c.i doesn't seem to increase at all in the test case description as I intended and expected it to be.

Clearly, the order of execution inside ScalaTest clauses are not top-down. Something happens earlier or later than in the order it's written, or perhaps doesn't happen at all depending on which clause it is inside, yet I can't wrap my head around it.

What's going on and why? Furthermore, how could I achieve the desired behaviour (c.i increasing, all test cases passing)?


Solution

  • Consider rewriting the test like so

    import org.scalacheck.Gen
    import org.scalatest._
    import org.scalatestplus.scalacheck.ScalaCheckDrivenPropertyChecks
    
    class HelloSpec extends WordSpec with MustMatchers with ScalaCheckDrivenPropertyChecks {
      private val c: C = new C()
    
      "class C" must {
        "add another class C" in {
          forAll (Gen.choose(1, 100).map(new C(_))) { x =>
            val expectedI = c.i + x.i
            c.add(x)
            c.i mustBe expectedI
          }
        }
      }
    }
    

    Note how here forAll is on the "inside" of the test body which means we have a single test which is using multiple inputs provided by forAll to test the system C. When it is on the "outside" like so

    forAll (Gen.choose(1, 100).map(new C(_))) { x =>
      s"Adding $x to $c" must {
        ...
        s"result in its .i property becoming $expectedI" in {
          ...
        }
      }
    }
    

    then forAll is misused to generate multiple tests where each has a single test input, however the purpose of forAll is to generate multiple inputs for system under test, not multiple tests. Furthermore, design of CUnitTest results in subsequent test depending on the state of the previous test which is buggy and harder to maintain. Ideally tests would run in isolation of each other where all the state needed is provided afresh as part of a test fixture.

    Few side notes: @RunWith(classOf[JUnitRunner]) should not be necessary, and GeneratorDrivenPropertyChecks is deprecated.