Over a million developers have joined DZone.
{{announcement.body}}
{{announcement.title}}

Test Methods Must Share Nothing

DZone's Guide to

Test Methods Must Share Nothing

Author Yegor Bugayenko makes a case that shared variables in test classes are evil.

· Java Zone
Free Resource

Learn how to troubleshoot and diagnose some of the most common performance issues in Java today. Brought to you in partnership with AppDynamics.

Constants... I wrote about them some time ago, mostly saying that they are a bad thing, if they are public. They reduce duplication, but introduce coupling. A much better way to get rid of duplication is by creating new classes or methods—a traditional OOP method. This seems to make sense, and, in our projects, I see less and less public constants. In some projects, we don't have them at all. But, one thing still bothers me: unit tests. Most programmers seem to think that when static analysis says that there are too many similar literals in the same file, the best way to get rid of them is via a private static literal. This is just wrong.

Image title

Unit tests, naturally, duplicate a lot of code. Test methods contain similar or almost identical functionality and this is almost inevitable. Well, we can use more of those @Before and @BeforeClass features, but sometimes it's just not possible. We may have, say, 20 test methods in one FooTest.java file. Preparing all objects in one "before" is not possible. So, we have to do certain things again and again in our test methods.

Let's take a look at one of the classes in our Takes Framework: VerboseListTest. It's a unit test and it has a problem. Look at that MSG private literal. It is used for the first time in setUp() method as an argument of an object constructor and then in a few test methods to check how that object behaves. Let me simplify that code:

class FooTest {
  private static final String MSG = "something";
  @Before
  public final void setUp() throws Exception {
    this.foo = new Foo(FooTest.MSG);
  }
  @Test
  public void simplyWorks() throws IOException {
    assertThat(
      foo.doSomething(),
      containsString(FooTest.MSG)
    );
  }
  @Test
  public void simplyWorksAgain() throws IOException {
    assertThat(
      foo.doSomethingElse(),
      containsString(FooTest.MSG)
    );
  }
}

This is basically what is happening in VerboseListTest and it's very wrong. Why? Because this shared literal MSG introduced an unnatural coupling between these two test methods. They have nothing in common because they test different behaviors of class Foo. But, this private constant ties them together. Now they are somehow related.

If and when I want to modify one of the test methods, I may need to modify the other one too. Say I want to see how doSomethingElse() behaves if the encapsulated message is an empty string. What do I do? I change the value of the constant FooTest.MSG, which is used by another test method. This is called coupling. And, it's a bad thing.

What do we do? Well, we can use that "something" string literal in both test methods:

class FooTest {
  @Test
  public void simplyWorks() throws IOException {
    assertThat(
      new Foo("something").doSomething(),
      containsString("something")
    );
  }
  @Test
  public void simplyWorksAgain() throws IOException {
    assertThat(
      new Foo("something").doSomethingElse(),
      containsString("something")
    );
  }
}

As you see, I got rid of that setUp() method and the private static literal MSG. What do we have now? Code duplication. String "something" shows up four times in the test class. No static analyzers will tolerate that. Moreover, there are seven (!) test methods in VerboseListTest, which are using MSG. Thus, we will have 14 occurrences of "something", right? Yes, that's right and that's most likely why one of the authors of this test case introduced the constant—to get rid of duplication. BTW, @Happy-Neko did that in pull request #513, @carlosmiranda reviewed the code, and I approved the changes. So, three people made/approved that mistake, including myself.

So, what is the right approach that will avoid code duplication and at the same time won't introduce coupling? Here it is:

class FooTest {
  @Test
  public void simplyWorks() throws IOException {
    final String msg = "something";
    assertThat(
      new Foo(msg).doSomething(),
      containsString(msg)
    );
  }
  @Test
  public void simplyWorksAgain() throws IOException {
    final String msg = "something else";
    assertThat(
      new Foo(msg).doSomethingElse(),
      containsString(msg)
    );
  }
}

These literals must be different. This is what any static analyzer is saying when it sees "something" in so many places. It questions us—why are they the same? Is it really so important to use "something" everywhere? Why can't you use different literals? Of course, we can. And, we should.

The bottom line is that each test method must have its own set of data and objects. They must not be shared between test methods ever. Test methods must always be independent, having nothing in common.

Having that in mind, we can easily conclude that methods like setUp() or any shared variables in test classes are evil. They must not be used and simply must not exist. I think that their invention in JUnit caused a lot of harm to Java code.

Understand the needs and benefits around implementing the right monitoring solution for a growing containerized market. Brought to you in partnership with AppDynamics.

Topics:
unit testing

Published at DZone with permission of Yegor Bugayenko, DZone MVB. See the original article here.

Opinions expressed by DZone contributors are their own.

THE DZONE NEWSLETTER

Dev Resources & Solutions Straight to Your Inbox

Thanks for subscribing!

Awesome! Check your inbox to verify your email so you can start receiving the latest in tech news and resources.

X

{{ parent.title || parent.header.title}}

{{ parent.tldr }}

{{ parent.urlSource.name }}