Coverage-Check 审核 bug:覆盖率增长时,依旧必选
此 Issue 与 https://jihulab.com/gitlab-cn/gitlab/-/issues/3145 相关,但 https://jihulab.com/gitlab-cn/gitlab/-/issues/3145 里面讨论了不止一个问题,所以单独开一个 Issue 来跟进 Coverage-Check 审核这一个问题。
问题描述
在 MR 中,如果测试覆盖率不变或增长,我们期待 Coverage-Check 审核不是必选项。但客户反馈说有时是必选项:
Except | Actual |
---|---|
影响范围:低概率偶现,SaaS
Self-managed
Upstream
JiHu
都能复现。
问题原因
在创建 MR 时,如果 Target Branch 的最新一条 Pipeline 没有测试覆盖率(比如状态是 Running、Failed、Skipped),那么认为此 MR 需要 Coverage-Check Approval。
在创建 MR 后,即使 Target branch Pipeline 又跑出测试覆盖率了,MR 的 Approval 规则也不会更新。
复现步骤
复现 Demo:https://jihulab.com/test-pages1/test-coverage-2024/-/merge_requests/5
- 创建 main 分支,把生成覆盖率的 Job 设置为
手动
触发 - 触发一条 main 的流水线,不要点击生成覆盖率
- 创建 feature 分支,提高覆盖率
- 触发一条 feature 的流水线,要点击生成覆盖率
- 创建 feature 到 main 的 MR
- 点击第 2 步中的 main 流水线生成覆盖率
- 可以看到下面这种异常状态,复现成功
覆盖率明明增加了,但仍然要求进行审核:
解决方案
方案一
把 MR Pipeline(就是 MR 页面上能看到的 Pipeline,即 Source Branch Pipeline)里对应测试覆盖率的 Job 重新跑一下。
原理:上面问题原因中提到 Target Branch 的更新不会让 MR Approval 规则更新,但 Source Branch 可以。
方案二
客户提到「这个功能基本没用」,那直接关掉 Coverage-Check 就行了呗。
方案三
客户好像在使用手动触发测试 Job,那把手动改成自动就行了。这样基本保证创建 MR 时主分支是有覆盖率的,就不会出现这个问题了。
方案四
把这件事当作 Bug 对待,在 Upstream 修复它。
修复方式是:当 Pipeline 状态变化时,去找到自己 Branch 作为 Target Branch 的 MR,刷新这些 MR 的 Approval 规则。
点击展开 - 修复方案的代码示例
git diff
diff --git a/ee/app/services/ci/sync_reports_to_approval_rules_service.rb b/ee/app/services/ci/sync_reports_to_approval_rules_service.rb
index c4a1c8c432fb..5b57d033651c 100644
--- a/ee/app/services/ci/sync_reports_to_approval_rules_service.rb
+++ b/ee/app/services/ci/sync_reports_to_approval_rules_service.rb
@@ -39,9 +39,15 @@ def sync_coverage_rules
end
def merge_requests_approved_coverage
- pipeline.merge_requests_as_head_pipeline.reject do |merge_request|
+ merge_requests_as_head_pipeline = pipeline.merge_requests_as_head_pipeline.reject do |merge_request|
require_coverage_approval?(merge_request)
end
+
+ merge_requests_as_taget_pipeline = pipeline.merge_requests_as_taget_pipeline.reject do |merge_request|
+ require_coverage_approval?(merge_request)
+ end
+
+ merge_requests_as_head_pipeline + merge_requests_as_taget_pipeline
end
def require_coverage_approval?(merge_request)