IT博客汇 | 【开源贡献笔记】前辈在单元测试里留下的 TODO 注释，你该信吗？

【开源贡献笔记】前辈在单元测试里留下的 TODO 注释，你该信吗？

Pil0tXia发表于 2023-06-27 16:35:54

Take a look

记录一种”Looks Good”，甚至单测也能跑通，但实际上起不到作用的单元测试写法。

PR 链接（Merged）：https://github.com/apache/eventmesh/pull/4139

这是 UrlMappingPattern 工具类中的 compile 方法：

public void compile() {
    acquireParamNames();
    String parsedPattern = urlMappingPattern.replaceFirst(URL_FORMAT_REGEX, URL_FORMAT_MATCH_REGEX);
    parsedPattern = parsedPattern.replaceAll(URL_PARAMETER_REGEX, URL_PARAMETER_MATCH_REGEX);
    this.compiledUrlMappingPattern = Pattern.compile(parsedPattern + URL_QUERY_STRING_REGEX);
}

这是现有的 UrlMappingPatternTest 测试类：

public class UrlMappingPatternTest {

    private static final String TEST_URL_MAPPING_PATTERN = "/test/{param1}/path/{param2}";

    private TestUrlMappingPattern urlMappingPattern;

    @Before
    public void setUp() {
        urlMappingPattern = new TestUrlMappingPattern(TEST_URL_MAPPING_PATTERN);
    }

    @Test
    public void testGetMappingPattern() {
        assertEquals("/test/{param1}/path/{param2}", urlMappingPattern.getMappingPattern());
    }
    
    @Test
    public void testCompile() throws NoSuchFieldException, IllegalAccessException {
        //TODO : Fix me to test the method compile(). It is better using Mockito not PowerMockito.
    }

    class TestUrlMappingPattern extends UrlMappingPattern {

        private Pattern compiledUrlMappingPattern;

        public TestUrlMappingPattern(String pattern) {
            super(pattern);
            compiledUrlMappingPattern = mock(Pattern.class);
        }
    }
}

GPT 会在私有字段的获取、子类的归属和正则表达式的替换上犯很多错误，这时候就不能帮我们省事了。前辈留下的注释说要用 Mockito，GPT 就给出了 Mockito.verify 方法，然后在此基础上加上反射和正确的正则表达式，于是 testCompile () 就可以跑通测试方法了。乍一看似乎没什么问题：

@Test
public void testCompile() throws NoSuchFieldException, IllegalAccessException {
    // Obtain compiledUrlMappingPattern field with reflection
        Field compiledUrlMappingPatternField = UrlMappingPattern.class.getDeclaredField("compiledUrlMappingPattern");
        compiledUrlMappingPatternField.setAccessible(true);
    urlMappingPattern.compile();
    
    // Verify that the compiledUrlMappingPattern field is updated
        Pattern compiledPattern = (Pattern) compiledUrlMappingPatternField.get(urlMappingPattern);
        assertNotNull(compiledPattern);
    
    // Verify that the mocked pattern is compiled with the expected regex
    Mockito.verify(urlMappingPattern.compiledUrlMappingPattern)
        .compile("/test/([%\\w-.\\~!$&'\\(\\)\\*\\+,;=:\\[\\]@]+?)/path/([%\\w-.\\~!$&'\\(\\)\\*\\+,;=:\\[\\]@]+?)(?:\\?.*?)?$");
}

但是跑测试类时，会使别的测试方法报错（虽然报错信息依然指向这里）：

org.mockito.exceptions.misusing.UnfinishedVerificationException: 
Missing method call for verify(mock) here:
-> at org.apache.eventmesh.admin.rocketmq.util.UrlMappingPatternTest.testCompile(UrlMappingPatternTest.java:95)

Example of correct verification:
    verify(mock).doSomething()

Also, this error might show up because you verify either of: final/private/equals()/hashCode() methods.
Those methods *cannot* be stubbed/verified.
Mocking methods declared on non-public parent classes is not supported.

