Skip to content

第 28 篇:PR 审核整改——把测试覆盖率从 64% 提到 81%

本系列第 28 篇。第 27 篇提交 PR 后,收到了审核意见:缺 doc/cjcov/ 覆盖率报告、有 4 条"冒烟断言"不算有效覆盖。这篇记录我怎么读懂审核意见、怎么用 cjcov 生成双版本覆盖率报告、以及踩到的 @OverflowWrapping 大坑。

一、收到的审核意见

PR 提交后,评审给了三条意见:

#维度问题
1项目结构缺少目录 doc/cjcov/
2测试覆盖缺少 cjcov 覆盖率报告(XML/JSON/HTML)
3测试覆盖4 条 @Expect(true, true) 冒烟断言不计有效覆盖

第 3 条点名了具体位置:ByteOrderMarkReaderTest.cj:74ParsedObjectStackTest.cj:12/19/28

💡 什么是"冒烟断言":像 @Expect(true, true) 这种恒真的断言,只能证明"代码没崩溃",但不验证任何实际的返回值或语义。评审认为这种断言不算有效测试覆盖——这个意见很中肯。

二、先搞清楚"要做什么"

老规矩,先别急着动手,先把三条意见拆开:

  1. 第 1、2 条其实是一件事:生成 cjcov 覆盖率报告,放到 doc/cjcov/
  2. 第 3 条:把 4 条恒真断言改成具体的值校验。

然后我对照《仓颉三方库共建计划》的目录规范,确认了报告该放哪:

{package}4cj/
├── doc/                    # 库设计文档、使用文档、LLT 覆盖报告
│   ├── cjcov/              # ← 覆盖率报告放这里
│   ├── feature_api.md
│   └── design.md

所以结论是:覆盖率的中间产物(coverage/cov_output/)都不进仓库,只把最终报告归档到 doc/cjcov/

三、把冒烟断言改成真断言

打开那 4 处一看,改起来很简单——把恒真断言换成对实际返回值的校验:

cangjie
// 改之前:只验证不崩溃
@Expect(true, true)

// 改之后:验证 BOM 检测返回"无字符集"
@Expect(charset.isNone(), true)          // ByteOrderMarkReaderTest.cj

// 改之后:验证栈的真实大小
@Expect(stack.size(), 0)                 // ParsedObjectStackTest.cj
@Expect(s1.size(), 1)
@Expect(s3.size(), 3)

这几行提交后(commit 575041a),PR 的三条意见就都覆盖到了。但真正的重头戏在后面——把覆盖率数字做上去

四、cjcov 是怎么用的

仓颉的覆盖率工具链分两步:

powershell
# 第一步:带覆盖率跑测试,生成 gcno/gcda 原始数据
cjpm test --coverage

# 第二步:用 cjcov 把原始数据转成 HTML/XML/JSON 报告
cjcov -s . -r . -o doc\cjcov\sts-1.1.3\report --html-details -x -j -i "src\plist"

cjcov 关键参数:

参数作用
-s . / -r .源码根 / 报告根
-o <dir>输出目录
--html-details生成逐文件 HTML 明细
-x输出 coverage.xml
-j输出 coverage.json
-i "src\plist"只统计生产源码,排除测试与示例代码

💡 坑 1:cjcov 不认非 ASCII 路径。我的开发目录里有中文(仓颉工具),cjcov-o 参数一旦带中文就报错。所以报告输出目录必须是纯英文(doc\cjcov\... 没问题)。有意思的是,envsetup.ps1 就算在中文路径下也能正常 source,只有 cjcov 的命令行参数挑路径。

💡 坑 2:-i "src\plist" 很关键。不加这个过滤,测试代码本身也会被算进覆盖率分母,数字虚高且没意义。共建计划要的是"生产源码的覆盖率",所以只统计 src\plist

五、第一次跑:64.5%,还发现一个 Bug

第一次生成报告,行覆盖只有 64.46%(1850/2870)。而且跑测试时有个用例直接崩了:

OverflowException
    at NSNumber.hashCode ...

hashCode 的整数溢出

原因是 Java 的 hashCode 惯用写法依赖整数静默溢出回绕

java
// Java:37 * hash 溢出后自动回绕,不报错
hash = 37 * hash + longVal;

而仓颉默认对整数溢出抛异常@OverflowThrowing)。所以 37 * hash 一旦溢出,直接崩。

踩坑:仓颉没有 &* 回绕运算符

我第一反应是找"回绕运算符",试了 &*&+

cangjie
hash = 37 &* hash &+ longVal    // ❌ 编译报错
// error: expected expression after '&', found '*'

仓颉根本没有这种运算符。查了官方文档才知道,仓颉用的是编译标记(注解),标注在函数声明上方:

注解溢出行为
@OverflowThrowing抛异常(默认
@OverflowWrapping回绕(和 Java 一致)
@OverflowSaturating饱和到最大/最小值

所以正确的修法是:函数体保持普通的 */+,在函数上方加一行注解:

cangjie
@OverflowWrapping
public func hashCode(): Int64 {
    var hash = this.numberType
    hash = 37 * hash + this.longVal          // 溢出自动回绕
    hash = 37 * hash + this.doubleVal.hashCode()
    hash
}

给 6 个类的 hashCode(NSNumber、NSData、UID、NSArray、NSDictionary、NSSet)都加上 @OverflowWrapping 后,崩溃消失,NSObjectHash 也从 0% 覆盖变成有覆盖。

💡 经验:从 Java 移植过来时,凡是依赖"整数溢出不报错"的代码(hashCode、哈希混合、位运算累加),在仓颉里都要显式加 @OverflowWrapping,否则默认会抛异常。

六、补测试:64% → 81%

修完 Bug,再针对报告里的"红色文件"补测试。我新增了三个测试文件:

测试文件补了什么
CoverageMiscTest.cj各数据类型的边界值、getter/setter、相等性
CoverageRoundTripTest.cj三种格式的"序列化→反序列化"往返一致性
CoveragePathsTest.cj纯函数(类型检测)+ 异常路径

其中 CoveragePathsTest.cj 专门打异常分支——我发现三大解析器(ASCII/Binary/XML)遇到畸形输入,全都抛 PropertyListFormatException,所以可以统一这样测:

cangjie
// 过短的二进制数据、非法头、缺偏移表……都应抛同一个异常
@ExpectThrows[PropertyListFormatException](BinaryPropertyListParser.parse(shortBytes))
@ExpectThrows[PropertyListFormatException](ASCIIPropertyListParser.parseString("{ key = "))
@ExpectThrows[PropertyListFormatException](XMLPropertyListParser.parse(unknownTagBytes))

一路补下来,行覆盖从 64.46% → 80.87% → 81.67%(2344/2870),测试用例数从 170 增加到 189,全部通过。

七、分支覆盖:实验能力,要老实说明

cjcov 还有个 -b 参数能出分支覆盖,但它目前是实验能力,数字不稳定。我在 summary.md 里明确写清楚:

本仓库以行覆盖为主口径。coverage.xml 中分支相关字段为 0、HTML 里分支列显示 -,即未启用分支统计,避免用不稳定的实验数据误导评审。行为正确性由 189 个单元测试保证。

💡 心得:覆盖率报告的可信度,一半靠数字,一半靠"把话说清楚"。哪些没测、为什么没测,主动讲明白比藏着掖着强。

有一个文件永远是 0%

报告里 PropertyListConverter.cj 显示 0 / 1。查了一下,它是移植时保留的占位类——只有一个私有构造器,没有任何对外方法,根本无法被测试触达。这种情况没必要硬凑,直接在 summary.md 里说明原因即可。

八、双版本报告结构

最后按共建规范,把报告整理成双版本结构,每个版本都自带环境说明和复现命令:

doc/cjcov/
├── README.md                 # 总览 + 复现方式
├── generate-coverage.ps1     # 一键生成脚本(按版本输出)
├── sts-1.1.3/
│   ├── environment.md        # cjc/平台/工具版本
│   ├── command.txt           # 复现命令
│   ├── summary.md            # 行覆盖 + 逐文件表 + 未覆盖原因
│   └── report/               # index.html + 逐文件 HTML + xml/json
└── lts-1.0.5/                # 结构同上

两个版本各跑一次 generate-coverage.ps1(脚本会自动清环境、跑测试、生成报告)。结果:

SDK 版本行覆盖单元测试
STS 1.1.32344 / 2870 = 81.67%189 全通过
LTS 1.0.52344 / 2870 = 81.67%189 全通过

💡 两版本数字完全一致——这其实是好事,说明同一套源码 + 测试在两个 SDK 上行为完全一致,没有版本相关的隐藏分支。

九、这一篇的踩坑总结

现象解决
冒烟断言@Expect(true,true) 不算有效覆盖换成对返回值的具体校验
hashCode 溢出跑测试抛 OverflowException函数上方加 @OverflowWrapping
找不到回绕运算符&*/&+ 编译报错仓颉用注解不用运算符
cjcov 挑路径中文路径参数报错报告输出目录用纯英文
覆盖率虚高测试代码也被统计-i "src\plist" 只算生产源码

十、小结

这一轮整改,表面上是"补个覆盖率报告",实际上顺带揪出了一个真实的溢出 Bug,还把测试从 170 补到 189、覆盖率从 64% 拉到 81%。

给后来者的建议:

  1. 审核意见先拆解再动手,很多条其实是同一件事。
  2. 覆盖率报告只放最终产物,中间的 coverage/cov_output/ 要 gitignore 掉。
  3. 数字之外,把"为什么没测到"讲清楚,评审看的是态度和严谨度。
  4. 从 Java 移植的哈希/累加代码,记得加 @OverflowWrapping

下一步:把整改后的分支推上去,等待 PR 复审。