OpenHAB Broadlink Binding situation report

After #dadlife, #newjob and #otherstuff got in the way for a while last year, I got back into my role as maintainer of the OpenHAB Broadlink device binding. My first priority was to create a binding JAR that would actually work with the newly-published OpenHAB version 2.5. As the Broadlink binding is still not part of the official OpenHAB binding repository, it doesn't "automagically" get the necessary changes applied when the upstream APIs change. Luckily, it wasn't too much work.

My priorities for 2020 for this binding remain unchanged; get it to a high-quality state, make it pass the (extremely strict) linter guidelines for OpenHAB code, and get it merged into the official codebase. For me to consider it high-quality, there are still the following tasks to do:

• Get a solid chunk of it covered by unit tests to prevent regressions; and
• Redesign the device-discovery and identification areas of the code, to make adding new devices easier

Unit Tests

Prior to OpenHAB 2.5, bindings/addons that wished to define tests had to create an entire second project that shadowed the production code and could only be run via a strange incantation to Maven which did horrible OSGi things to run integration-style tests. Essentially, OpenHAB addons were not unit-testable by conventional means. Which given most addons are developed by unpaid volunteers, naturally meant that hardly any addons had tests.

Fortunately, one of the major changes in the 2.5 architecture has been a move towards more Java-idiomatic unit testing. Finally, classic JUnit-style unit testing with Mockito mocking will be available for fast, reliable testing within the binding. I'll be shooting for at least 60% test coverage before I'll consider submitting a PR to OpenHAB.

Discovery redesign

I've been told that new versions of some popular Broadlink devices will be arriving in 2020. In anticipation of that, I want to make it much easier to add a new device. At the moment it requires defining a new subclass of BroadlinkBaseThingHandler (which is par-for-the-course for OpenHAB, being a pretty standard Java app), but also adding "magic numbers" in a number of places to assist in looking-up and identifying devices during "discovery" and also when they boot up. I want to consolidate this such that everything needed to support a device is located within one .java file - i.e. adding support for a new device will require exactly two changes in Git:

• The new .java file containing all the required information to support the new device; and
• Adding a reference to this class somewhere to "pick it up".

I see no technical reason why this can't happen, and consider it only fair if maintenance of the binding will (at least partly) be a burden on the core OpenHAB team. So again, I won't be submitting this binding to become official until that work is complete.

Thanks for all the kind words from users/testers of this binding - it's very rewarding to hear people using it with great success!

Building a retro game with React.js. Part 3 - I Like To Move It

So with most of the graphical pieces in position, it's time to make things move around.

Again, starting with the easy stuff, I wanted the four directional keys to move the Player around. But in Frenzy, you can only move (as opposed to draw) along the boundaries of the game area and on lines you have already drawn. So if we look at my first iteration of the code in GameArea to handle a request to move the Player left, it's something like this:

 
update = () => {
  if (this.keyListener.isDown(this.keyListener.LEFT)) {
    this.moveLeft();
  }
};

moveLeft = () => {
  if (this.canMove(Direction.LEFT)) {
    this.setState({
       playerX : this.state.playerX -1
    });
  }
}

I ended up bundling quite a lot of smarts into the Direction enumeration in order to make the logic less "iffy" and more declarative. That one Direction.LEFT key encapsulates everything that is needed to check whether a) the player is on a line that has the correct orientation (horizontal) and b) there is room on that line to go further to the left.
A line looks like this:

[Orientation.HORIZONTAL, 0, 0, 478, 0], // startX, startY, endX, endY

and Direction looks like this:

export const Direction = {
  LEFT: {
    orientation: Orientation.HORIZONTAL,
    primaryCoord: (x, y) => y,
    lineToPrimaryCoord: (line) => line[2],
    secondaryCoord: (x, y) => x,
    testSecondary: (c, line) => c > Math.min(line[1], line[3])
  },
  ...
}

My test for whether I can move in a certain direction is:

