需要重构以提高可测试性答案

【问题标题】：In Need of Refactoring in Order to Improve Testability需要重构以提高可测试性
【发布时间】：2009-07-17 14:18:06
【问题描述】：

我正在使用 mockito 测试一个简单的 DAO 层，但我发现了一个问题，基本上是一个难以测试的接口，我想知道您能否给我一些见解...

这是我要测试的方法：

public Person getById(UserId id) {
    final Person person = new PersonImpl();

    gateway.executeQuery(GET_SQL + id.getUserId(), new ResultSetCommand(){
      public int work(ResultSet rs) throws SQLException {
        if(rs.next()){
          person.getName().setGivenName(rs.getString("name"));
          person.getName().setFamilyName(rs.getString("last_name"));
        }
        return 0;
      }
    });
    return person;
  }

我使用 DatabaseGateway，它是我的 java 代码和 SQL 之间的接口，并且该方法接受一个匿名类，这是网关的方法 executeQuery：

 public int executeQuery(String sql, ResultSetCommand cmd) {
    try{
      Connection cn =  createConnection();
      PreparedStatement st = cn.prepareStatement(sql);
      int result = cmd.work(st.executeQuery());
      cn.close();
      return result;
    }catch(Exception e){
      throw new RuntimeException("Cannot Create Statement for sql " + sql,e);
    }
  }

问题是，由于那个匿名类，测试 PersonDAO 变得越来越困难。

如果有人提出更好的设计，我可以重构整个代码，甚至删除匿名类（我确信有一个更简单的，但我似乎找不到它）。

感谢大家的建议。

PD：如果您需要更多信息，请随时询问

编辑：很难做的测试

public void testGetPersonById(){
    DatabaseGateway gateway = mock(DatabaseGateway.class);
    when(gateway.executeQuery(anyString(),any(ResultSetCommand.class)));
    PersonDAO person_dao = new PersonDAOImpl(gateway);

    Person p = person_dao.getById(new UserId(Type.viewer,"100"));
  }

看到了吗？ ResultCommand 是模拟的一部分，我也有兴趣测试该代码...我应该为该特定命令进行单独测试吗？

【问题讨论】：

请举一个越来越难做的测试的例子。
我不太了解 Java，但我想说 ResultSetCommand 不相关。您正在测试 getPersonById 以确保它在给定有效 UserId 时返回正确的 Person，在给定无效 UserId 时抛出异常等。如果它正常工作，您不必关心它使用特定的 ResultSetCommand。

标签： java unit-testing refactoring mocking

【解决方案1】：

您可以单独创建接口及其实现，而不是使用匿名类。然后方法 executeQuery 将有一个 String 和这个接口作为参数。

因此您的测试将保持不变。您可以在另一个测试（接口实现的测试）中分离工作方法，这看起来很难测试。

结果会是这样的：

public Person getById(UserId id) {
    final Person person = new PersonImpl();

    gateway.executeQuery(GET_SQL + id.getUserId(), new MyInterfaceImpl(person));
    return person;
}

public int executeQuery(String sql, MyInterface cmd) {
    try{
      Connection cn =  createConnection();
      PreparedStatement st = cn.prepareStatement(sql);
      int result = cmd.work(st.executeQuery());
      cn.close();
      return result;
    }catch(Exception e){
      throw new RuntimeException("Cannot Create Statement for sql " + sql,e);
    }
  }

【讨论】：

所以你说我做了 2 个测试，一个用于 PersonDAO，另一个用于（不再是）匿名 ResultSetCommand 类？
@Pablo 是的，测试将是分开的（考虑到您想要对其进行单元测试，而不是进行集成测试）。如果你想要一个“更大的图片测试”，你可以让你的 executeQuery 只返回 ResultSet，并在你的 PersonDAO 中使用它。看看什么对你最有意义
我首先尝试了这种方法（返回 RS），但它引发了奇怪的错误，我相信是因为 RS 无法在连接关闭或类似的情况下工作，我不想放弃调用者负责关闭 RS 或连接
@Pablo 我相信你对 RS 的陈述是正确的。您需要关闭外部连接（同意这是一个坏主意）。如果您对我的建议有任何问题，请告诉我。但就个人而言，我认为单独测试“工作”方法没有问题（如果测试中断，则更容易找到错误）
没有。我认为这很好。如果没有人提供更好的答案，这将是公认的

【解决方案2】：

您可以花哨并“捕获”ResultSetCommand arg，然后使用模拟 ResultSet 在其上模拟回调：

/**
 * Custom matcher - always returns true, but captures the
 * ResultSetCommand param
 */
class CaptureArg extends ArgumentMatcher<ResultSetCommand> {
    ResultSetCommand resultSetCommand;
    public boolean matches(Object resultSetCommand) {
         resultSetCommand = resultSetCommand;
         return true;
    }
}

public void testGetPersonById(){
    // setup expectations...
    final String lastName = "Smith";
    final String firstName = "John";
    final CaptureArg captureArg = new CaptureArg();
    DatabaseGateway gateway = mock(DatabaseGateway.class);
    ResultSet mockResultSet = mock(ResultSet.class);
    when(gateway.executeQuery(anyString(), argThat(captureArg) ));
    when(mockResultSet.next()).thenReturn(Boolean.True);
    when(mockResultSet.getString("name")).thenReturn(firstName);
    when(mockResultSet.getString("last_name")).thenReturn(lastName);

    // run the test...
    PersonDAO person_dao = new PersonDAOImpl(gateway);
    Person p = person_dao.getById(new UserId(Type.viewer,"100"));

    // simulate the callback...
    captureArg.resultSetCommand.work(mockResultSet);

    // verify
    assertEquals(firstName, person.getName().getGivenName());
    assertEquals(lastName, person.getName().getFamilyName());
}

我对我是否喜欢这个感到矛盾 - 它暴露了您正在测试的方法的许多内部结构。但至少这是一种选择。

【讨论】：