Skip to content

testing.yaml: test_integrity 规则/阈值声明(ADR-0035 / #86,P2-1) - #115

Merged
randypanding merged 1 commit into
mainfrom
p2-1-test-integrity-policy
Aug 20, 2026
Merged

testing.yaml: test_integrity 规则/阈值声明(ADR-0035 / #86,P2-1)#115
randypanding merged 1 commit into
mainfrom
p2-1-test-integrity-policy

Conversation

@randypanding

Copy link
Copy Markdown
Contributor

摘要

P2-1(#86 / ADR-0035)的 policy 面:governance/policy/testing.yaml 新增 test_integrity 节——CI-Workflows test-integrity.yml(配套 PR CI-Workflows#17)运行时拉取本节覆盖检测器内置缺省。

  • 四条规则及严格度:TI-R1 测试文件删除 red / TI-R2 断言净下降 red / TI-R3 新增抑制标记 red / TI-R4 期望值改写 require_adr(调 red 则不可豁免)
  • 模式(ERE)四组:test_file / assertion / suppression / non_source——与检测器内置缺省逐字节一致(双源同值校验已过,本地 python 比对 MATCH×4)
  • escape_hatch:ADR-NNNN 引用式豁免+计数入账(P3-2 [自动合并 P3-2] C1 拆分 core/rest + ADR 实质校验 + 破玻璃自动 revert #96 落地后升级 scope 匹配)
  • fail_closed: true——policy 拉取失败=红
  • active_now 登记 T-12 test_integrity(placement: gate)

验证清单

  • YAML 全量解析通过(safe_load;rules/patterns 结构断言)
  • patterns 与 CI-Workflows scripts/test-integrity.sh 内置缺省逐字节一致(MISMATCH=0)
  • 不触碰 governance/expected-state.json(留给主 agent 集成)
  • 本仓 gate 绿(YAML 全量解析 + adr-required:ADR-0035 已合入 agent-registry)

@coderabbitai

coderabbitai Bot commented Aug 20, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@randypanding, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 8 minutes

Limit details: You’ve used all 10 included reviews currently available.

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

Wait for the limit to reset, then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: af31f4f8-941b-4f8c-9f8d-d09beed0d039

📥 Commits

Reviewing files that changed from the base of the PR and between 1355046 and c9cc61b.

📒 Files selected for processing (1)
  • governance/policy/testing.yaml

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Declare a fail-closed test integrity gate policy

✨ Enhancement ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Registers test integrity as an active pull-request gate.
• Defines four integrity rules with red or ADR-required enforcement.
• Supplies cross-language patterns and fails closed when policy retrieval fails.
Diagram

graph TD
  A["Testing policy"] --> B["CI workflow"] --> C["Integrity detector"] --> D{"Rule triggered?"}
  D -->|No| E["Gate passes"]
  D -->|Red| F["Gate fails"]
  D -->|ADR required| G["ADR waiver"] --> E
  B -->|Fetch failure| F
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Use language-specific AST detectors
  • ➕ Distinguishes semantic assertion changes more accurately than regular expressions
  • ➕ Reduces false positives from comments, strings, and formatting-only edits
  • ➖ Requires separate parsers and maintenance paths for every supported language
  • ➖ Adds substantial complexity beyond the cross-language P2-1 scope
  • ➖ Makes centralized rule portability and byte-for-byte fallback synchronization harder

Recommendation: Keep the declarative, language-neutral ERE policy for P2-1. It centralizes governance, supports the existing shell detector, and provides conservative fail-closed enforcement; AST-based detection is better treated as a later precision enhancement once false-positive data justifies the added complexity.

Files changed (1) +28 / -0

Other (1) +28 / -0
testing.yamlDefine the T-12 test integrity gate policy +28/-0

Define the T-12 test integrity gate policy

• Registers test integrity as an active gate and declares four rules covering test deletion, assertion decline, suppression additions, and expectation rewrites. Adds detector-compatible ERE patterns, ADR-based waiver semantics, and fail-closed policy retrieval behavior.

governance/policy/testing.yaml

@qodo-code-review

qodo-code-review Bot commented Aug 20, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Uppercase assertions evade TI-R2 🐞 Bug ≡ Correctness
Description
TI-R2 的大小写敏感正则无法匹配 ASSERT_EQEXPECT_EQREQUIRE 等常见全大写 C/C++ 断言宏。删除这些断言不会减少检测器统计值,因此可绕过声明为 red
的断言净下降检查。
Code

governance/policy/testing.yaml[87]

+    assertion: '\bassert|\bexpect\(|\bexpect\.|\brequire\(|\brequire\.|\bshould\b|\bt\.Error|\bt\.Fatal|\bself\.assert|\bfail_if\(|\bAssert|\bExpect\(|\bRequire\(|\bSo\('
Relevance

●●● Strong

This is a deterministic correctness gap in a machine-enforced regex that permits assertion-count
bypasses.

PR-#50
PR-#19

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The policy declares regex-level language-independent detection and makes a negative assertion net
red, but the expression contains only lowercase or initial-capital alternatives. Since ERE matching
is case-sensitive here, conventional all-uppercase assertion macros are not counted.

governance/policy/testing.yaml[69-69]
governance/policy/testing.yaml[79-80]
governance/policy/testing.yaml[85-87]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
TI-R2's assertion ERE omits conventional all-uppercase assertion macros, allowing assertion deletion to evade the red gate.

## Issue Context
The policy describes the detector as language-independent and defines any negative assertion net as red. Preserve compatibility with GNU ERE and synchronize the corrected expression with the CI-Workflows built-in default and its tests.

## Fix Focus Areas
- governance/policy/testing.yaml[79-87]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. T-12 severity contradicts policy ⊘ Outdated 🐞 Bug ≡ Correctness
Description
The active_now entry states that all four tampering forms fail directly red, while TI-R4 is
explicitly require_adr and therefore ADR-waivable. This gives policy readers or consumers of the
summary entry a stricter rule than the detailed machine-executable policy actually enforces.
Code

governance/policy/testing.yaml[25]

+     placement: gate, rule: "测试篡改四形态直接红(ADR-0035)", note: "P2-1 第一必需门"}
Relevance

●●● Strong

Recent accepted governance reviews prioritize aligning summaries, executable policy, and waiver
semantics.

PR-#48
PR-#19

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The newly added T-12 summary says all four forms are directly red, but the same added policy
documents ADR-based waivers and assigns TI-R4 require_adr, unlike the red severity assigned to
TI-R1 through TI-R3.

governance/policy/testing.yaml[24-25]
governance/policy/testing.yaml[70-72]
governance/policy/testing.yaml[77-84]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The T-12 active-policy summary says all four rules are directly red, contradicting TI-R4's `require_adr` severity and documented escape hatch.

## Issue Context
Keep the active registry description consistent with the authoritative machine-executable rule semantics so humans and downstream consumers receive the same policy.

## Fix Focus Areas
- governance/policy/testing.yaml[24-25]
- governance/policy/testing.yaml[70-84]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 8 rules
✅ Web pages:
  +2 more
Review mode: ⚖️ Balanced: 这会改变 gate 的机器可执行策略、阈值与 fail-closed 行为,属于有实际行为和 CI 守门影响的配置变更;范围虽小但不适合降为 lite,且逻辑集中、无需 extended 冗余审查。

Grey Divider

Tip of the day
💡 Did you know, you can copy the agent prompt from any finding and feed it to your IDE agent

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

note: "测试文件有删改行且零实现文件变更;调为 red 则不可豁免"}
patterns: # ERE;bash =~ 与 grep -E 语义(\b 为 GNU 扩展,ubuntu runner 原生)
test_file: '((^|/)(tests?|__tests__|__snapshots__|testdata)(/|$)|_test\.go$|\.test\.[cm]?[jt]sx?$|\.spec\.[cm]?[jt]sx?$|(^|/)test_[^/]*\.py$|_test\.py$|_test\.rs$|\.snap$)'
assertion: '\bassert|\bexpect\(|\bexpect\.|\brequire\(|\brequire\.|\bshould\b|\bt\.Error|\bt\.Fatal|\bself\.assert|\bfail_if\(|\bAssert|\bExpect\(|\bRequire\(|\bSo\('

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Uppercase assertions evade ti-r2 🐞 Bug ≡ Correctness

TI-R2 的大小写敏感正则无法匹配 ASSERT_EQEXPECT_EQREQUIRE 等常见全大写 C/C++ 断言宏。删除这些断言不会减少检测器统计值,因此可绕过声明为 red
的断言净下降检查。
Agent Prompt
## Issue description
TI-R2's assertion ERE omits conventional all-uppercase assertion macros, allowing assertion deletion to evade the red gate.

## Issue Context
The policy describes the detector as language-independent and defines any negative assertion net as red. Preserve compatibility with GNU ERE and synchronize the corrected expression with the CI-Workflows built-in default and its tests.

## Fix Focus Areas
- governance/policy/testing.yaml[79-87]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment thread governance/policy/testing.yaml
randypanding added a commit that referenced this pull request Aug 20, 2026
@randypanding
randypanding force-pushed the p2-1-test-integrity-policy branch from 9e1ba78 to c9cc61b Compare August 20, 2026 06:36
@randypanding
randypanding merged commit 820ffda into main Aug 20, 2026
7 checks passed
@randypanding
randypanding deleted the p2-1-test-integrity-policy branch August 20, 2026 06:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant