tr ouwens

by the way: things I want to say

Get to grips with your build, with Gradle

Last month, the Dutch Java Magazine published an article written by Hanno Embregts and myself. In it, we describe how we used Gradle in our project at the Dutch Railways.

It’s a big, monolithic project with ± 25.000 lines of Ant scripts, which we wanted to break up into separate components that can be built and released separately. Doing this with Ant would add more complexity to an already much too complex system of scripts. Maven wasn’t a good fit either, because being built with Ant since its inception, our project didn’t follow Maven’s conventions, and shoe-horning it into these conventions would be impractical. Gradle provided the right mix of structure and flexibility.

We started our migration on a small subcomponent with no dependencies to other parts of our codebase, to work our way up. That way we wouldn’t have to deal with deploying the moving target of our project, which was actively in development during the migration, into Nexus so our Gradle scripts could consume it. We tried it; it didn’t work. It’s much easier to go the other way round, to consume artifacts from Nexus that were already migrated to Gradle and put under a strict regimen of Semantic Versioning.

Still, our existing Ant scripts needed to deal with our new Gradle-built artifacts. We decided it would treat these like any other 3rd party dependency, which worked fine for the most part. However, we found we had to be careful that the content of artifacts was identical to the ones built by the original Ant scripts, especially in the case of MANIFEST.MF files and various kinds of XML files embedded in the JARs. We had tools to test this, and we even went so far as to change Gradle’s unit test output layout to match that of Ant, so we could compare those easily using a diff viewer.

In the end, we were very happy with the result, even if it took more effort than we expected initially. If you want to know more about the reasoning behind some of the choices we made, or how we solved some of the other issues we ran into, you can read the full article in the physical magazine, or you can read it online. Note: it’s in Dutch.

Foobal, Moneyball

Ever since Moneyball came out, winning at sports using statistics has become more and more mainstream. As I’ve blogged before, I have also been getting in on some of that action: I’ve been competing in my family’s soccer match betting game, where we guess the results of NAC Breda, our favourite team. But instead of predicting the outcomes myself, I’m using Foobal, a little Scala program that I’ve written for this very purpose.

And, like the Oakland A’s, FC Midtjylland and Brentford FC, this has not been without success: I won this year’s pool! And by quite some margin, too. Here’s the trophy:

Trophy

On the last few lines, it says J10 anO15 which really should have been 2015 JanO. I guess the thick black line got in the way :). I’ll keep it until next year, when I pass it on to the next winner.

Anyway, Foobal probably did so well because NAC has been playing very, err… consistently this season. Sadly, due to this consistency, NAC were also relegated from the Honor Division to the First Division. I must say, it’s sad to win the pool under such sad circumstances. To Foobal’s moral credit, though, it did predict a win for the final and deciding match, which was lost during extra time.

The relegation also means that next year, NAC will have to face many teams that Foobal does not have data on. This will be quite a challenge, but it gives me a good opportunity to tweak the algorithm a bit and see if I can come up with something even more successful. I’m only getting started!

Hacking Java enums

The other day, I was debugging some enum related code in EqualsVerifier. I had this enum:

enum CallMe { YES, NO, MAYBE }

And two variables, original and clone, containing a value of said enum. Here’s what the bug looked like in Eclipse’s debugger:

Call Me Maybe

So, what do we see here? We see two variables of type EnumHack$CallMe (the enum was an inner class, so that makes sense). Both enums have the same name and ordinal, so they are equal. They also have different ids (33 and 34, respectively), so they’re not the same object.

Wait, what!?

That’s right: two different instances of the same enum constant. In other words, original.equals(clone) returns false, even though both variables are set to CallMe.MAYBE. How is that even possible? I thought it wasn’t. In the words of Joshua Bloch:

This approach [of using a one-element enum to implement a singleton, JO] provides an ironclad guarantee against multiple instantiation, even in the face of sophisticated serialization or reflection attacks.

The Java Language Specification, section 8.9 says this:

An enum type has no instances other than those defined by its enum constants. It is a compile-time error to attempt to explicitly instantiate an enum type (§15.9.1).

The final clone method in Enum ensures that enum constants can never be cloned, and the special treatment by the serialization mechanism ensures that duplicate instances are never created as a result of deserialization. Reflective instantiation of enum types is prohibited. Together, these four things ensure that no instances of an enum type exist beyond those defined by the enum constants.

Clearly, despite the fact that an enum constant can never be cloned, I had a clone on my hands.

So what happened? I traced the problem back to this:

@Test
public void hackAnEnum() throws Exception {
    CallMe original = CallMe.MAYBE;
    CallMe clone = ObjectAccessor.of(original).copy();
    
    assertEquals(original.name(), clone.name());
    assertEquals(original.ordinal(), clone.ordinal());
    assertFalse(original.equals(clone));
    assertFalse(original == clone);
}

I added the asserts so you can see for yourself what’s going on: if you copy/paste this to your IDE and put EqualsVerifier on the classpath, you’ll be able to run it. This test passes, which means that both original.equals(clone) and original == clone are, indeed, false.

So what does this ObjectAccessor do? It’s part of EqualsVerifier’s reflection library, and as you probably guessed, it makes a copy of the given object. If I factor out all EqualsVerifier code, I end up with this:

CallMe clone = new ObjenesisStd().newInstance(CallMe.class);
for (Field f : Enum.class.getDeclaredFields()) {
    f.setAccessible(true);
    f.set(clone, f.get(original));
}

Apart from the reference to ObjenesisStd, this is all standard Java reflection code. So, what is this Objenesis thing, then? From their website:

Objenesis is a small Java library that serves one purpose: To instantiate a new object of a particular class.

In other words, it can instantiate any object, without calling its constructor. So how does Objenesis work, exactly? Well, that depends. It uses the Strategy pattern to choose from several different ways of instantiating objects, depending on what kind of JVM you’re running, and probably some other factors, too. In my case, it expanded to this:

Constructor<Object> objectConstructor =
    Object.class.getConstructor((Class[]) null);
Class<?> reflectionFactoryClass =
    Class.forName("sun.reflect.ReflectionFactory");
Method method = reflectionFactoryClass.getDeclaredMethod("getReflectionFactory");
Object reflectionFactory = method.invoke(null);
Method newConstructorForSerializationMethod =
    reflectionFactoryClass.getDeclaredMethod("newConstructorForSerialization", Class.class, Constructor.class);
Constructor<CallMe> ctr = (Constructor<CallMe>)
    newConstructorForSerializationMethod.invoke(reflectionFactory, CallMe.class, objectConstructor);
CallMe clone = ctr.newInstance((Object[]) null);

That’s some incredibly hairy scary code. Let’s pretend we never saw this. The only thing we need to remember is that all this can be done without resorting to actually changing the bytecode at runtime; it’s all reflection.

While Objenesis maybe a relatively unknown library, it is actually pretty widely used. The most famous libraries that use it are Mockito (and basically all mocking frameworks), and Spring Framework. But there are more. Many more. EqualsVerifier uses it to instantiate values for the fields of the class it’s testing.

OK, now we that know this, can we go one further? It turns out we can:

Call Me Sometime

We can add our own enum constants. Here’s how:

CallMe sometime = new ObjenesisStd().newInstance(CallMe.class);
Field ordinal = Enum.class.getDeclaredField("ordinal");
ordinal.setAccessible(true);
ordinal.set(sometime, 4);
Field name = Enum.class.getDeclaredField("name");
name.setAccessible(true);
name.set(sometime, "SOMETIME");

It’s actually pretty simple. I guess the JVM’s guarantees aren’t ironclad enough for this particular sophisticated reflection attack. And to think I actually found it by accident! I fixed the bug in EqualsVerifier before it ever got released into production, so all’s well that ends well, I guess.

P.S. Oh and don’t try this at home kids! …or at least, not in production.

Configuring my NAS

The other day, I had to re-install my NAS, and of course, I’d forgotten exactly how I had configured it. That meant finding back lots of websites, some of which apparently didn’t exist anymore. This time, I’ve written down all the steps I’ve taken, just in case, for some sad, unforeseen reason, I ever have to do it again.

What do I want from my NAS? Not much, actually:

  • I want it to store my files (duh)
  • I want to use an rsync script to copy my photos to my NAS
  • I want filenames with special characters (éüå) to be treated sanely by said rsync script
  • I want it to run CrashPlan so all my data is automatically backed up to the cloud

I don’t need it to be a media server; I have Spotify and Netflix for that.

My NAS is a Synology DS213j running DSM 5.1.

Starting out

After choosing a server name, a user name and a password, make sure to first install all the updates from the Control Panel. Add the user account to the administrators group, then disable admin and guest account. Next, create a shared folder: in File Station, click Create, Create New Shared Folder. Give it a name and click OK. Give your user Read/Write permissions to the folder.

Set up SSH

Once the preliminaries are out of the way, you can set up SSH, which is needed for rsync.

  • Go to Control Panel, “Terminal & SNMP” and check “Enable SSH service”
  • Go to Control Panel, User, Advanced, and check “Enable user home service”. This is needed to be able to log in as a different user than root later.

