junit Java单元测试-将封闭方法调用的方法的返回值设置为空

9w11ddsr  于 12个月前  发布在  Java
关注(0)|答案(3)|浏览(138)

我有这个方法,并执行完成。然而,当我查看Jacoco报告时,它说else {}catch()部分没有被单元测试代码覆盖率覆盖。

public class SomeService {
    
    public List<Something> getAll() throws SomeCustomException {
        List<Something> listOfSomething = new ArrayList<>();
        try (Data data = DataUtil.get(Constants.SOME_TABLE_NAME)){
            ResultSet resultSet = data.retrieveAll();
            if(resultSet != null && !resultSet.isEmpty()){
                Something something;
                for (ResultSetRow row : resultSet){
                    something = new Something();
                    something.setX(row.get("someColumnX"));
                    //...
                    listOfSomething.add(something);
                }
            }else{
                throw new SomeCustomException("ResultSet is null");
            }
        }catch (ExceptionOne | ExceptionTwo e){
            throw new SomeCustomException(e.getMessage(), e);
        }
        return listOfSomething;
    }
    
}

因此,我尝试创建一个单元测试方法来尝试覆盖else(){}部分。但是,我认为我这样做是不正确的,或者对someServiceMock没有影响
基本上,我希望resultSet为空,以便它转到else {}部分并覆盖else {}分支上的测试。问题是,我不确定如何设置它,因为它在getAll()方法中

@Test
void getAllReturnsEmpty() {
    SomeService someServiceMock = mock(SomeService.class);
    ResultSet resultSetMock = mock(ResultSet.class);
    when(!resultSetMock.isEmpty).thenReturn(false);
    
    assertThat(someServiceMock.getAll().isEmpty()).isTrue();
}

有没有办法达到我想要的?我希望Java代码覆盖率达到99%
谢谢你

ve7v8dk2

ve7v8dk21#

