mirror of
https://github.com/GeWuYou/GFramework.git
synced 2026-05-07 00:39:00 +08:00
fix(tooling): 收口PR评审遗留nitpick
- 修复 PR review 脚本对 failed-test 额外列表格的解析容错 - 清理 AsyncExtensionsTests 中多余的等待并保留参数名断言 - 补充脚本回归测试并同步 analyzer-warning-reduction 恢复点
This commit is contained in:
parent
1c87272f6b
commit
067d72fada
@ -635,7 +635,15 @@ def parse_failed_test_details(block: str) -> list[dict[str, str]]:
|
||||
if table_section is None:
|
||||
return details
|
||||
|
||||
for name_cell, message_cell in re.findall(r"<tr>\s*<td>(.*?)</td>\s*<td>(.*?)</td>\s*</tr>", table_section.group("body"), re.S):
|
||||
row_pattern = re.compile(
|
||||
r"<tr>\s*<td>(?P<name>.*?)</td>\s*<td>(?P<message>.*?)</td>(?:\s*<td>.*?</td>)*\s*</tr>",
|
||||
re.S,
|
||||
)
|
||||
|
||||
# Test Reporter tables may grow extra columns over time; only the first two are required here.
|
||||
for row_match in row_pattern.finditer(table_section.group("body")):
|
||||
name_cell = row_match.group("name")
|
||||
message_cell = row_match.group("message")
|
||||
name = collapse_whitespace(strip_tags(html.unescape(name_cell))).lstrip("❌").strip()
|
||||
failure_message = normalize_failure_message(message_cell)
|
||||
if name:
|
||||
|
||||
@ -0,0 +1,53 @@
|
||||
#!/usr/bin/env python3
|
||||
"""Regression tests for the GFramework PR review fetch helper."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import importlib.util
|
||||
from pathlib import Path
|
||||
import unittest
|
||||
|
||||
|
||||
SCRIPT_PATH = Path(__file__).with_name("fetch_current_pr_review.py")
|
||||
MODULE_SPEC = importlib.util.spec_from_file_location("fetch_current_pr_review", SCRIPT_PATH)
|
||||
if MODULE_SPEC is None or MODULE_SPEC.loader is None:
|
||||
raise RuntimeError(f"Unable to load module from {SCRIPT_PATH}.")
|
||||
|
||||
MODULE = importlib.util.module_from_spec(MODULE_SPEC)
|
||||
MODULE_SPEC.loader.exec_module(MODULE)
|
||||
|
||||
|
||||
class ParseFailedTestDetailsTests(unittest.TestCase):
|
||||
"""Cover failed-test table parsing edge cases for CTRF comments."""
|
||||
|
||||
def test_parse_failed_test_details_ignores_trailing_columns(self) -> None:
|
||||
"""Extra columns should not prevent extracting the name and failure message."""
|
||||
block = """
|
||||
### ❌ **Some tests failed!**
|
||||
<table>
|
||||
<tbody>
|
||||
<tr>
|
||||
<td>❌ RegisterMigration_During_Cache_Rebuild_Should_Not_Leave_Stale_Type_Cache</td>
|
||||
<td><pre>Expected: False\nBut was: True</pre></td>
|
||||
<td>failed</td>
|
||||
<td>35.3s</td>
|
||||
</tr>
|
||||
</tbody>
|
||||
</table>
|
||||
"""
|
||||
|
||||
details = MODULE.parse_failed_test_details(block)
|
||||
|
||||
self.assertEqual(
|
||||
details,
|
||||
[
|
||||
{
|
||||
"name": "RegisterMigration_During_Cache_Rebuild_Should_Not_Leave_Stale_Type_Cache",
|
||||
"failure_message": "Expected: False\nBut was: True",
|
||||
}
|
||||
],
|
||||
)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@ -225,7 +225,7 @@ public class AsyncExtensionsTests
|
||||
/// 测试WithRetry方法遵守ShouldRetry谓词
|
||||
/// </summary>
|
||||
[Test]
|
||||
public async Task WithRetry_Should_Respect_ShouldRetry_Predicate()
|
||||
public void WithRetry_Should_Respect_ShouldRetry_Predicate()
|
||||
{
|
||||
static Task<int> ThrowShouldNotRetry(string parameterName)
|
||||
{
|
||||
@ -245,7 +245,6 @@ public class AsyncExtensionsTests
|
||||
taskFactory.WithRetryAsync(3, TimeSpan.FromMilliseconds(10),
|
||||
ex => ex is not ArgumentException));
|
||||
|
||||
await Task.Delay(50).ConfigureAwait(false); // 等待任务完成
|
||||
Assert.That(attemptCount, Is.EqualTo(1)); // 不应该重试
|
||||
Assert.That(exception, Is.Not.Null);
|
||||
Assert.That(exception!.InnerExceptions, Has.Count.EqualTo(1));
|
||||
|
||||
@ -6,12 +6,14 @@
|
||||
|
||||
## 当前恢复点
|
||||
|
||||
- 恢复点编号:`ANALYZER-WARNING-REDUCTION-RP-080`
|
||||
- 当前阶段:`Phase 80`
|
||||
- 恢复点编号:`ANALYZER-WARNING-REDUCTION-RP-081`
|
||||
- 当前阶段:`Phase 81`
|
||||
- 当前焦点:
|
||||
- `2026-04-27` 已补齐 `$gframework-pr-review` 对 GitHub Test Reporter / CTRF 测试摘要与失败用例详情的提取,当前文本输出可以直接显示失败测试名和 failure message 摘要
|
||||
- `SettingsModelTests.RegisterMigration_During_Cache_Rebuild_Should_Not_Leave_Stale_Type_Cache` 已按 `System.Threading.Lock` 的真实语义修正测试实现,并完成针对性 Release 测试验证
|
||||
- 当前剩余 warning 热点仍集中在 `YamlConfigSchemaValidator*`、`YamlConfigLoader.cs` 与大批量 `MA0048` 文件名拆分;这些 slice 仍高于本轮 PR review / test follow-up 的低风险边界
|
||||
- `2026-04-27` 已复核 PR `#295` 的 latest-head review,确认 `ThrowShouldNotRetry` 的 `ParamName` open thread 属于 stale finding,本地代码已经使用传入值而非 `nameof(parameterName)`
|
||||
- 已清理 `AsyncExtensionsTests.WithRetry_Should_Respect_ShouldRetry_Predicate` 中的冗余 `Task.Delay(50)`,保留 `ParamName == nameof(taskFactory)` 断言锁定契约
|
||||
- 已增强 `.agents/skills/gframework-pr-review/scripts/fetch_current_pr_review.py` 的 failed-test 表格解析,允许 `Name` / `Failure Message` 后出现尾随额外列
|
||||
- 已新增 Python `unittest` 回归用例覆盖“尾随额外列不影响前两列提取”的场景
|
||||
- 当前剩余 warning 热点仍集中在 `YamlConfigSchemaValidator*`、`YamlConfigLoader.cs` 与大批量 `MA0048` 文件名拆分;这些 slice 仍高于本轮 PR review follow-up 的低风险边界
|
||||
|
||||
## 当前活跃事实
|
||||
|
||||
@ -20,18 +22,26 @@
|
||||
- `python3 .agents/skills/gframework-pr-review/scripts/fetch_current_pr_review.py --json-output <current-pr-review-json>`
|
||||
- 最新结果:成功;当前分支对应 PR 为 `#295`
|
||||
- 当前测试报告输出已能显示 `Summary` 统计、失败测试名称,以及 `Name / Failure Message` 表格中的关键信息
|
||||
- 当前 GitHub latest-head review 仍显示 `1` 条 open thread,但该线程指向的 `nameof(parameterName)` 问题已不在本地代码中成立,属于 stale finding
|
||||
- 当前 latest review 中仍有 `2` 条与本地工作树一致的 nitpick:`AsyncExtensionsTests` 冗余等待,以及 failed-test 表格解析对尾随列不鲁棒
|
||||
- 当前直接验证结果:
|
||||
- `python3 .agents/skills/gframework-pr-review/scripts/test_fetch_current_pr_review.py`
|
||||
- 最新结果:成功;`Ran 1 test in 0.000s`, `OK`
|
||||
- `python3 .agents/skills/gframework-pr-review/scripts/fetch_current_pr_review.py --section tests --json-output /tmp/current-pr-review-postfix.json`
|
||||
- 最新结果:成功;真实 PR 评论抓取仍能输出 `2` 份测试报告,失败用例详情保持可见
|
||||
- `dotnet test GFramework.Core.Tests/GFramework.Core.Tests.csproj -c Release --filter "FullyQualifiedName~WithRetry_Should_Respect_ShouldRetry_Predicate"`
|
||||
- 最新结果:成功;`Failed: 0, Passed: 1, Skipped: 0, Total: 1`
|
||||
- `dotnet test GFramework.Game.Tests/GFramework.Game.Tests.csproj -c Release --filter "FullyQualifiedName~RegisterMigration_During_Cache_Rebuild_Should_Not_Leave_Stale_Type_Cache"`
|
||||
- 最新结果:成功;`Failed: 0, Passed: 1, Skipped: 0, Total: 1`
|
||||
- 当前分支 stop-condition 指标:
|
||||
- `git diff --name-only refs/remotes/origin/main...HEAD | wc -l`
|
||||
- 最新结果:`30`
|
||||
- 最新结果:`35`
|
||||
- `git diff --numstat refs/remotes/origin/main...HEAD`
|
||||
- 最新结果:`642` changed lines
|
||||
- 当前批次摘要:
|
||||
- 三轮低风险 warning 清理已在此前验证中将仓库根 warning 从 `639` 降到 `397`
|
||||
- 当前批次的已完成 slice 明细已迁移到归档,active todo 仅保留恢复真值
|
||||
- 本轮新增内容为 PR review 工具链补强与单个失败测试修正,不扩展 warning reduction 的热点清理边界
|
||||
- 本轮新增内容为 PR review nitpick 收口与脚本回归测试补齐,不扩展 warning reduction 的热点清理边界
|
||||
- 当前建议保留到下一波次的候选:
|
||||
- `GFramework.Game/Config/YamlConfigLoader.cs` 的 `MA0158`(单点可修,但文件本身同时承载其他高耦合 warning)
|
||||
- 测试项目中的 `MA0048` 文件名拆分波次(会显著增加 changed-file 数)
|
||||
@ -43,7 +53,7 @@
|
||||
- `MA0158` 迁移涉及 `net8.0` / `net9.0` / `net10.0` 多目标兼容。
|
||||
- 缓解措施:复用 `StoreSelection.cs` 已存在的 `#if NET9_0_OR_GREATER` 专用锁模式,不在 `net8.0` 引入不兼容 API。
|
||||
- 当前 PR open thread 与 CI 失败信号仍依赖新提交进入远端 PR head 才能复核。
|
||||
- 缓解措施:本轮提交后重新执行 `$gframework-pr-review`,同时确认 review thread 与 failed test signal 是否一起收口。
|
||||
- 缓解措施:本轮提交并推送后重新执行 `$gframework-pr-review`,确认 stale open thread 是否被 GitHub 收口,以及两条 nitpick 是否从 latest review 中消失。
|
||||
|
||||
## 活跃文档
|
||||
|
||||
@ -68,6 +78,6 @@
|
||||
|
||||
## 下一步建议
|
||||
|
||||
1. 提交本轮 `$gframework-pr-review` 解析增强、`SettingsModelTests` 修复与 `ai-plan` 同步。
|
||||
2. 推送后重新执行 `$gframework-pr-review`,确认 PR `#295` 的 failed test detail 与 open thread 是否已更新为新 head 真值。
|
||||
1. 提交本轮 `AsyncExtensionsTests` / `$gframework-pr-review` nitpick 修复、Python 回归测试与 `ai-plan` 同步。
|
||||
2. 推送后重新执行 `$gframework-pr-review`,确认 PR `#295` 的 stale open thread、nitpick 与测试报告是否已刷新为新 head 真值。
|
||||
3. 若后续继续推进 warning reduction,建议另开下一波次处理 `YamlConfigLoader.cs` 热点或测试项目 `MA0048` 拆分波次。
|
||||
|
||||
@ -1,38 +1,39 @@
|
||||
# Analyzer Warning Reduction 追踪
|
||||
|
||||
## 2026-04-27 — RP-080
|
||||
## 2026-04-27 — RP-081
|
||||
|
||||
### 阶段:补强 `$gframework-pr-review` 的测试报告提取并修复 `SettingsModelTests` 失败用例
|
||||
### 阶段:核实 PR `#295` 的剩余 nitpick,并补齐脚本解析回归测试
|
||||
|
||||
- 触发背景:
|
||||
- 用户补充了 GitHub Test Reporter / CTRF 的完整 PR 评论,指出现有 `$gframework-pr-review` 输出虽然能显示 `failed=1`,但没有抓到失败测试名称与详细报错
|
||||
- PR 评论中的真实失败用例为 `RegisterMigration_During_Cache_Rebuild_Should_Not_Leave_Stale_Type_Cache`,报错集中在测试通过反射持锁后两个任务提前完成
|
||||
- 用户再次执行 `$gframework-pr-review`,需要根据当前 PR `#295` 的 latest-head review 继续核实哪些反馈仍需在本地处理
|
||||
- 远端 review 显示 `1` 条 open thread 与 `2` 条 nitpick,需要区分 stale finding 与仍然成立的本地问题
|
||||
- 主线程实施:
|
||||
- 扩展 `.agents/skills/gframework-pr-review/scripts/fetch_current_pr_review.py` 的 `parse_test_report`,增加 CTRF `Summary` 统计、紧凑 failed-tests 列表、以及 `Name` / `Failure Message` HTML 表格的提取
|
||||
- 更新 `gframework-pr-review` skill 文档的输出预期,明确脚本现在应提取 GitHub Test Reporter / CTRF 的失败详情
|
||||
- 修正 `GFramework.Game.Tests/Setting/SettingsModelTests.cs`,让反射取得的 `_migrationMapLock` 在 `NET9_0_OR_GREATER` 下通过 `System.Threading.Lock.EnterScope()` 持锁,而不是退化成 `Monitor` 语义
|
||||
- 复核 `/tmp/current-pr-review.json` 与本地 `AsyncExtensionsTests.cs`,确认 open thread 指向的 `nameof(parameterName)` 问题已在现有代码中修复,属于 stale finding
|
||||
- 删除 `GFramework.Core.Tests/Extensions/AsyncExtensionsTests.cs` 中 `WithRetry_Should_Respect_ShouldRetry_Predicate` 的冗余 `Task.Delay(50)`,将测试改回同步断言路径
|
||||
- 调整 `.agents/skills/gframework-pr-review/scripts/fetch_current_pr_review.py` 的 `parse_failed_test_details`,允许 failed-test HTML 表格在 `Name` / `Failure Message` 后追加额外列
|
||||
- 新增 `.agents/skills/gframework-pr-review/scripts/test_fetch_current_pr_review.py`,以 `unittest` 覆盖“尾随额外列不影响前两列提取”的回归场景
|
||||
- 验证里程碑:
|
||||
- `python3 -c "... parse_test_report(...)"`(基于 `/tmp/current-pr-review.json` 的现有原始评论)
|
||||
- 结果:成功;已能解析 `tests=2156`、`passed=2155`、`failed=1`、`duration=35.3s`,并抓到 `RegisterMigration_During_Cache_Rebuild_Should_Not_Leave_Stale_Type_Cache` 的 failure message
|
||||
- `python3 .agents/skills/gframework-pr-review/scripts/fetch_current_pr_review.py --section tests --json-output /tmp/current-pr-review-v2.json`
|
||||
- 结果:成功;文本输出已直接显示失败测试名与报错摘要
|
||||
- `dotnet test GFramework.Game.Tests/GFramework.Game.Tests.csproj -c Release --filter "FullyQualifiedName~RegisterMigration_During_Cache_Rebuild_Should_Not_Leave_Stale_Type_Cache"`
|
||||
- `python3 .agents/skills/gframework-pr-review/scripts/test_fetch_current_pr_review.py`
|
||||
- 结果:成功;`Ran 1 test in 0.000s`, `OK`
|
||||
- `python3 .agents/skills/gframework-pr-review/scripts/fetch_current_pr_review.py --section tests --json-output /tmp/current-pr-review-postfix.json`
|
||||
- 结果:成功;真实 PR 评论抓取仍显示 `2` 份测试报告,失败测试名与 failure message 摘要保持可见
|
||||
- `dotnet test GFramework.Core.Tests/GFramework.Core.Tests.csproj -c Release --filter "FullyQualifiedName~WithRetry_Should_Respect_ShouldRetry_Predicate"`
|
||||
- 结果:成功;`Failed: 0, Passed: 1, Skipped: 0, Total: 1`
|
||||
- 当前结论:
|
||||
- `$gframework-pr-review` 现在已经能把 PR 测试评论里的关键失败用例信息直接提出来,不再只停留在 `failed=1`
|
||||
- `SettingsModelTests` 当前失败是测试锁语义与生产代码不一致导致的误判,已在本地复现并修正通过
|
||||
- 本轮 latest-head review 中只有 `AsyncExtensionsTests` 冗余等待与 failed-test 表格尾随列容错性两个 nitpick 仍与本地代码一致,现已修复
|
||||
- `ThrowShouldNotRetry` 的 `ParamName` open thread 属于 stale finding,本地代码已经符合预期,只需等待新提交进入远端后复核 thread 状态
|
||||
|
||||
## 活跃风险
|
||||
|
||||
- PR 上的 latest-head review thread 与测试报告仍需要等新提交进入远端后再复核。
|
||||
- 缓解措施:提交并推送后重新执行 `$gframework-pr-review`,只以新的 latest-head 和 test report 为准。
|
||||
- `YamlConfigSchemaValidator*`、`YamlConfigLoader.cs` 与 `MA0048` 拆分仍是下一波次的高耦合候选。
|
||||
- 缓解措施:保持本轮边界只处理 PR review 工具链与失败测试,不顺手扩展 warning reduction 范围。
|
||||
- 缓解措施:保持本轮边界只处理 PR review nitpick follow-up,不顺手扩展 warning reduction 范围。
|
||||
|
||||
## 下一步
|
||||
|
||||
1. 完成本轮提交。
|
||||
2. 推送后重新执行 `$gframework-pr-review`,确认 PR `#295` 的 failed test detail 与 unresolved thread 是否已刷新。
|
||||
2. 推送后重新执行 `$gframework-pr-review`,确认 PR `#295` 的 stale open thread 与 nitpick 是否已刷新。
|
||||
|
||||
## 历史归档指针
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user