Invariant Properties

  • rss
  • Home

Should Mocked Method Arguments Make Assumptions?

Bear Giles | March 27, 2016

I’m cleaning up some tests at my new job and came across an interesting difference in opinion. Here are two ways to write a test. Which is better?

Context: the class being tested acts as a translation layer between our internal abstraction layer and a traditional JDBC driver. JPA and other ORM frameworks are not a viable option since we must work with arbitrary databases. We usually don’t want to call verify() since we want to test behavior, not implementation, but in this case the behavior we want to test is whether the appropriate calls are made to the mocked JDBC objects.

(Implementation note: we are using easymock but we can create similar code in jmock and mockito.)

Approach 1

  1. final Connection conn = createMock(Connection.class);
  2. final PreparedStatement stmt = createMock(PreparedStatement.class);
  3.  
  4. // more initialization
  5.  
  6. expect(conn.prepareStatement("insert into foo(a, b, c) values (?, ?, ?)")).andReturn(stmt);
  7. stmt.setString(1, "Larry");
  8. expectLastCall();
  9. stmt.setString(2, "Curly");
  10. expectLastCall();
  11. stmt.setString(3, "Moe");
  12. expectLastCall();
  13. expect(stmt.execute()).andReturn(true);
  14.  
  15. // more initialization
  16.  
  17. replay(conn, stmt);
  18.  
  19. // run code under test
  20.  
  21. verify(conn, stmt);
  22. }
final Connection conn = createMock(Connection.class);
final PreparedStatement stmt = createMock(PreparedStatement.class);

// more initialization

expect(conn.prepareStatement("insert into foo(a, b, c) values (?, ?, ?)")).andReturn(stmt);
stmt.setString(1, "Larry");
expectLastCall();
stmt.setString(2, "Curly");
expectLastCall();
stmt.setString(3, "Moe");
expectLastCall();
expect(stmt.execute()).andReturn(true);

// more initialization

replay(conn, stmt);

// run code under test

verify(conn, stmt);
}

Approach 2

  1. final Connection conn = createMock(Connection.class);
  2. final PreparedStatement stmt = createMock(PreparedStatement.class);
  3.  
  4. final String expectedSql = "insert into foo(a, b, c) values (?, ?, ?)";
  5. final List<String> expectedValues = ImmutableList.of("Larry", "Curly", "Moe");
  6.  
  7. final Capture<String> actualSql = newCapture();
  8. final Capture<String> actualValues = newCapture(CaptureType.ALL);
  9.  
  10. // more initialization
  11.  
  12. expect(conn.prepareStatement(capture(actualSql))).andReturn(stmt);
  13. for (int i = 0; i < expectedValues.size(); i++) {
  14.     stmt.setString(eq(i + 1), actualValues);
  15.     expectLastCall();
  16. }
  17. expect(stmt.execute()).andReturn(true);
  18.  
  19. // more initialization
  20.  
  21. replay(conn, stmt);
  22.  
  23. // run code under test
  24.  
  25. assertEquals(expectedSql, actualSql);
  26. assertEquals(expectedValues, actualValues);
  27.  
  28. verify(conn, stmt);
  29. }
final Connection conn = createMock(Connection.class);
final PreparedStatement stmt = createMock(PreparedStatement.class);

final String expectedSql = "insert into foo(a, b, c) values (?, ?, ?)";
final List<String> expectedValues = ImmutableList.of("Larry", "Curly", "Moe");

final Capture<String> actualSql = newCapture();
final Capture<String> actualValues = newCapture(CaptureType.ALL);

// more initialization

expect(conn.prepareStatement(capture(actualSql))).andReturn(stmt);
for (int i = 0; i < expectedValues.size(); i++) {
    stmt.setString(eq(i + 1), actualValues);
    expectLastCall();
}
expect(stmt.execute()).andReturn(true);

// more initialization

replay(conn, stmt);

// run code under test

assertEquals(expectedSql, actualSql);
assertEquals(expectedValues, actualValues);

verify(conn, stmt);
}

Both tests will fail if the expected SQL or values are different than expected. The former creates a framework exception (unexpected method call/missing method call), the latter creates a standard JUnit failure giving expected and actual values.

My coworker argues that the first approach is better since it’s more concise. That’s a good point – shorter methods are easier to understand and maintain.

I argue that the second approach is better since the messages are a lot clearer and the code is much more explicit that we’re concerned about the arguments to the mocked methods. The second approach will also be a lot easier to convert to a parameterized test in the future. Furthermore the actual code (not the snippet above) often uses the expectedValues value as an argument to the code under test. This reduces the risk of breaking tests because we updated one value but not the corresponding value.

Given-When-Then

Academic programs rarely spend much time on testing methodologies and many companies still treat testing as grunt work for the junior people instead of something critical to the long-term success of the project. We would never think it’s enough to just sit down and bang out code but we’ll do exactly that with test code.

