Java

Standards and Guidelines

jUnit

Test Classes

• Must be located under src/test/java as per Maven convention.
• Must be in the same package as the class they are testing.

• Must have the same name as the class they are testing and be suffixed with Test, e.g., MyClassTest.java.

  • Exception: If a class requires parametrized tests, multiple test classes can exist for a single class.

Test Methods

• Class setup method (@BeforeClass) must be named setupClass().
• Test setup method (@Before) must be named setup().
• Class clean up method (@AfterClass) must be named tearDownClass().
• Test clean up method (@After) must be named tearDown().
• Test methods can optionally be prefixed with test, .e.g, testMultiplyPositiveNumbers().
• Must follow the usual method naming conventions, i.e., first letter lowercase, rest of name camel-cased.
• Must have a name that clearly describes what is being tested, e.g., testRequestIsRejectedWhenCredentialsAreMissing().
  • When a method is difficult to name properly or test a complex scenario, proper JavaDoc comments should be added to the test method.

Spock

Test Classes

• Must be located under src/test/groovy as per Maven convention.
• Must have the same name as the class they are testing and be suffixed with Spec, e.g., MyClassSpec.groovy.

Test Methods

• Please see the aforementioned Spock Primer for test method documentation.

Best Practices

General

1. Test fixtures that are common to all the tests in a test class and need to be initialized only once should be put inside the @BeforeClass method.
2. Test fixtures that are common to all the tests in a test class and need to be initialized before every test is run should be put inside the @Before method.
3. Clean up code that needs to be run after every test method should be put inside the @After method.
4. Clean up code that needs to be run only once after all tests have been run should be put inside the @AfterClass method.
5. Minimize the use of private inner-classes (anonymous or named) as they usually encapsulate a separate responsibility and could be extracted and unit tested on their own.
6. Unit tests should test all exception scenarios, including checked and unchecked exceptions thrown by themselves or their dependencies.

See Testing Best Practices for other general best practices.

Assertions

1. Favor jUnit's assertThat()and hamcrest Matchers or AssertJ over plain jUnit assert methods such as assertTrue, assertEquals, etc. (see http://junit.sourceforge.net/doc/ReleaseNotes4.4.html for the rationale).
2. If you use jUnit's assert methods, consider providing a message to make the assertion failure clearer, e.g., assertEquals("Unexpected multiplication result", 4, multiplcator.multiply(2, 2));
3. A test method should only test one specific behavior and only contain assertions related to that behavior.
   
   • Too many assertions is usually an indication that the test is testing too many behaviors.
   • Validating too many things inside the same test method makes it difficult to properly name the test method and determine exactly what went wrong when the test fails.
    Expand source

   // Do not do this
   @Test
   public void testMultiply() {
   	assertThat(multiplicator.multiply(2, 2), equalTo(4));
   	assertThat(multiplicator.multiply(2, -2), equalTo(-4));
   	assertThat(multiplicator.multiply(-2, -2), equalTo(4));
   }
    
   // Do this instead
   @Test
   public void testMultiplyPositiveNumbers() {
   	assertThat(multiplicator.multiply(2, 2), equalTo(4));
   }
    
   @Test
   public void testMultiplyPositiveWithNegative() {
   	assertThat(multiplicator.multiply(2, -2), equalTo(-4));
   }
    
   @Test
   public void testMultiplyNegativeNumbers() {
   	assertThat(multiplicator.multiply(-2, -2), equalTo(4));
   }

4. When testing that an exception should be thrown,  use  @Test(expected=<ExceptionClass.class>) or the ExpectedException rule and let the jUnit runner fail the test. Do not use a try/catch block in your code and call fail() if the exception isn't thrown.

    Expand source

   // Do not do this
   @Test
   public void testMultiplyOverflowWrongWay() {
   	try {
   		multiplicator.multiply(Integer.MAX_VALUE, Integer.MAX_VALUE);
   		fail("Expected multiplication overflow");
   	}
   	catch (MultiplicationOverflowException e) {
   		// Success
   	}
   }
    
   // Do this instead
   @Test(expected=MultiplicationOverflowException.class)
   public void testMultiplyOverflowWrongWayRightWay() throws MultiplicationOverflowException {
   	multiplicator.multiply(Integer.MAX_VALUE, Integer.MAX_VALUE);
   }

