2016-09-09

Scalatest: thoughts and ideas

Spark uses Scalatest for its testing.

I alternate between "really liking this" and "bemoaning aspects of it".

Overall: I think the test language is better than JUnit, especially once you embrace more of the scala language. But the test execution could be improved, especially maven integration, and, to a lesser extent, that test reporting.

France 2016

Likes


eventually()


  eventually(timeout, policy) { closure };

This does a busy wait for the closure to complete (i.e. not throw an exception), policies include exponential backoff.

Pro: easy to write clauses in a test case which block until a state is reached or things timeout

Con: there's no way by default for those clauses to declare that they will never complete. That happens, especially if you are waiting for some state such as the number of events to be exactly 3 in a queue. If the queue size goes to 4, it's never going to drop to 3, so the clause could fail fast.

I think the fix there would be to define a specific "unrecoverable assertion failure" exception and raise that, have "eventually()" not swallow them.


the === check,


 assert(3 === events.queueSize)

This does good reporting on what the values are on an equality failure. It does need rigorousness about the order; I follow the junit assertEquals policy of expected-value-first. Projects really need a style guide/policy here.

Weakness: no typechecking at compile time. Better get those types right. Though as your tests should run through all assertions unless it is somehow nondeterministic, you'll find out soon enough.

Intercept:



  val ex = intercept[IOException.class] {
    doSomethingThatShouldRaiseAnIOE()
  }
  assert(ex.contains("host"), s"no 'host' in $ex")

Here there's a straightforward catch of a specific type (and failure raised if it didn't happen). As the exception is returned, the test case can continue. And with string formatting with the s"" syntax, reasonable groovy-esque diagnostics.


Matchers and the declarative specification syntax


  update("submissions") should be(5)

Ignoring the fact that the RFC2517 means that the syntax ought to be "must" and not "should", this style is fairly similar to JUnit 4 matchers, which is not something I ever do much of.

Pro: looks nicer than assert() clauses
weaknesses: you can add more details in assert() clauses —provided you sit down and do it.


Could try harder

 

the before and after mechanisms:, beforeclass and afterclass



While seems and easy way to declare things to run before tests, it doesn't work well with traits/mixins. In contrast, having setup() and teardown() methods, combined with Scala's strict order of evaluating traits and what what "super.setup()" would mean in a trait, allows you to write tests with traits where different traits can execute their setup/teardown clauses in a deterministic manner. Note that JUnit's before/after stuff is also a bit ambiguous, so it's not a denunciation of Scalatest here —more a commentary of the setup problem.

ignoring tests.


I've ended up doing this in the tests for spark-cloud, writing a conditional ctest() function. The secret there is to know that the test(name) { closure } declaration is not declaring a method, as in JUnit, but simply declaring an invocation of the test() method —an invocation which takes place during the construction of an instance of the test suite. There is nothing to stop you wrapping that call in other mechanisms: extra conditions, dynamic parameters, etc. It's only a closure, you understand.

Maven integration



Needs better wildcard and test suite declarations. I've only just discovered the ability to include a string in the name of the test case to run:

mvt -Dtest=none '-Dsuites=org.apache.spark.deploy.history.HistoryServerSuite incomplete' ; say moo

This command
  • doesn't run any java tests (because -D test=none doesn't match anything)
  • runs every test in HistoryServerSuite which has the world "incomplete" in the title
  • says "moo" after, so I can get on with other things while scalac gets round to building things, scalatest to running them. This matters because Scalac is so, so slow.

Reporting

The test reporter "helpfully" strips off the stack trace on assertion failures, so you don't get the full stack. This seems a good idea, except in the special case "you have the assertions inside a method which your test cases call". Do that and you suddenly lose the ability to detect what's wrong because the stack trace has been stripped —all you get is the name of the utility method. Just give us the full stack and stop trying to be clever..

My new conditional test runner


Here, then , is my ctest() function to declare a test which is conditional on the state of the entire suite, and an optional extra per-test predicate

/**
* A conditional test which is only executed when the
* suite is enabled and the `extraCondition`
* predicate holds.
* @param name test name
* @param detail detailed text for reports
* @param extraCondition extra predicate which may be
* evaluated to decide if a test can run.
* @param testFun function to execute
*/
protected def ctest(
  name: String,
  detail: String = "",
  extraCondition: => Boolean = true)
  (testFun: => Unit): Unit = {
  if (enabled && extraCondition) {
    registerTest(name) {
      logInfo(s"$name\n$detail\n---------------")
      testFun
    }
  } else {
    registerIgnoredTest(name) {
      testFun
    }
  }
}


What is critical to know is that this ctest() function, is not declaring any function/method in the test suite. It's just a function invoked in the suite's constructor,  interpreted in the order of listing in the file, expected to register the associated closure for invocation.

The normal test() function registers the test; the new ctest() function takes some extra detail text (logged @ info, and when I do something about reporting, may make it to the HTML), a condition for optional execution and that closure. There's also a suite-wide enabled() predicate. This is something which can be subclassed or mixed-in through a tratit for configurable execution. As an example. there are object store tests for both S3A and Azure, which are only enabled if the relevant credentials have been supplied.

Here's a more complex example


private[cloud] class S3aLineCountSuite
  extends CloudSuite with S3aTestSetup {

  init()

  override def useCSVEndpoint: Boolean = true

  def init(): Unit = {
    if (enabled) {
      setupFilesystemConfiguration(conf)
    }
  }

  override def enabled: Boolean = super.enabled

     && hasCSVTestFile

  ctest("S3ALineCountReadData",
    "Execute the S3ALineCount example") {
    val sparkConf = newSparkConf(CSV_TESTFILE.get)
    sparkConf.setAppName("S3ALineCountDefaults")
    assert(0 === S3LineCount.action(sparkConf, Seq()))
  }
}


The enabled predicate is true if the S3aTestSetup mixin was happy (that is, I've added credentials), and a CSV test file is defined. Then we run an example application, verifying
its exit code is 0.

Do I like this? Mostly.
 

Things I'm yet to explore


Async testing:

How to get the most out of scalatest 

(at least as far as I've learned so far)

  • Keep a strict ordering of (expected === actual). I use the same ordering as JUnit —I hope others are consistent there, as it's not so obvious in a test failure that things are different.
  • Add extra details in assert() clauses.
  • Embrace eventually()
  • Give every test case in a test suite a string which is at least partially unique. That way, you can run an individual test with the -Dsuite mechanism.
  • Try to make them fast. This really matters on spark, because there are a lot of tests to run.
  • If you are going near traits, have setup/teardown methods tagged as before {} and after{}, then use them through the traits consistently
  • Don't go too overboard in traits, even if you spent enough time in C++ to be perfectly comfortable with mixins and expect the maintainers to be happy with that too. As you can't make that latter expectation: use carefully.
  • Do try all the different spec mechanisms, rather than just stick with one because "it most reminds you of what you used to use". You could be missing out there
  • However: don't try all the different spec mechanisms wildly. Do try to be consistent with the rest of the project
Most of all: read the underlying source code. It's there. Look at what test(name) { clause } does and follow it through. See how tests are executed. Think about how to take best advantage of that in your test suites.

No comments:

Post a Comment

Comments are usually moderated -sorry.