No, we need to think about what we’re trying to accomplish and the best way to do it. Fortunately some very smart people have been thinking about this for a long time. One approach is Given-When-Then. Some test frameworks are explicitly built around this approach. JUnit is not but it’s easy to follow as a heuristic.

The second approach is easy to convert to Given-When-Then.

Given-When-Then Approach

  1. Capture<String> setupForPreparedStatement(Connection conn, PreparedStatement stmt, Capture<Object> actualValues)
  2.         throws SQLException {
  3.     final Capture<String> actualSql = newCapture();
  4.  
  5.     expect(conn.prepareStatement(capture(actualSql))).andReturn(stmt);
  6.     for (int i = 0; i < expectedValues.size(); i++) {
  7.         Object obj = expectedValues.get(i);
  8.         if (obj == null) {
  9.             stmt.setNull(eq(i + 1), anyInt());
  10.         } else if (obj instanceof String) {
  11.             stmt.setString(eq(i + 1), (String) actualValues);
  12.         } else if (obj instanceof BigDecimal) {
  13.             stmt.setBigDecimal(eq(i + 1), (BigDecimal) actualValues);
  14.         } else ... {
  15.         }
  16.         expectLastCall();
  17.     }
  18.     expect(stmt.execute()).andReturn(true);
  19.  
  20.     return actualSql;
  21. }
  22.  
  23. public void test1() throws SQLException {
  24. given:
  25.     final Connection conn = createMock(Connection.class);
  26.     final PreparedStatement stmt = createMock(PreparedStatement.class);
  27.     final Capture<String> actualValues = newCapture(CaptureType.ALL);
  28.  
  29.     setupForPreparedStatement(conn, stmt, actualValues);
  30.  
  31. when:
  32.     final List<Object> expectedValues = ImmutableList. of("Larry", "Curly", "Moe");
  33.  
  34. then:
  35.     replay(conn, stmt);
  36.     // run code under test
  37.     verify(conn, stmt);
  38.  
  39.     assertEquals(expectedValues, actualValues);
  40. }
Capture<String> setupForPreparedStatement(Connection conn, PreparedStatement stmt, Capture<Object> actualValues)
        throws SQLException {
    final Capture<String> actualSql = newCapture();

    expect(conn.prepareStatement(capture(actualSql))).andReturn(stmt);
    for (int i = 0; i < expectedValues.size(); i++) {
        Object obj = expectedValues.get(i);
        if (obj == null) {
            stmt.setNull(eq(i + 1), anyInt());
        } else if (obj instanceof String) {
            stmt.setString(eq(i + 1), (String) actualValues);
        } else if (obj instanceof BigDecimal) {
            stmt.setBigDecimal(eq(i + 1), (BigDecimal) actualValues);
        } else ... {
        }
        expectLastCall();
    }
    expect(stmt.execute()).andReturn(true);

    return actualSql;
}

public void test1() throws SQLException {
given:
    final Connection conn = createMock(Connection.class);
    final PreparedStatement stmt = createMock(PreparedStatement.class);
    final Capture<String> actualValues = newCapture(CaptureType.ALL);

    setupForPreparedStatement(conn, stmt, actualValues);

when:
    final List<Object> expectedValues = ImmutableList. of("Larry", "Curly", "Moe"); 

then:
    replay(conn, stmt);
    // run code under test
    verify(conn, stmt);

    assertEquals(expectedValues, actualValues);
}

(For clarity I’ve removed the test for the expected SQL statement.)

In this code there is absolutely nothing in the setup that makes assumptions about what values will be passed to the mocked objects. It doesn’t even make assumptions about what SQL commands will be passed, just that we want to create a prepared statement, populate it, then execute it.

It’s not hard to take this a step further and decree that all ‘expect’ calls should be done in setup methods and maintained by the developer working on the tested class. The actual test methods will be maintained by other people and follow the “test behavior, not implementation” rule.

If we don’t need to verify() the mocked objects – and remember we usually won’t since we want to test behavior and not implementation – then we can make the test even more concise.

Given-When-Then Approach without verify()

  1. Capture<String> setupForPreparedStatement(Capture<Object> actualValues) throws SQLException {
  2.     final Connection conn = createMock(Connection.class);
  3.     final PreparedStatement stmt = createMock(PreparedStatement.class);
  4.     final Capture<String> actualSql = newCapture();
  5.  
  6.     expect(conn.prepareStatement(capture(actualSql))).andReturn(stmt);
  7.     for (int i = 0; i < expectedValues.size(); i++) {
  8.         Object obj = expectedValues.get(i);
  9.         if (obj == null) {
  10.             stmt.setNull(eq(i + 1), anyInt());
  11.         } else if (obj instanceof String) {
  12.             stmt.setString(eq(i + 1), (String) actualValues);
  13.         } else if (obj instanceof BigDecimal) {
  14.             stmt.setBigDecimal(eq(i + 1), (BigDecimal) actualValues);
  15.         } else ... {
  16.         }
  17.         expectLastCall();
  18.     }
  19.     expect(stmt.execute()).andReturn(true);
  20.     replay(conn, stmt);
  21.  
  22.     return actualSql;
  23. }
  24.  
  25. public void test1() throws SQLException {
  26. given:
  27.     final Capture<String> actualValues = newCapture(CaptureType.ALL);
  28.     setupForPreparedStatement(actualValues);
  29.  
  30. when:
  31.     final List<Object> expectedValues = ImmutableList. of("Larry", "Curly", "Moe");
  32.  
  33. then:
  34.     // run code under test
  35.  
  36.     assertEquals(expectedValues, actualValues);
  37. }