API documentation and unit tests

Documentation (JavaDoc) and unit tests should always be in-sync and support each other.

1. Unit test every constraint or exception documented in the class' API (public and protected methods).

2. Document every constraint and exception covered by the unit test.

    Expand source

   // Class
    
   public class TextParser {
   	/**
   	 * Counts the number of times a {@code token} appears in a string.
   	 *
   	 * @param text
   	 *		text to search. Cannot be {@code null}.
   	 * @param token
   	 *		token to look for. Cannot be empty or {@code null}.
   	 * @throws IllegalArgumentException
   	 *		thrown if {@code text} is {@code null}, or if {@code token} is {@code null} or empty.
   	 */
   	public int countOccurences(String text, String token) throw IllegalArgumentException {
   		Validate.notNull(text, "Text cannot be null");
   		Validate.notNull(token, "Token cannot be null");
   		Validate.notEmpty(token, "Token cannot be empty");
   
   		// Implementation
   	}
   }
    
   // Unit Test
    
   public class TextParserTest {
   	private TextParser textParser;
    
   	@Before
   	public void setup() {
   		textParser = new TextParser();
   	}
    
   	@Test
   	public void testCountOccurencesFindsOneOccurence() {
   		// Test code and assertions
   	}
    
   	@Test
   	public void testCountOccurencesFindsNoOccurences() {
   		// Test code and assertions
   	}
   
   	// More happy path test cases...
   	@Test(expected=IllegalArgumentException.class)
   	public void testCountOccurencesFailsWithNullText() {
   		textParser.countOccurences(null, "A");
   	}
    
   	@Test(expected=IllegalArgumentException.class)
   	public void testCountOccurencesFailsWithNullToken() {
   		textParser.countOccurences("Some text", null);
   	}
   
   
   	@Test(expected=IllegalArgumentException.class)
   	public void testCountOccurencesFailsWithEmptyToken() {
   		textParser.countOccurences("Some text", "");
   	}
   }

Mock Objects

1. Production code should avoid final methods and classes as well as static methods whenever possible.
   • Rationale: Those cannot be mocked with regular mocking frameworks such as Mockito or Spock and require the use of tools such as PowerMock.
2. Mock objects should always be created using a mocking framework, never written by hand.
   • Rationale: Writing mock object by hand may require complex code to mock certain behaviors, which increases the chance of introducing bugs in the mocking code and the maintenance cost of the code base.
3. All the dependencies of a class should be replaced with mock objects before the test is run.
   • Exception: Value objects and beans that do not have any behavior do not have to be mocked.
   • Exception: Dependencies used by legacy code do not need to be replaced with mock objects if doing so requires too much effort or introduces too much risk.
4. When verifying a mock object's expectations, expect specific arguments. In other words, avoid argument matchers such as any(), anyList(), etc.
   • Rationale: The goal of verifying expectations is to ensure that a class respects the contract it has with its dependencies. Using any...() doesn't enforce this.
   • Exception: If the value does not support the current test case and is tested in another unit test.
5. Favor argThat() or ArgumentCaptor over custom Matcher to validate argument values
   • Rationale: Custom Matchers are usually more difficult to write, read and maintain than using simple argThat() or ArgumentCaptors with assertions.
6. Avoid the use of Mockito's thenAnswer() and doAnswer() to validate input parameters, use ArgumentCaptor instead
   • Rationale: those methods are meant to setup mock expectations, not verify their behavior.
   • Exception: When the response to return needs to be built from the parameters provided to the mocked method.
   • Exception: When other mocking techniques cannot be used to verify expectations, e.g., output parameters need to be verified.

   • Exception: When the code delegates to Callable, Runnable or any other kind of FunctionalInterface.

      Expand source

     // Class
     
     public class SecureRunner implements Callable {
         private final Security security;
     
         public SecureRunner(Security security) {
             this.security = security;
         }
     
         public String runSecurely() {
             return security.runWithSubjectOrElevate(this::call);
         }
     
         public String call() {
             return "It works!"
         }
     }
     
     // Unit Test
     
     
     @RunWith(MockitoJUnitRunner.class)
     public class SecureRunnerTest {
         @Mock
         private Security security;
     
         public void testDoSomethingIsCalled() {
             when(security.runWithSubjectOrElevate(any(Callable.class))).thenAnswer(invocation -> ((Callable) invocation.getArguments()[0]).call());
     
             SecureRunner secureRunner = new SecureRunner(security);
     
             String message = secureRunner.runSecurely();
     
             assertThat(message, equalTo("It works!"));
         }
     }