You now have root SSH access though the Admin account. (You disabled that account earlier, but root is still active, and its password is the same as the NAS’s admin account’s password.) Let’s try it from the command-line: ssh root@diskstation. You have to type ‘yes’, then the admin password, and you’re in. Nice.

You want to be able to log in using the new user account you defined earlier. From a root SSH session:

  • Open /etc/passwd with vi.
  • Find the line for your user, and at the end of that line, replace /sbin/nologin with /bin/sh.
  • Run chmod u+s /bin/busybox, or else you won’t be able to gain root access later. (You’ll get errors like “must be suid to work properly” when you try to run su.) In fact, I’ve lost root once after an update because the permissions of this file were reset, so I invite you to add it to the crontab as well:
  • vi /etc/crontab, then add the line 0 0 * * * root chmod u+s /bin/busybox. That will restore the permission every day at midnight.
  • Restart the diskstation to make sure that the new crontab is loaded. You may want to test this to convince yourself it works, before you proceed, because if something goes wrong, you’ll be locked out of your NAS.

Now, you really should disable direct root access. Perform the following steps carefully, or you might get locked out of root forever.

  • vi /etc/ssh/sshd_config and change the line #PermitRootLogin yes to PermitRootLogin no.
  • Let’s also change #MaxAuthTries 6 to MaxAuthTries 3.

If you want to beef up security a bit more with SSH keys, read this article.

Enable special characters

Not all Synology diskstations support UTF-8 on the terminal, which is where you want to run rsync. So if you have files or folders with non-ASCII characters in them, you may want to make sure that your diskstation does support them, or else you’ll run into trouble later.

SSH into your diskstation and type locale. If it looks like this, you’re good:

LANG=en_US.utf8
LC_CTYPE="en_US.utf8"
LC_NUMERIC="en_US.utf8"
LC_TIME="en_US.utf8"
LC_COLLATE="en_US.utf8"
LC_MONETARY="en_US.utf8"
LC_MESSAGES="en_US.utf8"
LC_PAPER="en_US.utf8"
LC_NAME="en_US.utf8"
LC_ADDRESS="en_US.utf8"
LC_TELEPHONE="en_US.utf8"
LC_MEASUREMENT="en_US.utf8"
LC_IDENTIFICATION="en_US.utf8"
LC_ALL=en_US.utf8

I.e., en_US.utf8 should be everywhere. If it’s not, go to this blog post and follow the instructions to fix it. It’s a bit of work, but it’s worth it, I promise.

Enable rsync

  • Go to the diskstation’s main menu and start the app “Backup & Replication”.
  • Go to the Backup Services tab.
  • Check “Enable network backup service”.
  • Click Apply.

Now you can rsync from the pc to the NAS like this (assuming your shared folder is called documents):

rsync --delete -av --exclude ".DS_Store" --exclude ".fseventsd"
      --exclude ".Spotlight-V100" --exclude ".TemporaryItems"
      --exclude ".Trashes" --exclude "@eaDir"
      /some/directory/ user_name@diskstation:/volume1/documents

Install CrashPlan

  • Open the Package Center’s Settings, and change the Trust Level to “Any publisher”.
  • Just follow Scott Hanselman’s instructions. He explains it much better than I could. Note that, if you’re on a Mac, CrashPlan’s configuration file can be found in /Applications/CrashPlan.app/Content/Resources/Java/conf/ui.properties

The CrashPlan client prevents the diskstation from going into hibernation mode. Therefore, it may be a good idea to schedule it to run only for a few hours a day. Log in via SSH and gain root via su. Then vi /etc/crontab and add the following two lines:

0 1 * * * root /var/packages/CrashPlan/scripts/start-stop-status start
0 5 * * * root /var/packages/CrashPlan/scripts/start-stop-status stop

That starts up the service at 01:00 and shuts it back down at 05:00.

Wrapping up

Restart the diskstation one final time, to make sure all settings have taken effect.

That’s it! We’re done.

It's just test code

“It’s just test code” is something I hear way too often, from people who really should know better. Somehow, there is a pervasive sentiment that unit test code does not need to be held to the same standards as production code. I’m not sure why, but it seems to be associated with the idea that the “customer” will never “see” that code.

This kind of thinking is dangerous. As your code base grows, badly maintained test code will slow you down at least as much as badly maintained production code does.

I’ve seen tests that were so terse that I couldn’t figure out what they were testing. I’ve seen tests that were so long and convoluted that I couldn’t figure out how to fix them after they broke when I changed something in the production code. I’ve seen suites of tests that were so badly organized that I couldn’t figure out where to find the tests for a specific feature. All of these were written by people who are paid to write this stuff. Some of them were even me!

