Testing the Juniors

Stefan S has recently joined the ranks of software developers, having taken on his first job as a junior developer. After a few months of on-boarding with Terry, another new developer, they’re now both actually getting assigned real work on tickets that deliver new functionality.

Stefan proudly pushed his first feature, complete with plenty of unit, functional, and end-to-end tests. After a little brushing up during code-review, it was merged along with a few “atta boys”, and Stefan was feeling pretty good about himself.

A few days later, he pulled the latest changes, and ran the test suite. And all of the tests he wrote suddenly failed. Stefan’s stomach dropped into his shoes, and he struggled to think: “How did I mess up this badly?”

Except Stefan didn’t mess up that badly. A quick check on source control history showed that Terry had added some new commits- one of which “optimized” Stefan’s code by adding a NullPointerException.

Stefan was relieved, but annoyed. He opted to, in his mind, “be a bro”, and not open a ticket that the rest of the team could see, and instead messaged Terry directly. “Your changes have broken functionality. You need to fix it.”

At 5:05PM, Terry pushed a fix, and messaged Stefan, “Tests don’t fail anymore,” then left for the weekend. Terry was correct, the tests stopped failing.

(Names anonymized by Stefan)

public class MyPerfectlyDoneTestClassNotReadyToHaveTheHunsReleasesUponItself {
@Test
public void addition_isCorrect() {
assertEquals(4, 2 + 2);
}

@Test
public void test(){
try {
// lots of logic and setup 20+ lines
} catch (Throwable throwable) {
assertTrue(true);
}
}
@Test
public void test2(){
// different logic
assertTrue(true);
}
@Test
public void test3(){
try {
// yet more well thought out logic. Since there was setUp to these tests
// he had to move the setUp to the actual methods and since they throw he had to put them in try catch
} catch () {
assertFalse(false);
}
}
@Test
public void test40Plus(){
// I give up
assertTrue(true);
}
}

So, yes. Terry just asserted true on any test that threw an exception. Or, asserted false. He also stripped out the set-up methods for the test suite and copy/pasted the setup code into methods. This massive change to the test suite was quite a bit of work- quite a bit more than just fixing the bug with the null pointer.

The lesson here, for Stefan, is that “being a bro” isn’t a great idea. Opening a ticket and creating visibility isn’t throwing Terry under the bus- Terry’s a junior and these kinds of mistakes are still likely to happen, and any healthy organization will be willing to allow growth. When we see mistakes, we can correct them.

The additional visibility also helps the organization spot the deeper WTF: Terry’s code should never have ended up in a branch anyone else was touching. CI and code review should have stopped Terry from breaking code in a way that impacted anyone else.

Source: Read More