Skip to content

feat: harden NativeAOT source generator coverage#402

Merged
latte-gh merged 2 commits into
masterfrom
feat/aot-experience-roadmap
Jul 24, 2026
Merged

feat: harden NativeAOT source generator coverage#402
latte-gh merged 2 commits into
masterfrom
feat/aot-experience-roadmap

Conversation

@liuhaoyang

Copy link
Copy Markdown
Member

Summary

  • refresh roadmap around NativeAOT, migration, cross-cutting packages, and trust assets
  • add Source Generator diagnostics for byref-like parameters and return values
  • add NativeAOT-specific coverage modes and PR checks for 100% unit / 95% E2E scoped coverage

Verification

  • ./.github/scripts/check-coverage.sh collect nativeaot-unit --output /tmp/aspectcore-nativeaot-unit.env && ./.github/scripts/check-coverage.sh assert nativeaot-unit --input /tmp/aspectcore-nativeaot-unit.env
  • ./.github/scripts/check-coverage.sh collect nativeaot-e2e --output /tmp/aspectcore-nativeaot-e2e.env && ./.github/scripts/check-coverage.sh assert nativeaot-e2e --input /tmp/aspectcore-nativeaot-e2e.env
  • dotnet publish tests/AspectCore.NativeAot.E2E/AspectCore.NativeAot.E2E.csproj -c Release -r linux-x64 -o ./publish-aot --no-restore && ./publish-aot/AspectCore.NativeAot.E2E
  • git diff --check

@liuhaoyang liuhaoyang left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

评审总结

方向正确:把 byref-like 参数/返回值的运行时 InvalidProgramException 前移到编译期诊断(ACSG010/ACSG011),并为 NativeAOT 路径建立独立覆盖率门禁。诊断逻辑、规则抽取、测试覆盖都比较扎实。

合入前建议优先处理下面 P1 两项(Debug 构建断、覆盖率取整误杀),P2 的路径匹配最好在 CI 上跑一次确认。其余为可跟进改进项。

Thursday 修复后我再来复审。

Comment thread src/AspectCore.SourceGenerator/Properties/AssemblyInfo.cs
Comment thread .github/scripts/check-coverage.sh Outdated
Comment thread .github/scripts/check-coverage.sh
Comment thread .github/workflows/build-pr-ci.yml
Comment thread src/AspectCore.SourceGenerator/Emit/NativeAotSignatureDiagnostic.cs

@liuhaoyang liuhaoyang left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

复审结论:5 条 review 全部解决 ✅

逐项核对了 7a114c0 fix: address nativeaot coverage review feedback 的 diff:

P1 — AssemblyInfo.cs#if DEBUG ✅ 已解决

补了 #if DEBUG 无 key IVT + #else 带 public key IVT,与 AspectCore.Core/Properties/AssemblyInfo.cs 模式一致。Debug SourceGeneratorDiagnosticTests 8/8 pass 验证了这一点。

P1 — 覆盖率整数除法截断 ✅ 已解决

  • coverage_from_cobertura 改为 f"{covered * 100 / valid:.2f}",输出两位小数。
  • 新增 coverage_passedcoverage + 1e-9 >= threshold 浮点比较,避免边界误杀。
  • assert_coverage^[0-9]+$ 校验换成 coverage_is_number,支持小数。
  • 非 NativeAOT 路径用 root_line_rate + printf '%.0f' 保持原整数行为,向后兼容。
  • 本地验证 100.00%/100%、97.86%/95% 均 pass。

P2 — scope 路径与 Cobertura filename 不匹配 ✅ 已解决

  • 新增 matches_scope:支持 exact match + /suffix + \suffix,兼容 coverlet 不同路径格式。
  • CI 日志输出 Scope matched classes: N 及文件名,便于排障。
  • 本地验证匹配到 3 个目标文件。

P2 — execution check 结论写死 success / 标题 passed ✅ 已解决

  • 标题改为 NativeAOT Test Execution: collected (UT x%, E2E y%)
  • summary 文案明确说明「Threshold enforcement is reported by the NativeAOT Coverage check」,不再误导。

P3 — 只报第一个 byref-like 参数 ✅ 已解决(按建议保留行为 + 注释)

NativeAotSignatureDiagnosticRules.AnalyzeFirstOrDefault 上方补了注释,说明是刻意设计、保持诊断聚焦并沿用已有 params 行为。

额外观察(非阻塞,可后续跟进)

  • coverage_from_coberturascope_files 为空时也会打印 Scope matched classes 调试行,对非 NativeAOT 路径无影响(该函数只在 NativeAOT 模式调用),但如果以后复用要注意。
  • integer_averageint(sum // len) 截断而非四舍五入,与原 bash 整数除法行为一致,OK。

修复质量很好,覆盖率脚本重构后更健壮(去掉了 bc 依赖、统一用 Python 处理浮点)。没有新的阻塞问题。

@latte-gh latte-gh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@latte-gh
latte-gh enabled auto-merge (squash) July 24, 2026 15:16
@latte-gh
latte-gh merged commit d43c4b3 into master Jul 24, 2026
19 checks passed
@latte-gh
latte-gh deleted the feat/aot-experience-roadmap branch July 24, 2026 16:27
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.

2 participants