7. Only verify conditions that are related to the behavior being tested. Over-verification makes tests brittle and break for no good reason.
   • Example: If a mock has a method that is currently called only once but could be called multiple times without impacting the result of the test, do not fail the test (e.g., verify(mockObject, times(1)).expectedMethod()) if it is called more than once.
8. Mocking tools such as PowerMock should only be used to unit test complex legacy code that cannot be easily refactored.
   1. Exception: An acceptable use of PowerMock outside of legacy code testing is when a class needs to use a system or third-party class' static method. This is especially useful when testing exception scenarios that would otherwise be difficult or impossible to test. It can also help prevent dependencies on the environment itself, one of the basic qualities of a good unit test.

Dependency Injection

Leveraging dependency injection provided by tools such as Blueprint is a great way to make writing unit tests easier (see Mock Objects section above).

1. Production classes should be written in such a way that their dependencies are injected through their constructors or setter methods and not created or looked-up from within the classes themselves.
2. Prefer dependency injection over static methods in production classes.
   • Rationale: Static methods create a tight coupling between a class and its dependencies. This makes it difficult to test the class in isolation or test edge-cases, such as the static method throwing an exception or returning a specific value.

3. When a dependency cannot be injected (e.g., multiple instances of the same dependency are needed within the class), consider creating and injecting a factory class or a Supplier, or use the Factory Method design pattern.

4. When testing concurrent classes that use a thread Executor, use MoreExecutors.directExecutor() to inject an Executor that runs in the testing thread and eliminate any issues associated with tested concurrent code.
5. Similarly, when testing concurrent classes that use an ExecutorService, replace it with MoreExecutors.newDirectExecutorService().

Code Coverage

1. Getters and setters should not be tested unless they contain some logic.
2. Aim for 100% code coverage and always set or update the thresholds in the module's pom file to match the actual code coverage (minus 2% to account for rounding errors).
3. Coverage must have at least 75% at the bundle level for instruction, line and path coverage as well as cyclomatic complexity.
   • Exception: These thresholds can be lower if the code contains many getters and setters, conditional logging statements or try-with-resources blocks.
   • Exception: Legacy code does not have to hit the 75% mark when committed.
4. Coverage thresholds should always go up, unless the new code added only contains getters and setters or logging statements
   
   • Rationale: The goal is to increase those thresholds over time and reach 75-80% overall coverage in the future; lowering the numbers goes against that goal.
5. Write tests that cover exceptions thrown by a class' dependencies.

   • Example: If a dependency can throw an IllegalArgumentException, a test should exist to ensure that the exception is properly handled, even if the behavior is to let the exception bubble up.

     Methods that throw exceptions

     Jacoco will not mark methods that do not return (ie. methods that throw Runtime Exceptions) as having been covered, even if they are. If there is a case where a helper method constructs and throws an exception, it will better satisfy Jacoco to have that method return the created exception that the caller can then throw.

6. Ensure that all paths in a stream or Optional are covered, including all filter, nonNull, etc.

   Streams

   By default, JaCoCo will consider a stream as a single path and mark all the lines as covered as long as the line has been executed once. This means that actual path coverage may not be complete.

   Same comment applies to Optional.orElse methods.

Smells

1. Slow unit tests
   • Slow unit tests is usually an indication that the test uses a slow external dependency. In those cases, the test should be changed to mock out that dependency.
   • Unit tests that need wait for a multi-threaded class to complete it's work before doing assertion and verification is another typical example. In those cases, mocking out the threaded out code or injecting direct executors (see above) usually helps solve those problems.

Anti-Patterns

General

1. The visibility of private methods should not be changed only to allow their internal logic to be tested.
   • Rationale: Making a method non-private breaks a class' encapsulation and couples the test with the class' implementation details.
   • Exceptions: Constructors, setters or factory methods made package-private so they can be used to inject mock dependencies are acceptable but should only be used as a last resort.

See Testing Anti-Patterns for other general testing anti-patterns.