For example, EqualsVerifier’s test suite had devolved over the years into a state that it was virtually unmaintainable. Fixing bugs had become very hard and time-consuming. This was not because the production code was hard to understand; I’d kept that sufficiently well-factored. No, it was because it had become increasingly hard to fix the existing tests after a change, and to find a good location for a new test. When I did a training with Michael Feathers last year, I asked him how I could best fix this. His advice was basically to start over from scratch.

I did not start over from scratch. I did, however, spend several weeks cleaning up and heavily refactoring the test code. Here are some things I’ve learned from that experience, and also from experience on other projects. I’ll use a running example of classes modeling a card game.

Test naming

This is the most important thing. You might understand what a test is supposed to test when you’re writing it, but will you still understand it in three months? Will somebody else?

For example, one of your test methods might be named testShuffle(). First of all, we’re not living in the nineties anymore. With JUnit 4’s @Test, there’s no need to prefix all your test methods with the word test. It’s just clutter, so get rid of it. Second of all, I can see that this test probably performs a test on the shuffle() method. But what does it test, exactly? The happy path? If so, which happy path? Maybe it even tests all possible interactions with this method? If so, it’ll be quite a long test method, I suppose.

Another example of a badly named test is shuffleFails(). Why does it fail? And why is it supposed to? How am I supposed to know? I could look at the code, but even that might not tell me everything I need to know. It might be specified behavior, or an observation of a known bug that I want to change later.

For EqualsVerifier, I’ve started using a modified “given-when-then” style for naming my test methods. I reverse the order to “then-when-given”, and I often leave out the “given” or “when” part when I don’t need them. I like to start the name with the method I’m exercising (if appropriate), and I separate the parts with an underscore for extra readability. For example: shuffleReshufflesTheDeck_givenAPreShuffledDeck(), dealGivesThreeCards_whenRequestingThreeCards_givenAShuffledDeck(), dealThrowsIllegalStateException_whenCalled_givenAnUnshuffledDeck().

Yes, those are long Java names, but it beats having a shorter name and putting the “given-when-then” description in a comment for several reasons. First of all, you won’t have to invent a short name for your test in addition to the long “given-when-then” name. Naming things is hard, so this is one less thing you need to name. Secondly, the method name is picked up by your test runner, while any comments are not. So if a test fails, you immediately know what’s up; you don’t need to go to the test to read the “given-when-then”.

In languages and/or framworks that support it, I always prefer a string over an identifier to name my tests. For example, in ScalaTest you can also write:

"shuffle" should "reshuffle the deck given a pre-shuffled deck" in {
  // ...
}

Obviously, this is much more readable than a camel-cased Java identifier.

Magic numbers

Many people believe magic numbers are OK in unit tests. I think, however, that introducing constants can be very beneficial in making a test more understandable. For example:

@Rule ExpectedException thrown = ExpectedException.none();

@Test
public void dealThrowsISE_whenAsking79Cards() {
    thrown.expect(IllegalStateException.class);
    deck.deal(79);
}

@Test
public void dealThrowsISE_whenAskingTooManyCards() {
    thrown.expect(IllegalStateException.class);
    deck.deal(NUMBER_OF_CARDS_IN_A_DECK + 1);
}

Not everybody might immediately recognize that 79 is not some aribitrarily large number, but that it is, in fact, precisely one more than the number of cards in the French game of Tarot. In the second example, it’s immediately clear what’s going on. You don’t even need to see the actual value for NUMBER_OF_CARDS_IN_A_DECK.

Here’s another example:

@Test
public void dealGivesTheExactNumberOfCardsAsked_whenAskingSomeNumber() {
    List<Card> hand = deck.deal(SOME_NUMBER);
    assertThat(hand.size(), is(SOME_NUMBER));
}

@Test
public void dealGivesAFullHandOfCards_whenAskingTheNumberOfCardsInAHand() {
    List<Card> hand = deck.deal(NUMBER_OF_CARDS_IN_A_HAND);
    assertThat(hand.size(), is(NUMBER_OF_CARDS_IN_A_HAND));
}

The difference may be subtle, but in the first case, I don’t care about the specific number as long as it’s the same in both instances, whereas in the second one, I do care about the specific number. The first test looks like it’s part of a test suite for a generalized CardDeck class, while the second one might be specific to a particular game.

Note that magic numbers aren’t necessarily numbers; they can be Strings or even entire objects too. When I don’t care about the specific magic number, I like to prefix it with the word “Some”.

