已关闭
refactor: package-level mutable globals in repo/sync and pr/sync block parallel testing #409
aflyingto创建于 6月29日关闭于 6月29日
aflyingto
6月29日 评论:
6月29日 评论:
验证记录
- 当前分支: main
- 复现命令: grep -rn 'var gitRun|var gitRunWithEnv|var gitRunInDirWithEnv' pkg/cmd/repo/sync/ pkg/cmd/pr/sync/
- 实际结果:
- pkg/cmd/repo/sync/sync.go:22:
var gitRun = gitpkg.RunWithEnv - pkg/cmd/pr/sync/sync.go:23:
var gitRunWithEnv = gitpkg.RunWithEnv - pkg/cmd/pr/sync/sync.go:24:
var gitRunInDirWithEnv = gitpkg.RunInDirWithEnv
- pkg/cmd/repo/sync/sync.go:22:
- 结论: 问题仍存在,包级可变全局状态阻塞并行测试,继续修复


aflyingto
6月29日 评论:
6月29日 评论:
开发进度
- 根因: repo/sync 和 pr/sync 使用包级 var 声明包装 git 函数以支持测试替换,但引入了包级可变全局状态,阻塞并行测试
- 主要修改: 将 gitRun/gitRunWithEnv/gitRunInDirWithEnv 从包级变量移入 Options struct 字段;pr/sync 的 syncCommits 改为接收函数参数而非访问包级变量
- 测试: go test ./pkg/cmd/repo/sync/ ./pkg/cmd/pr/sync/ — 29 tests passed; go test ./... — 1268 tests passed
- 实际命令验证: 本次重构不改变命令行为,仅变更依赖注入方式。go build 通过,go test 全量通过
- 安全影响: 无硬编码凭证、无认证路径变更
- 风险: risk/medium(classify-change-risk.py 判定为 medium,因修改了运行时路径)
- 未覆盖项: 命令行为本身未被修改,无需行为级回归测试
- 关联 PR: #316


aflyingto
6月29日 评论:
6月29日 评论:
发现来源
架构审查 (Agent 6)
位置
pkg/cmd/repo/sync/sync.go:22— var gitRun = gitpkg.RunWithEnvpkg/cmd/pr/sync/sync.go:23-24— var gitRunWithEnv, gitRunInDirWithEnv证据
两个 sync 命令使用包级 var 声明包装 git 函数以支持测试替换(Go 常见的函数 mock 模式),但引入了包级可变全局状态。
影响
若未来启用并行测试 (go test -parallel),不同测试 goroutine 可能互相覆盖这些变量,导致竞态条件和不确定的测试失败。
建议
将这些函数依赖移入 Options struct 作为可注入的函数字段(遵循 command-template.md 中的行为函数注入模式)。这已在 clone.go、fork.go 等命令中使用。