就像人们在评论中所说的那样,你嘲笑了错误的事情。您可以模拟静态的东西(如“DataUtil#get”),但由于这并没有触及问题的核心,因此假设我们有

public class SomeService {
   Supplier<Data> dataSupplier; // set in constructor, can be () -> DataUtils.get(SOME_TABLE)

   ...
        try (Data data = dataSupplier.get()){
...

要获得您的覆盖范围,您可以

// exception cases
// exception one
when(dataSupplier.get()).thenThrow(new ExceptionOne());
// exception two
when(dataSupplier.get()).thenThrow(new ExceptionTwo());
// empty retrieval cases
Data data = mock(Data.class);
when(dataSupplier.get()).thenReturn(data);
// covers != null case
when(data.retrieveAll()).thenReturn(null);
// covers != null case
when(data.retrieveAll()).thenReturn(List.of());

// verify SomeCustomException is thrown

所有这四个都需要在单独的测试用例中(具有相同的验证)。

vecaoik1

vecaoik12#

这个问题很复杂,但它变得复杂的每一点都分解成几个选项,所有这些选项都返回到:这不是个好主意

data.retrieveAll()的本质

这里只有两个选择:

  1. data.retrieveAll()可以返回null
  2. data.retrieveAll()是指定的,所以它不可能返回这个值。
    如果是第一种情况,那就...这样写测试。有一种方法可以在这里创建一个data(模拟Data.get(Constants.SOME_TABLE_NAME)来实现这一点)。然而,案例1听起来像是一个严重的错误:

子考虑:查询表中所有行的性质

这分为3个选项,* 可能 * 被认为是'..然后它应该返回null '。而null对所有这些都是错误的:
1.表存在但没有记录。你不应该返回null-你应该返回一个空的结果集(第一次调用.next()已经返回了false)。
1.table不存在,这很奇怪。你不应该返回null-你应该抛出SQLException。它应该抛出一些东西,我认为抛出SQLException以外的任何东西都是错误的,但这取决于情况。扔点东西。
1.表不存在,这一点也不奇怪--这是数据库的正常设计。在这种情况下,有一些语义概念,“表不存在”表示,通常它被视为等同于“表 * 确实 * 存在,但其中没有行”。因此,不要返回null-返回一个空的结果集,就像情况#1一样,因为对调用者来说,“table exists and is empty”和“table does not exist”之间应该没有区别。
我不知道你的应用程序是做什么的,也不知道它应该如何工作,但它会归结为这3个之一,而且没有一个以“.”结尾。而null可能被忽略。

回到retrieveAll()的本质

我们现在已经证明它只能是情况2:它不可能返回null。它可能抛出一些东西,或者可能返回一个空的结果集。

逻辑结论

你不会测试不可能的事情,除非代码本身是这样指定的:如果必须的话,你可以为Data.retrieveAll编写一个单元测试,以确认它不返回null(它本身是不可能被覆盖的,因为它不能),但是你当然可以编写测试,如果你试图查询一个不存在的表,确实会抛出一个异常,如果你查询一个空表,确实会返回一个空的结果集。
一旦你通过了这些测试,你就不会在另一个测试中重新踏过那个地面。想想看:如果“code that does X using A”(这里的X是你的getAll()方法,这里的AData.retrieveAll)的测试也意味着你必须测试A的各个方面,那么它在哪里结束?程序代码往往会导致,这并不夸张,在“顶级调用”(比如,加载youtube.com,youtube的首页)中,导致字面上数千的子系统被调用:用户的播放列表和偏好需要加载,他们的谷歌身份验证身份需要加载和确认,等等。
你想写一个50,000行长的单元测试吗?它主要是数千个其他单元测试的复制/粘贴,这些单元测试本身有20,000行长,因为它们也测试它们下面的整个堆栈。
当然不是,那绝对不成比例。如果Data.retrieveAll被指定为不返回null,你就不用为它写代码(你的else行必须完全消失,因为它在一个不可能的场景中触发),你也不用测试它,除非在retrieveAll方法本身的单元测试中,如果有的话。
一旦你停止编写代码来覆盖不可能的事情,你就不会再在覆盖范围内找到漏洞。
同样的原理也适用于catch block。这也分为几个步骤,但没有一个步骤会导致你需要接受代码覆盖率中的漏洞:
1.方法可以引发该异常。然后模拟它,或者只是创建一个可靠地抛出它的示例。例如,如果您有一个db查询处理器,并且您想测试掉出来的SQL语句,只需运行查询"THIS IS NOT A VALID SQL QUERY;",或者"SELECT 1/0;"
1.方法不可能抛出它。那么就不要为它编写catch,因此也不要测试catch代码,因为它无法运行。如果相关(通常不相关),请编写代码以确保不会在该方法的单元测试中抛出此异常,而不是在getAll方法的单元测试中。它应该列在throws子句中。

1.令人烦恼的是,你正在实现一个方法,因为一个超类型(接口或超类)拥有它,它抛出一个检查异常,而你的实现永远不会抛出。但是,您可以简单地从实现的抛出列表中忽略该异常。然而,如果调用者(你的getAll方法)将对象作为它的超类型,javac仍然会要求你捕捉它(或抛出它),这可能导致catch块不能发生。这整个场景是可疑的-API需要审查。可能需要取消检查该异常(在任何情况下都不可能发生的异常应该取消检查,或者需要更新API以将这些情况分离出来)。如果您无法解决API问题,我们将使用解决方案:你要么模仿它,要么使用类似lombok的@SneakyThrows,或者你分叉库,或者你接受你有一个catch块,jacoco将其标记为未覆盖。在上面贴上一个便条,告诉jacoco你不在乎,继续前进--一旦我们决定使用一个丑陋的库,需要变通方法才能正常工作,我们就离开了“我如何写出漂亮干净的代码”这个主题。
1.这个方法不可能 * 今天 * 抛出它,但你希望它很快就会在不久的将来更新,你希望你正在编写的使用这个方法的代码已经被编写来处理可能在不久的将来相关的情况,在这种情况下,你真的必须模拟它并测试它。Jacoco的警告是“正确的”,因为只要你不测试那个catch块路径,你的测试用例就是不完整的。

如果必须使用assert

如果让一个案例,比如“如果Data.retrieveAll返回null会怎么样”完全不被发现,可能会让人感觉很糟糕。不仅仅是“没有针对该场景的测试”,甚至是“代码甚至没有尝试处理该场景”。但是,你每天都在做。你有没有这样写过:

String v = z.toLowerCase();
assertTrue(v instanceof String);

或者,如果你认为你知道,你已经忘记了测试成千上万的不可能性。这就是为什么我们不测试这些东西:“不可能的奇异事物”的列表是无限的,因此,显然不可能测试所有这些。
然而,也许你认为“data.retrieveAll()返回null”的不可能性仍然是不可能的,但比例如"不可能“。你的JVM以某种方式结束了,toLowerCase()返回的对象是一个非String。如果您必须处理它,我建议您添加一个Assert。不是junitAssert,而是实际的Assert:

assert resultSet != null: "Data.retrieveAll() broke spec and returned null";

返回您的代码片段

你的代码片段中有一个非常重要的bug,你的单元测试不会发现,jacoco也不会为你修复。幸运的是:这是一个很好的例子,说明为什么试图掩盖每一个案件只意味着你最终会用这些东西把自己的腿炸掉。你已经把事情弄得太复杂了,这样做在代码中引入了一个你可能永远不会真正触发的bug。(如果一个bug不可能被触发,它还是bug吗?你必须去问一位哲学家,当你在这的时候,问他们一棵树在森林中福尔斯,当周围没有人听到它的声音时,它是否仍然发出声音。
这里的问题是,如果返回一个空的(不是null,而是空的),您的代码会抛出一个自定义异常**,并带有错误的消息**。“错误/误导性文本”的问题是,你不能对这些文本进行单元测试。注解和方法名也有同样的问题。如何编写一个单元测试来捕获:

int add(int a, int b) {
  return a + b;
}

命名错误,因为它应该被称为plusadd意味着这段代码实际上改变了a(这在java中是不可能的,但是,仍然-错误的名称)?您必须接受消息和命名超出了单元测试的范围,因此也超出了jacoco可以为您做的范围。
以下是编写代码片段的正确方法:

List<Something> listOfSomething = new ArrayList<>();
    try (Data data = DataUtil.get(Constants.SOME_TABLE_NAME)) {
      for (ResultSetRow : data.retrieveAll()) {
        Something something = new Something();
        something.setX(row.get("someColumnX"));
        listOfSomething.add(something);
      }
    } catch (ExceptionOne | ExceptionTwo e) {
      throw new SomeCustomException(e);
    }
    return listOfSomething;
}

注意事项:

  • 它是 * 远 * 短。

  • 它的局部变量具有更好的作用域(声明一次局部变量并在循环中重用它不会带来性能提升。不相信我吗?在两个版本上运行javap,并了解它如何在生成的字节码中产生零差异,从而证明它不可能产生差异。

  • 它只是随意地假设Data.retrieveAll()不可能返回null。这很好--你也不用对"foo".toLowerCase()进行空检查--你相信你调用的方法做了它们的规范说它们做的事情。即使你不这样做,它的效果也正是你想要的:如果retrieveAll不符合其规范并返回null,则会发生NullPointerException。这是件好事NPE的名称是正确的(问题确实是空指针出现在编码器认为它不可能发生的地方),它的堆栈跟踪是正确的(它精确地指向发生这种误解的地方,并且错误代码在跟踪中很高),并且它的消息是正确的(在现代VM上,消息是“retrieveAll()”或类似的东西)。

  • 它内联一个简单的一次性变量。(resultSet作为变量消失)。

  • 它清理了 Package 异常的创建。你不需要重新处理这个信息。

  • 如果你觉得空因确实需要注意,有assert。但是,在这里您不需要这样做-null的情况得到了正确的处理,不需要显式的assert(它会导致NPE)。这就是你想要的)。

  • 相反,如果结果集为空,则不会抛出。但是,为什么要这样做?“没有行”肯定是一个有效的状态。如果它不是一个有效的状态,那么,当然,检查它并抛出一些东西,并模拟Data.retrieveAll,以便您可以创建该状态。您现在测试的是来自Data.retrieveAll的有效响应(即,retrieveAll被指定为这样,它说它确实有可能返回一个结果集,其中有零行),但从getAll()的Angular 来看,这不是一个有效的响应/场景。

  • 不过,这听起来很奇怪。getAll中没有任何东西表明它将“无结果”转换为异常。这意味着至少您的方法可能命名得不好。我打赌正确的答案是接受“没有结果”和“一些结果”一样有效。但是如果没有,要么getAll需要重命名,要么你需要确保上下文(这个类所在的类的名称,或者在例如package-info.java解释了如何在心理上对这个API做什么进行建模),需要让人们真正明白,“没有结果”是一个意外的场景,如果出现就会导致异常。

yh2wf1be

yh2wf1be3#

只是评论Assert,更可取的是使用

assertThat(someServiceMock.getAll()).isEmpty();

它不仅读起来更好,而且错误消息也会显式(它会抱怨你的getAll列表不是空的),而不是非显式的expected true but got false

相关问题