static canPlayerMoveOnExistingLine = (playerX, playerY, direction, lines) => {
  const candidates = lines.filter(line => {
    return (line[0] === direction.orientation)
  });
    
  const pri = direction.primaryCoord(playerX, playerY);
  const primaryLines = candidates.filter(candidateLine => {
    return direction.lineToPrimaryCoord(candidateLine) === pri;
  });

  if (primaryLines.length > 0) {
    const sec = direction.secondaryCoord(playerX, playerY);
    const found = primaryLines.find(line => {
      return direction.testSecondary(sec, line);
    });

    return typeof found !== 'undefined';
  }
  return false;
} 

Declared static for ease of testing - easy and well worth doing for something like this where actually moving the player around is time-consuming and tedious. It's working well as it stands, although as we all know, naming things is hard. It's pretty easy to follow the process though. At this point I'm holding a lines array in this.state and doing filter and find operations on it as you can see above. We'll have to wait and see whether that will be fast enough. It may well be a good idea to keep a currentLine in state, as most of the time it will be unchanged from the last player movement. Next up, it's time to start drawing some new lines on the screen!

Kudos

I am starting to build up some tremendous respect for the original author of this game; although often dismissed as "very simple" there are some tricky little elements to coding this game and I'm only just scratching the surface. To achieve the necessary performance on an 8-bit, 1MHz processor with RAM measured in the handfuls of kilobytes is super impressive. Assembly language would have been necessary for speed, making the development and debugging a real pain. I haven't even started thinking about how to do the "fill" operation once a line has been drawn and it encloses some arbitrary space, but I suspect the original developer "sniffed" the graphics buffer to see what was at each pixel location - a "luxury" I don't think I'll have!

Time Tracking

Up to about 6 hours now.

The best feeling in software development?

I've been spending a lot of time in Javascript-land these days and while it's pretty exciting, I sure do miss some of the luxuries that really emphasise the maturity and elegance of statically-typed Scala.
Here's an example. It's incredibly-simple, but one of the most satisfying things in software development in my opinion. First, a quick definition:

trait Insertable {
  val insertedAt: Long
} 

And now, the source of all the joy:

def findMostRecentlyInserted(xs:Set[Insertable]):Option[Insertable] = {

}

Yep, that's it. An empty function. It won't even compile.

But this moment, right here, with type signatures locked down and the cursor flashing in the right spot, is where your brain can finally shift up from boilerplate mode into full power. How am I going to get from that set of candidates to the right one? What will be the fastest way? Will it read well? How should I test this?

If I've got my "good boy" hat on I'll stop here and write a few tests, which will of course fail. But sometimes, I'll just let the Scala compiler guide me - a function this simple is a great example of where strong typing really helps prevent errors. Anyone else with me on this?

Scala by Stealth part 2: Scala-powered tests

Testing from Scala

Now for the fun part - we get to write some Scala!

Now if may turn out that this ends up being the end of the Scala road at your shop, due to restrictive policies about production code. That's a shame, and it could take a very long time to change. I know of one large company where "Java for production code, Scala for tests" has been the standard now for several years. Sure it's not perfect, but it's better than nothing, and developers who haven't yet caught the Scala bug can learn it in their day job.

The tests you write may eventually be the only unit tests for this code, so I would strive for complete coverage rather than merely a copy of the "legacy" Java-based tests. For the purposes of measuring this coverage I can highly recommend the jacoco4sbt plugin which is simple to get going, well-documented and produces excellent output that makes sense in Scala terms (some other Java-based coverage tools seem to struggle with some of the constructs the Scala compiler emits).

In addition to (possibly) getting introduced to Scala and also learning the basics of writing a specs2 test, you might even discover that your code under test is a little tricky to test from this new perspective. This is a good thing, and if it encourages a bit of mild refactoring (while keeping both Java- and Scala-based unit tests passing of course) then so much the better.

Once you've got some solid, measurable coverage from the Scala side (I like to shoot for 90+% line coverage), it's time to commit those changes again, and push them to your CI build. If you haven't already, install the JaCoCo Plugin for Jenkins so you can get pretty coverage graphs on the project page, and even automatically fail the build if coverage drops below your nominated threshold(s).

Switching your Jenkins build project to SBT