at org.apache.eventmesh.admin.rocketmq.util.UrlMappingPatternTest$TestUrlMappingPattern.<init>(UrlMappingPatternTest.java:105)
at org.apache.eventmesh.admin.rocketmq.util.UrlMappingPatternTest.setUp(UrlMappingPatternTest.java:44)

原因分析

首先，想要使用 Mockito.verify 来验证是否使用预期的参数调用了指定方法，其验证的对象必须是一个 mock 对象。UrlMappingPatternTest 测试类的 TestUrlMappingPattern 子类中提供了一个已经被 mock 过的对象。

mock 的主要作用是模拟对象预期的行为，而这里只需要将预期的值与实际的值相比较即可，不需要模拟行为，所以只需要利用反射获取 UrlMappingPattern 类中的私有字段即可，然后用 assertEquals 断言判断。

但是，因为子类中继承了超类的构造方法、mock 了 compiledUrlMappingPattern 并且在 UrlMappingPatternTest 测试类中被实例化为 urlMappingPattern，所以，在反射中使用 urlMappingPattern.getclass()，获取到的将是 TestUrlMappingPattern 子类中 mock 的 compiledUrlMappingPattern 字段，而该 mock 字段是没有初始化的，不应该被用作比较。从这里可以看出报错信息是不够准确的。

正确的做法是在反射中使用 UrlMappingPattern.class，这样获取的才是实际的 compiledUrlMappingPattern。

@Test
    public void testCompile() throws NoSuchFieldException, IllegalAccessException {
        // Obtain compiledUrlMappingPattern field with reflection
        Field compiledUrlMappingPatternField = UrlMappingPattern.class.getDeclaredField("compiledUrlMappingPattern");
        compiledUrlMappingPatternField.setAccessible(true);

        urlMappingPattern.compile();

        // Verify that the compiledUrlMappingPattern field is updated
        Pattern compiledPattern = (Pattern) compiledUrlMappingPatternField.get(urlMappingPattern);
        assertNotNull(compiledPattern);

        // Verify that the mocked pattern is compiled with the expected regex
        String expectedRegex = "/test/([%\\w-.\\~!$&'\\(\\)\\*\\+,;=:\\[\\]@]+?)/path/([%\\w-.\\~!$&'\\(\\)\\*\\+,;=:\\[\\]@]+?)(?:\\?.*?)?$";
        Pattern expectedPattern = Pattern.compile(expectedRegex);
        assertEquals(expectedPattern.pattern(), compiledPattern.pattern());
    }

点题

那么话说回来，这最后不是完全没用上 Mockito 吗？

是的，确实用不上，也没有用它的理由。代码库里的注释很重要，但也不要被误导了。

如果你一定要用 Mockito，当然也可以，但你必须要让 urlMappingPattern.compiledUrlMappingPattern 返回预期的结果，所以你只能在 TestUrlMappingPattern 子类中重写 compile 方法，也不得不把私有的 acquireParamNames 方法和字符串常量临时标记为 public：

@Override
public void compile() {
    acquireParamNames();
    String parsedPattern = getMappingPattern().replaceFirst(URL_FORMAT_REGEX, URL_FORMAT_MATCH_REGEX);
    parsedPattern = parsedPattern.replaceAll(URL_PARAMETER_REGEX, URL_PARAMETER_MATCH_REGEX);
    this.compiledUrlMappingPattern = Mockito.mock(Pattern.class);
    Mockito.when(compiledUrlMappingPattern.pattern()).thenReturn(parsedPattern + URL_QUERY_STRING_REGEX);
}

于是，testCompile 测试方法通过了，但其它的测试方法又报错失败了，因为你给它们引用的 urlMappingPattern.compiledUrlMappingPattern 制造了额外的行为。

现在你还想继续按注释说的做吗？😉

最后，感谢为我 Review 并提出宝贵意见的贡献者们，在 PR 下思维的碰撞是一件很令人喜悦的事情。