CodeSOD: A Dropped Pass

A charitable description of Java is that it's a strict language, at least in terms of how it expects you to interact with types and definitions. That strictness can create conflict when you're interacting with less strict systems, like JSON data.

Tessie produces data as a JSON API that wraps around sensing devices which report a numerical value. These sensors, as far as we care for this example, come in two flavors: ones that report a maximum recorded value, and ones which don't. Something like:

 { dataNoMax: [ {name: "sensor1", value: 20, max: 0} ], dataWithMax: [ {name: "sensor2", value: 25, max: 50 } ] }

By convention, the API would report max: 0 for all the devices which didn't have a max.

With that in mind, they designed their POJOs like this:

 class Data { String name; int value; int max; } class Readings { List<Data> dataNoMax; List<Data> dataWithMax; }

These POJOs would be used both on the side producing the data, and in the client libraries for consuming the data.

Of course, by JSON convention, including a field that doesn't actually hold a meaningful value is a bad idea- max: 0 should either be max: null, or better yet, just excluded from the output entirely.

So one of Tessie's co-workers hacked some code into the JSON serializer to conditionally include the max field in the output.

QA needed to validate that this change was correct, so they needed to implement some automated tests. And this is where the problems started to crop up. The developer hadn't changed the implementation of the POJOs, and they were using int.

For all that Java has a reputation as everything's an object", a few things explicitly aren't: primitive types. int is a primitive integer, while Integer is an object integer. Integers are references. ints are not. An Integer could be null, but an int cannot ever be null.

This meant if QA tried to write a test assertion that looked like this:

assertThat(readings.dataNoMax[0].getMax()).isNull()

it wouldn't work. max could never be null.

There are a few different ways to solve this. One could make the POJO support nullable types, which is probably a better way to represent an object which may not have a value for certain fields. An int in Java that isn't initialized to a value will default to zero, so they probably could have left their last unit test unchanged and it still would have passed. But this was a code change, and a code change needs to have a test change to prove the code change was correct.

Let's compare versions. Here was their original test:

/** Should display max */assertEquals("sensor2", readings.dataWithMax[0].getName())assertEquals(50, readings.dataWithMax[0].getMax());assertEquals(25, readings.dataWithMax[0].getValue());/** Should not display max */assertEquals("sensor1", readings.dataNoMax[0].getName())assertEquals(0, readings.dataNoMax[0].getMax());assertEquals(20, readings.dataNoMax[0].getValue());

And, since the code changed, and they needed to verify that change, this is their new test:

/** Should display max */assertEquals("sensor2", readings.dataWithMax[0].getName())assertThat(readings.dataWithMax[0].getMax()).isNotNull()assertEquals(25, readings.dataWithMax[0].getValue());/** Should not display max */assertEquals("sensor1", readings.dataNoMax[0].getName())//assertThat(readings.dataNoMax[0].getMax()).isNull();assertEquals(20, readings.dataNoMax[0].getValue());

So, their original test compared strictly against values. When they needed to test if values were present, they switched to using an isNotNull comparison. On the side with a max, this test will always pass- it can't possibly fail, because an int can't possibly be null. When they tried to do an isNull check, on the other value, that always failed, because again- it can't possibly be null.

So they commented it out.

Test is green. Clearly, this code is ready to ship.

Tessie adds:

[This] is starting to explain why our git history is filled with commits that fix failing test" by removing all the asserts.

[Advertisement] Otter - Provision your servers automatically without ever needing to log-in to a command prompt. Get started today!