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!

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 ...

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.

SotSoG 2010 Hall of Fame

Standing on the Shoulders of Giants

When I first moved back from The Dark Side to the world of Java, I was astonished at the vibrant software ecosystem that had grown up around Sun's baby. When I left the Java world at the end of the 20th century, the JDK 1.2 JARs were pretty-much all I knew.

Cut to 2005 and The Apache Project has gone ballistic, churning out libraries for just-about everything and an industry-standard servlet container. There are more app servers, JDBC drivers, XML parsers and webapp frameworks than be conceived by a sane mind, and SourceForge is hosting literally thousands more useful Java projects.

Some of the projects we all take for granted in the Java world represent many thousands of development and testing hours. Truly we stand on the shoulders of giants when our own projects succeed thanks to their hard work and generosity.

Some projects more than others really make Java development a pleasure, and hence I'm calling them out for the inaugural Millhouse Group SotSoG Awards (actual prize is nothing except a warm fuzzy feeling for those involved...)

• The Spring Framework - Still the defacto IoC framework of choice, but that's really just the tip of the iceberg...
• Apache Maven - Yes, it enforces a standard project layout. And that's the way I like it.
• Eclipse IDE - A hulking great beast of an IDE. Plugins for stuff that hasn't even been thought of yet. And still totally free.
• Apache Tomcat - Still my choice, and getting better every release.
• Mockito - a lovely, fluent mocking API that itself stands on the shoulders of EasyMock!

All of these projects deserve their own blog posts, and will get them in time. But in the meantime, if you're not familiar with any of the above, Google them up and give 'em a go! Enjoy the view.