Refactor

As stated above, I’ve seen single test methods that were over twenty lines in length. They often contain comments which may or may not have gotten stale, lots of initialization code that may or may not be copy-pasted over several different test methods in slightly modified forms, and lots and lots of assertions checking tiny little fields on a big object. When one of these tests fails, or you’ve made a change in the production code that requires the test to change as well, it might take a very long time to figure out what’s going on in that test, what it’s actually doing, and what it should be doing. Also, if one assert fails, you don’t know in advance how many of the following asserts in the method will fail as well. You don’t have all the information that you should be having.

In other words, these tests tend to be very brittle, leading to lots of frustrating time wasted in maintaining them. So how to fix this smell?

It should be obvious, really. Test code is code, so you can refactor it. Move initialization code to an @Before set-up method, or to a private helper method. Give the helper method parameters if you have different flavors of initialization. Big assert blocks can be moved to a helper too. So instead of this:

    assertThat(hand.get(0).getValue(), is(Value.ACE));
    assertThat(hand.get(0).getSuit(), is(Suite.SPADES));

You should have something like this:

    assertCard(hand.get(0), Value.ACE, Suite.SPADES);

// ...

private void assertCard(Card actualCard, Value expectedValue, Suite expectedSuite) {
    assertThat(actualHand.getValue(), is(expectedValue));
    assertThat(actualHand.getSuit(), is(expectedSuite));
}

However, as I said, you shouldn’t be asserting too much in a single test, because you’ll lose information. If the first assert fails, you no longer know what the rest will do. It might be the difference between a typo and a major design flaw and you won’t know until you’ve fixed the first assertion and the second one can be executed, and the third, etc. But when you refactor this test into several smaller ones, you’ll immediately see how big your problems are just by the number of tests that are failing.

By refactoring mercilessly, your test methods will become shorter and clearer. It will become easier to understand what a test does, and it will become easier to maintain a whole suite of tests, because if something changes in the initialization, there’s now only one place where you need to make that change, instead of lots of different individual test methods.

Additionally, remember those comments that may or may not have gotten stale? If you’ve named your helper methods well, you probably don’t need those comments anymore. Just sayin’.

This is what I mean when I say that test code is code, too. You don’t tolerate excessive copy-pasting in production code; why would you tolerate it in test code?

Arrange, act, assert

This is the same thing as “given-when-then”, and it’s a good way to organize your test method. Consider:

@Test
public void dealGivesKnownCards_whenShufflingWithAKnownSeed() {
    Random rand = new Random(TEST_SEED);
    CardDeck deck = new CardDeck(rand);
    assertThat(deck.size(), is(52));
    deck.shuffle();
    List<Card> hand = deck.deal(2);
    Card one = hand.get(0);
    assertCard(one, Value.ACE, Suite.SPADES);
    Card two = hand.get(1);
    assertCard(two, Value.QUEEN, Suite.HEARTS);
}

versus:

@Test
public void dealGivesKnownCards_whenShufflingWithAKnownSeed() {
    // Arrange
    Random rand = new Random(TEST_SEED);
    CardDeck deck = new CardDeck(rand);
    deck.shuffle();

    // Act
    List<Card> hand = deck.deal(2);
    Card one = hand.get(0);
    Card two = hand.get(1);

    // Assert
    assertThat(deck.size(), is(52));
    assertCard(one, Value.ACE, Suite.SPADES);
    assertCard(two, Value.QUEEN, Suite.HEARTS);
}

This is obviously a contrived example, but in general, it’s nice to always keep tests in the same order, so you will always know where to find things in tests that you may not have seen in a while. Note that you don’t always need an Arrange section, as it might be covered by the @Before set-up method. If I’m unable to disentangle your test in this way, I see it as a signal that I need to refactor the test into several smaller ones. If any of the blocks becomes more than a few lines in length, I will start extracting methods.

By the way, I don’t actually write the comments in real life, I added them here for educational purposes.

Static analysis

If you use static analysis tools like FindBugs or CheckStyle, do you apply them to your test code as well? Because you should. Use the same configuration that you use for your production code, but with a few exceptions. For example, in test code, it’s ok if test methods throw Exception, and test classes can have non-static public fields (as they are required for JUnit’s Rules). Therefore, you can disable the corresponding CheckStyle checks for your test code.

Conclusion

Test code is not “just” test code; you should treat it as if it were production code, and keep it to the same standards. Refactor it. Take care of it. Love it. If you do, you will end up with a test suite that you can actually maintain. Future-you will love you for it.