Speaking of which, you'll be wanting to adjust your Jenkins job (or equivalent) to push your new, somewhat-Scala-ish artifact to your Nexus (or equivalent). Firstly, for safety, I would actually be duplicating the existing job and disabling it rather than getting all gung-ho with something that can potentially be a very carefully-configured, nay curated Jenkins project configuration.

Luckily this should be pretty straightforward if you employ the Jenkins SBT Plugin - set the Actions to something like clean jacoco:cover publish to get the optimal blend of cleanliness, test-coverage visualisation, speed, and build traceability.

If for any reason you can't use the plugin, I'd recommend using your CI tool's Run script functionality, and including a dead-simple shell script in a suitable place in your repository; e.g.:

#!/bin/bash
echo "Running `which sbt`"
sbt -Dsbt.log.noformat=true -Dbuild.version=$BUILD_NUMBER clean jacoco:cover publish

Once you've got everything sorted out and artifacts uploading, you'll notice that your Nexus now has a new set of artifacts alongside your old Java ones, with a _2.10 (or whatever Scala version you're running) suffix. Scala in your corporate repo! Progress!

Scala By Stealth, Part 1: SBTifying your Mavenized Build

I was faced with updating and extending some old Java code of mine recently, and it seemed like much more of a chore than it used to. The code in question does a lot of collection manipulation, and I was looking at the Java code (which was, if I say so myself, not too bad - clean, thoroughly-tested and using nice libraries like Google Guava where at all possible) thinking "ugh - that would be a couple of lines in Scala and way more readable at the same time".

At this point I realised it would be a perfect candidate for a step-by-step guide for converting a simple Maveni[sz]ed Java library project (e.g. resulting in a JAR file artifact) to an SBT-based, Scala library.

Shortly after that I realised this could be a terrific way for a traditional "Java shop" where everything up until now has been delivered as JARs (and/or WARs) into a private Nexus to get its feet wet with Scala without having to go with a risky "big-bang" approach. An iterative migration, if you will. So let's get started!

A tiny bit of background first though - I'm not going to bother anonymising the library I'll be migrating, because I will almost certainly forget to do so somewhere in the example snippets I'll be including. So I'll say it here: the library is called brickhunter, and it's the "engine" behind the web-scraping LEGO search engine you can use at brickhunter.net. The site itself is a Java/Spring MVC/JQuery webapp that I launched in late 2012, and was the last significant bit of Java I ever wrote. It includes brickhunter.jar as a standard Maven dependency, pulling it from my private Maven repo hosted by CloudBees.

Step 0 (A Precondition): A Cared-For Maven Java Project

You need to be doing this migration for a library that has redeeming qualities, and not one that suffers from neglect, lack of test coverage, or a non-standard building process. Generally, using Maven will have made the latter difficult, but if, somehow, weird stuff is still going on, fix that. And make sure your tests are in order - comprehensive, relevant and not disabled!

Step 1: An SBTified Java Project

• Create a new directory alongside the "legacy" project directory with a suitable name. For me, the obvious one was brickhunter-scala.
• Now recursively copy everything under src from legacy to new. Hopefully that gets everything of importance; if not, see Step 0 and decide what should be done.
• While a number of people have written helpers to automate the creation of a build.sbt from a pom.xml, unless you have a truly enormous number of dependencies, you're probably better-off just writing it yourself. For one thing, it's the obvious entry point to the enormous world of SBT, and there's plenty to learn;
• In a typical Maven shop you may have quite a stack of parent POMs bringing in various dependencies - I found the quickest way to get all of them into SBT style was by invoking mvn dependency:tree which for my project, gave me:

  [INFO] +- org.jsoup:jsoup:jar:1.6.1:compile
  [INFO] +- commons-lang:commons-lang:jar:2.6:compile
  [INFO] +- com.google.guava:guava:jar:11.0.1:compile
  [INFO] |  \- com.google.code.findbugs:jsr305:jar:1.3.9:compile
  [INFO] +- log4j:log4j:jar:1.2.16:compile
  [INFO] +- org.slf4j:slf4j-api:jar:1.6.4:compile
  [INFO] +- org.slf4j:slf4j-log4j12:jar:1.6.4:compile
  [INFO] +- com.themillhousegroup:argon:jar:1.1-SNAPSHOT:compile
  [INFO] +- org.testng:testng:jar:6.3.1:test
  [INFO] |  +- junit:junit:jar:3.8.1:test
  [INFO] |  +- org.beanshell:bsh:jar:2.0b4:test
  [INFO] |  +- com.beust:jcommander:jar:1.12:test
  [INFO] |  \- org.yaml:snakeyaml:jar:1.6:test
  [INFO] +- org.mockito:mockito-all:jar:1.9.0:test
  [INFO] \- org.hamcrest:hamcrest-all:jar:1.1:test
  

• Anything transitive (i.e. indented once or more) can be omitted as SBT will work that out for us just as Maven did.
• The eagle-eyed might notice an in-house dependency (argon) which clearly isn't going to be found in the usual public repos - it will need its own resolver entry in build.sbt.
• Here's how mine looked at this point:

name := "brickhunter-scala"

organization := "com.themillhousegroup"

version := "0.1"

scalaVersion := "2.10.3"

credentials += Credentials(Path.userHome / ".ivy2" / ".credentials")

resolvers += "tmg-private-repo" at "https://repository-themillhousegroup.forge.cloudbees.com/private/"

libraryDependencies ++= Seq(
  "org.jsoup"             % "jsoup"           % "1.6.1",
  "commons-lang"          % "commons-lang"    % "2.6",
  "com.google.guava"      % "guava"           % "11.0.1",
  "log4j"                 % "log4j"           % "1.2.16",
  "org.testng"            % "testng"          % "6.3.1"         % "test",
  "org.mockito"           % "mockito-all"     % "1.9.0"         % "test",
  "com.themillhousegroup" % "argon"           % "1.1-SNAPSHOT"  % "test"
)

• At this point, firing up SBT and giving it a compile command should be successful. If so, pat yourself on the back, and commit all pertinent files in source control. This is a good milestone!

Step 2: A Tested SBTified Java Project

• Compiling is all very well but you can't really be sure your SBT-ification has been a success until all the tests are passing, just like they did in Maven. They did all pass in Maven, didn't they?
• Here's where I hit my first snag, as my Java tests were written using the TestNG framework, which SBT has no idea how to invoke. And thus, the brickhunter-scala project gets its first plugin, the sbt-testng-interface.
• But now when running sbt test, instead of "0 Tests Found", I get a big stack trace - the plugin is expecting to find a src/test/resources/testng.yaml and I don't have one, because Maven "just knows" how to run a load of TestNG-annotated tests it finds in src/test/java, and I've never needed to define what's in the default test suite.
• The fix is to create the simplest possible testng.yaml that will pick up all the tests:

  name: BrickhunterSuite
  threadCount: 4
   
  tests:
    - name: All
      packages:
      - com.themillhousegroup.brickhunter
  

• And now we should have the same number of tests running as under Maven, and all passing. Commit all the changes!

Next time: Publishing the new artifact to your private repository.

Conditional SBT Keys for Jenkins Builds

A little SBT snippet that's proven useful (place this in a file like jenkins-only.sbt alongside your build.sbt):

    // Settings which should only be applied if this SBT is being run by Jenkins

    // Jenkins nodes always have $JENKINS_URL set - that is our telltale:
    lazy val underJenkins:Option[String] = scala.util.Properties.envOrNone("JENKINS_URL")
    def ifUnderJenkins[T](setting: => T):Seq[T] = underJenkins.map ( s=> setting ).toSeq


    // Write a JUnit-format output especially for Jenkins:

    testOptions in Test ++= ifUnderJenkins ( Tests.Argument("junitxml") )

Why: Well in this case, it gives Jenkins the output he/it needs to do pretty "Test Result Trend" graphs and have clickable error messages when tests fail, while simultaneously giving developers the well-structured, detailed test output that SBT gives by default.
How: As mentioned in the code comments, and in the Jenkins documentation, a Jenkins node will have the JENKINS_URL environment variable set, and a dev (probably) won't.
Nice Features:

• Exploits the fact that an Option[T] can be converted to a zero-or-one-length Seq[T] with toSeq()
• Parameterized ifUnderJenkins so that any SBT key can be handled conditionally
• By-name parameter (that's the setting: => T ) means the setting won't even be evaluated unless needed

Argument Capture with Scala / Specs2 / Mockito, part 2: The neater way

So in my last post, I indicated my dissatisfaction with the ugliness of creating a Mockito ArgumentCaptor in Scala - turns out someone else had the same thought, and it's already fixed in Specs2:

  "Application Main Controller" should {

    "Pass a non-empty list to the template" in {
      val result = controller.newsAction(FakeRequest())
      status(result) must beEqualTo(200)

      val captor = capture[List[String]]

      there was one(mockedNewsRenderer).apply(captor)
      val theArgument = captor.value
      theArgument.isEmpty must beFalse
    }
  }

Nice.

Using Mockito's ArgumentCaptor in Scala

I've been doing what an old colleague of mine used to call "Stack-Whoring" a bit recently - trying to get a few more badges and also up my general reputation on Stack Overflow. It's been fun and educational, and I wanted to post (mainly for my own reference) how to use Mockito's powerful ArgumentCaptor facility from inside a Specs2 example:

I'm just going to nab my own answer from the relevant SO question:

class ApplicationSpec extends Specification with Mockito {

  val mockedNewsRenderer = mock[List[String] => Html]

  val controller = new ApplicationController {
    override def renderNews(s:List[String]) = mockedNewsRenderer(s)
  }

  "Application News Controller" should {

    "Pass a non-empty list of news items to the template" in {
      val result = controller.newsAction(FakeRequest())

      status(result) must beEqualTo(200)

      val captor = ArgumentCaptor.forClass(classOf[List[String]])

      there was one(mockedNewsRenderer).apply(captor.capture())
      val theArgument = captor.getValue
      theArgument.isEmpty must beFalse
    }
  }
}

The trickiest bit (if you're already familiar with ArgumentCaptor usage in Java) is the use of classOf[T] to get the Scala equivalent of Java's .class so you can then call ArgumentCaptor.forClass().

There's probably scope there for a Pimp My Library tweak to tidy up that particular line - I'm thinking I'd like to just be able to say:

  val captor = ArgumentCaptor.for[List[String]]

Stay tuned ...

The elusive single FakeApplication specs2 test

As has been noted in numerous places, the Play 2 documentation on testing kinda suggests that spinning up a FakeApplication is something you can/should do in every single one of your test examples, e.g.:

  // Don't do this
class MyControllerSpec extends Specification {

  "My controller" should {

    "return a 404 on top-level GET to non-existent resource" in {
       running(FakeApplication()) {
         val home = route(FakeRequest(GET, "/mycontroller/blurg")).get
         status(home) must equalTo(NOT_FOUND)
       }
     }

     "serve up JSON on list request" in {
       running(FakeApplication()) {
         val home = route(FakeRequest(GET, "/mycontroller/list")).get
         status(home) must equalTo(OK)
         contentType(home) must beSome.which(_ == "application/json")
       }
     }
  }
}

Trust me when I say that it is not a good idea. At the very least slow tests, inconsistent test results and general Weird Things™ will happen.

What you most-likely want is something like this:

import play.api._
import play.api.test._
import org.specs2.specification._
import org.specs2.mutable._

/**
 * Mix this in to your Specification to spin up exactly one Play FakeApplication
 * that will be shut down after the last example has been run.
 * Override 'theApp' to use a customised FakeApplication
 */
trait FakePlayApplication {
  this: Specification =>

  def theApp = FakeApplication()

  def startApp = {
    System.err.println(s"Starting $theApp")
    Play.start(theApp)
  }

  def stopApp = {
    System.err.println(s"Stopping $theApp")
    Play.stop()
  }

  override def map(fs: => Fragments) = Step(startApp) ^ fs ^ Step(stopApp)
}

Which you could use in my previous example as follows:

class MyControllerSpec extends Specification with FakePlayApplication {

  "My controller" should {

    "return a 404 on top-level GET to non-existent resource" in {
         val home = route(FakeRequest(GET, "/mycontroller/blurg")).get
         status(home) must equalTo(NOT_FOUND)
     }

     "serve up JSON on list request" in {
         val home = route(FakeRequest(GET, "/mycontroller/list")).get
         status(home) must equalTo(OK)
         contentType(home) must beSome.which(_ == "application/json")
     }
  }
}

Less repetition, faster execution, and most importantly, RELIABLE TESTS!

Missing Jenkins Feature?

I really like Jenkins. If you are disciplined and treat him well, he gives you a tremendous amount of building power for the princely sum of $0.00. I've used Bamboo a little bit and it was fine, but I really can't see the value proposition when Jenkins does the same job and offers over 600 plugins to do just about everything you could possibly think of with a software build.

One thing that does seem to be missing (as far as I can tell) from Jenkins is "smart" detection of the current build killer. A colleague of mine spotted this today. Of course, this is a situation that shouldn't happen if people did the right thing and didn't check in on a broken build, but hey, we're all human. So here's the normal break-fix situation:

User adolf commits bad code
Build runs and breaks (3 test fails)
adolf correctly identified as breaker
	User bob commits good code into broken build
	Build runs and breaks the same way (3 test fails)
	adolf remains correctly identified as breaker
		User caz commits fixed code/tests
		Build runs and passes
		Build is green, huzzah for caz

Note that of course the items in the 3 columns don't necessarily occur perfectly sequentially as shown above. In fact, a far more common mode of failure would be an "innocent mistake" from bob who didn't realise a build was in progress when he pushed his changes, viz:

User adolf commits bad code
Build runs and breaks (3 test fails)	User bob commits good code
adolf correctly identified as breaker	Build runs and breaks the same way (3 test fails)
	adolf remains correctly identified as breaker
		User caz commits fixed code/tests
		Build runs and passes
		Build is green, huzzah for caz

OK. Now for the not-so-great, but definitely not-too-uncommon "firestorm of crap checkins" - this often actually happens when builds are unstable and/or slow and so people have almost no choice but to Russian-Roulette their pushes:

User adolf commits bad code
Build runs and breaks (3 test fails)	User boris commits MORE bad code
adolf correctly identified as original breaker	Build runs and breaks even more (4 new test fails, 7 test fails total)
	adolf remains correctly identified as original breaker
		User caz commits fixed code/tests for adolf's breakage
		Build runs but 4 tests still fail (boris' breakage)
		Build is still red, but still identifies adolf as the culprit (incorrectly)

Because the build never fully recovered, Jenkins just leaves the original culprit as the bad guy. Of course, a little bit of further investigation will show that boris is now to blame, but it would be much better (especially if your name is up on a Big Screen of Shame that Important People might see!) if Jenkins could calculate the true culprit correctly.

New Jenkins plugin, anyone?

Make Sure You're Testing The Right Things

I'm going to come right out and say it. I love Unit Tests.

I love mocking the collaborators with Mockito (especially the annotation-driven mode). I love the small but oh-so-valuable mini-refactors that are sometimes necessary to make a class testable. And I love watching the code coverage march inexorably to the 100% asymptote.

What makes me very sad is seeing a sorry set of unit tests, and/or a poor development environment for testing. What do I mean?

• If you're not mocking your collaborators, you're not unit testing. And if you're not unit testing, your code is Instant Legacy Code™. You cannot properly test any non-trivial class without needing to mock its dependencies.


• Tests should be named in terms of expected behaviour. Method name testParseIntNullString() says nothing that couldn't be gleaned from looking at the test code itself. Method name shouldThrowFormatExceptionWhenParsingNullString() tells you the intention of both the code under test and the test itself. It's documentation that won't get out of sync with the code.


• Use the facilities provided by your testing package. I'm a TestNG fan, but recent JUnit versions are pretty good too. I use TestNG's groups option in the @Test annotation to specify the style and/or scope of each test method. Another good one is the expected or expectedException annotation option that states what is going to be thrown. Much, much tidier than having try..catch blocks in test methods.


• Code coverage is more than just a percentage. I use the eCobertura Cobertura plugin for Eclipse while I'm writing tests - to visually indicate which parts of the code I'm actually hitting. I've lost count of the number of times it's shown me an if I thought I had covered is still "red" due to a missing precondition.

Unit-Testing Time-Oriented Code

Part 2: Are We There Yet?

The counterpoint to actively forcing the current thread to wait is checking whether a certain amount of time has elapsed. Unit-testing code like this is a similar challenge; we certainly don't want to have to sleep() in our unit tests in order to verify our code - we have better things to do with our (and our processor's!) time.

Let's look at a typical (if contrived) bit of time-elapsed-measuring code:

  public void sendRequest(Packet packet, int slowThresholdMillis) 
                throws ExcessiveLatencyException {
    final Date sendTime = new Date();
    networkInterface.sendAsynchronously(packet, new AckCallback() {
      public void ackReceived() {
        Date ackTime = new Date();
        long timeTaken = ackTime.getTime() - sendTime.getTime();
        if (timeTaken > slowThresholdMillis) {
          throw new ExcessiveLatencyException(timeTaken);
        }
      }
    }
  }

Now clearly we need to be able to cover that "took too long to respond" path in our unit tests. Of course we could pass in a negative value of slowThresholdMillis but that's not really a realistic situation, and indeed a requirement will probably come along to validate against non-negative and non-zero values for that parameter.

Again the solution is to refactor out the parts which are currently hard to unit test. As a side-effect we'll probably get a much cleaner sendRequest() method - I've certainly never liked seeing all that millisecond-twiddling sitting alongside higher-level abstractions like networkInterface.sendAsynchronously()!

What we need is commonly known as a Stopwatch component, and given there are several proven implementations to choose from, there's absolutely no need to re-invent the wheel here.

Let's go with the Spring implementation because we've almost certainly already got it in our classpath:

public class RequestSender {
  private static final StopWatch DEFAULT_STOPWATCH = new StopWatch();
  private final StopWatch sw;
  
   public RequestSender() {
    this.sw = DEFAULT_STOPWATCH;
  }

  /** Package-visible for unit testing */
  RequestSender(StopWatch stopWatch) {
    this.sw = stopWatch;
  }

  public void sendRequest(Packet packet, int slowThresholdMillis) 
                throws ExcessiveLatencyException {
    sw.start();
    networkInterface.sendAsynchronously(packet, new AckCallback() {
      public void ackReceived() {
        sw.stop();
        if (sw.getLastTaskTimeMillis() > slowThresholdMillis) {
          throw new ExcessiveLatencyException(sw.getLastTaskTimeMillis());
        }
      }
    }
  }

Notice how a minor refactor for testability has already improved the code - we've saved a line and delegated off some drudgery to a reliable third-party library. I've gone for a constructor-injected approach here for two reasons, namely that having a StopWatch is essential to the correct operation of the class - and I would say the same for the (elided) networkInterface collaborator, and secondly, it gives a straightforward way to ensure that sw is final so I can use it in the ackReceived() callback. All that remains now is to mock the StopWatch implementation in our unit tests so we can exercise all the paths through the code:

public class RequestSenderTest {
  private RequestSender testInstance;
  @Mock
  private StopWatch mockStopWatch;

  @BeforeMethod(groups="unit")
  public void setup() {
    MockitoAnnotations.initMocks(this);
    testInstance = new RequestSender(mockStopWatch);
  }

  @Test(groups="unit", expectedExceptions=ExcessiveLatencyException.class)
  public void checkLargeLatencyThrowsException() {
    Mockito.when(mockStopWatch.getLastTaskTimeMillis()).thenReturn(200);
    testInstance.sendRequest(mockPacket, 100);
  }
}

Unit-Testing Time-Oriented Code

Part 1: Sleepy Time

Every now and again, we come across a need to write code that is time-sensitive. It could be as simple as waiting a certain period before timing-out a request, putting a pre-delay in an autocomplete control, sending a rapid-fire chatter of calls, or measuring performance of some other aspect of the system. In all these cases, unit testing can be a real pain.

We all want our unit tests to run as quickly as possible; if it takes longer than a couple of tens of seconds to run, we'll potentially lose focus, switch to browsing the web and get distracted, or worse yet, not run the tests at all. But if the unit tests for our time-sensitive classes take real-time amounts of time to run, we could be looking at enormously time-consuming test runs (imagine running a suite of 10 or more tests where "timeout after 30 seconds" is the expected outcome!)

The solution is of course to somehow mock out the code that requires or takes time to execute. In effect, we want a mock that lets us "fast-forward" time. Let's first look at the classic naïve way to "wait a bit" - sleep:

  public void sendPings(int numberOfPings, int delayMillis) {
    for (int i=0; i<numberOfPings; i++) {
      sendPing();
      waitForMillis(delayMillis);
    }
  }

  private void waitForMillis(int millisToWait) {
    try {
      Thread.sleep(millisToWait);
    } catch (InterruptedException e) {
      Thread.currentThread().interrupt();
    }
  }

How can we fully test the sendPings() method without incurring the time cost of waiting between pings? Well, we could of course only test with very small values of delayMillis, but that's still going to take a bit of time. Let's look at our overall aim for unit-testing sendPings:

• We'd like to mock out the actual pinging mechanism so we can count how many times it was invoked - hence checking our for-loop logic
• We'd like to verify that the sleeping mechanism is correctly told to sleep for delayMillis
• We can pretty-much trust that the Java sleep() method works as advertised
• We don't want to actually wait numberOfPings times delayMillis milliseconds in order to see if a certain combination works

What we need to do is classic Refactoring, classic Clean Code, classic SRP, classic Design Patterns, classic OO, you name it. We extract the waiting mechanism into a class that is solely concerned with waiting:

  public interface WaitProvider {
    void waitForMillis(int millisToWait);
  }

  public class ThreadSleepingWaitProvider implements WaitProvider {
    public void waitForMillis(int millisToWait) {
    try {
      Thread.sleep(millisToWait);
    } catch (InterruptedException e) {
      Thread.currentThread().interrupt();
    }
  }

So now the interesting bits of our pinging class under test look something like this:

public class PingSender {
  private static final WaitProvider DEFAULT_WAIT_PROVIDER =
    new ThreadSleepingWaitProvider();
  private WaitProvider waitProvider = DEFAULT_WAIT_PROVIDER;

   public void sendPings(int numberOfPings, int delayMillis) {
    for (int i=0; i<numberOfPings; i++) {
      sendPing();
      waitProvider.waitForMillis(delayMillis);
    }
  }
  
  void setWaitProvider(WaitProvider newWP) {
    this.waitProvider = newWP;
  }
}

Thus, our default behaviour is still to use the Thread.sleep() mechanism, but we can substitute a mock WaitProvider to speed up our unit tests. We just use the package-visible setter:

public class PingSenderTest {
  private PingSender testInstance;
  @Mock
  private WaitProvider mockWaitProvider;

  @BeforeMethod(groups="unit")
  public void setup() {
    testInstance = new PingSender();
    MockitoAnnotations.initMocks(this);
    testInstance.setWaitProvider(mockWaitProvider);
  }

  @Test(groups="unit")
  public void checkDelayIsPropagatedToWaitProvider() {
    testInstance.sendPings(3, 333);
    Mockito.verify(mockWaitProvider, Mockito.times(3)).waitForMillis(333);
  }
}

And so on and so on ... Note that I am not suggesting for a minute that the unit tests using the mock WaitProvider are the only testing that should be done on PingSender. There should be a full set of integration tests which use the "default" provider with reasonable (if short) delays specified, and possibly even a further set (perhaps annotated appropriately) that use relatively large delayMillis values.

In the next installment, we'll look at unit testing the situation where rather than waiting for a set time, we need to test whether a certain amount of time has elapsed.