Capture<String> setupForPreparedStatement(Capture<Object> actualValues) throws SQLException {
    final Connection conn = createMock(Connection.class);
    final PreparedStatement stmt = createMock(PreparedStatement.class);
    final Capture<String> actualSql = newCapture();

    expect(conn.prepareStatement(capture(actualSql))).andReturn(stmt);
    for (int i = 0; i < expectedValues.size(); i++) {
        Object obj = expectedValues.get(i);
        if (obj == null) {
            stmt.setNull(eq(i + 1), anyInt());
        } else if (obj instanceof String) {
            stmt.setString(eq(i + 1), (String) actualValues);
        } else if (obj instanceof BigDecimal) {
            stmt.setBigDecimal(eq(i + 1), (BigDecimal) actualValues);
        } else ... {
        }
        expectLastCall();
    }
    expect(stmt.execute()).andReturn(true);
    replay(conn, stmt);

    return actualSql;
}

public void test1() throws SQLException {
given:
    final Capture<String> actualValues = newCapture(CaptureType.ALL);
    setupForPreparedStatement(actualValues);

when:
    final List<Object> expectedValues = ImmutableList. of("Larry", "Curly", "Moe"); 

then:
    // run code under test

    assertEquals(expectedValues, actualValues);
}
Comments
No Comments »
Categories
java, politics
Comments rss Comments rss
Trackback Trackback

Archives

  • May 2020 (1)
  • March 2019 (1)
  • August 2018 (1)
  • May 2018 (1)
  • February 2018 (1)
  • November 2017 (4)
  • January 2017 (3)
  • June 2016 (1)
  • May 2016 (1)
  • April 2016 (2)
  • March 2016 (1)
  • February 2016 (3)
  • January 2016 (6)
  • December 2015 (2)
  • November 2015 (3)
  • October 2015 (2)
  • August 2015 (4)
  • July 2015 (2)
  • June 2015 (2)
  • January 2015 (1)
  • December 2014 (6)
  • October 2014 (1)
  • September 2014 (2)
  • August 2014 (1)
  • July 2014 (1)
  • June 2014 (2)
  • May 2014 (2)
  • April 2014 (1)
  • March 2014 (1)
  • February 2014 (3)
  • January 2014 (6)
  • December 2013 (13)
  • November 2013 (6)
  • October 2013 (3)
  • September 2013 (2)
  • August 2013 (5)
  • June 2013 (1)
  • May 2013 (2)
  • March 2013 (1)
  • November 2012 (1)
  • October 2012 (3)
  • September 2012 (2)
  • May 2012 (6)
  • January 2012 (2)
  • December 2011 (12)
  • July 2011 (1)
  • June 2011 (2)
  • May 2011 (5)
  • April 2011 (6)
  • March 2011 (4)
  • February 2011 (3)
  • October 2010 (6)
  • September 2010 (8)

Recent Posts

  • 8-bit Breadboard Computer: Good Encapsulation!
  • Where are all the posts?
  • Better Ad Blocking Through Pi-Hole and Local Caching
  • The difference between APIs and SPIs
  • Hadoop: User Impersonation with Kerberos Authentication

Meta

  • Log in
  • Entries RSS
  • Comments RSS
  • WordPress.org

Pages

  • About Me
  • Notebook: Common XML Tasks
  • Notebook: Database/Webapp Security
  • Notebook: Development Tips

Syndication

Java Code Geeks

Know Your Rights

Support Bloggers' Rights
Demand Your dotRIGHTS

Security

  • Dark Reading
  • Krebs On Security Krebs On Security
  • Naked Security Naked Security
  • Schneier on Security Schneier on Security
  • TaoSecurity TaoSecurity

Politics

  • ACLU ACLU
  • EFF EFF

News

  • Ars technica Ars technica
  • Kevin Drum at Mother Jones Kevin Drum at Mother Jones
  • Raw Story Raw Story
  • Tech Dirt Tech Dirt
  • Vice Vice

Spam Blocked

53,314 spam blocked by Akismet
rss Comments rss valid xhtml 1.1 design by jide powered by Wordpress get firefox