Files
HomeAgent/internal/plugin/proc/grandchild_design_test.go
JianFeeeee 9020590e13 test(proc): 修 grandchild 测试的三个设计缺陷(不是生产代码问题)
## 定位结论

`TestKillReturnsEvenWhenGrandchildSurvives` 曾在 `go test ./...`(600s 超时)
与 `make test`(20.4s FAIL)里失败,但**单跑 0.24s 通过**、连跑 3 次全绿
⇒ 「单跑绿、合跑红」。查下来是**三个测试设计缺陷**,
生产代码(`process.go`)没问题。

### 缺陷 1:名字说 Survives,实际测的是「被杀」

| 测试 | 源 | 孙进程 | kill(-pgid) 能杀吗 |
| --- | --- | --- | --- |
| …EvenWhenGrandchildSurvives | grandchildPluginSource | sleep 400,**不**设 Setpgid | **能** |
| …WhenGrandchildEscapesProcessGroup | escapingGrandchildSource | sleep 401 + Setsid | **不能** |

`grandchildPluginSource` 自己的注释写着「孙进程**不**设 Setpgid:它要留在
插件的进程组里」⇒ 第一个测试里孙进程不会 Survive。容易让人误以为
「脱组场景已被覆盖」,而它覆盖的是另一个场景。

**已改名** `…WhenGrandchildDiesWithProcessGroup`。

### 缺陷 2:判据数的是全系统进程

两个计数器扫 `/proc` 找 `"sleep 400"` / `"sleep 401"` 字符串,
**不区分父子关系** ⇒ 同机任何命中同样 cmdline 的进程/容器都串味。

原注释记过一次前车之鉴(「我第一版就踩了:明明单跑通过,合跑却红」),
但当时只加了 base 快照,**没解决全局匹配这个根因** —— base 救不了
「别的测试中途拉起 sleep 400」。

**已修**:新增 `procPPid()`,两个计数器都限定 PPid 属于本测试的插件。
顺带补 `e.Name()` 的 `Atoi` 校验(原来会把 /proc/self、/proc/net 也读一遍)。

### 缺陷 3:defer 清理「拿不到 pid 就整个跳过」

`if pid := pluginPid(p); pid > 0 { Kill }` 在 pid 取不到时静默跳过
⇒ 残留 sleep 400 污染后续测试 ⇒ 变成下一个测试的假失败。

**已修**:新增 `cleanupSleepMarkers(marker)`,按唯一 cmdline 标记兜底清理。

## ★ 我被推翻的一个假设

我一度认定根因是 `waitLoop` 里 `p.cmd.Wait()` **先阻塞**、拆管道在**之后**
(`process.go:359-367`)—— Go 的 `exec` 里 `Wait()` 会等 copy goroutine,
而那些要等所有管道写端关闭,孙进程持有着 ⇒ 死锁。

**实测推翻了它**:把拆管道提到 `Wait` 之前,那个测试 **5 次全 FAIL**
(改前只是偶发)。真正的根因是上面三个测试设计问题;「Wait 阻塞」只是
**被孙进程持管道放大**的效应。

⇒ 改生产代码不但没修好,还把偶发变成必现。**先证明因果再动手。**

## 判据:grandchild_design_test.go(4 条)

★ 它是**查源码文本**的,我一改源文件(改名/加 ppid 限定/加兜底清理),
锚点就全过期 ⇒ 三条判据一起红。

⇒ 判据自己被重构打断时,要改的是**判据的锚点**(认新旧两种形态),
不是回退修复。最后把判据①从「解函数体比对 spawn 参数」简化为
「只问名字是否还说 Survives」—— 少耦合一层,少失效一处。

变异测试三个都抓到:改名回 Survives / 抽掉 ppid 限定 / 去掉兜底清理。

## 门禁

- 全量 `go test ./...`:**43 包 ok、0 FAIL**
- `internal/plugin/proc` 连跑 **5 次全绿**(原来会红的地方)
- `go test -race ./internal/plugin/proc/`:ok
- 无 sleep 400/401 残留

## 顺带

`67def30` 之后 README 的 biome 格式化不再 churn:仓库**没有** biome 配置,
格式来自流水线默认 ⇒ 每次提交后它都会把工作区改脏。我这次会话里反复
`git checkout --` 把它丢掉,那是和流水线对抗。**提交后 biome 再跑就是
no-op**,问题根除。
2026-09-28 10:00:10 +08:00

166 lines
6.7 KiB
Go
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

package proc
import (
"os"
"path/filepath"
"strconv"
"strings"
"testing"
)
// 三个测试设计缺陷的判据。全部从**源码文本**提取 —— 这里要防的是
// 「判据与源码漂移」,而这几个缺陷恰恰是漂移造成的。
//
// ## 缺陷 1:名字说 Survives,实际测的是「被杀」
//
// TestKillReturnsEvenWhenGrandchildSurvives spawn grandchildPluginSource
// → sleep 400,**不**设 Setpgid
// → 留在插件进程组内
// → kill(-pgid) **能**杀掉它
//
// grandchildPluginSource 的注释自己写着:
// 「孙进程**不**设 Setpgid:它要留在插件的进程组里,才代表真实场景」
//
// 所以这个测试里孙进程**不会活下来**,与名字里的 Survives 相反。
// 真正测脱组(孙进程活下来)的是
// TestKillReturnsWhenGrandchildEscapesProcessGroup,它用
// escapingGrandchildSource(sleep 401 + Setsid: true)。
//
// ⇒ 两个测试不是「一个多余」,而是**名字与语义对不上**。
// 保留两个可以,但名字必须说清各自测什么。
//
// ## 缺陷 2:判据数的是**全系统**进程数
//
// countShimGrandchildren() 扫 /proc 找 "sleep 400",
// countEscapingGrandchildren() 扫 "sleep 401"。
// 两者都不是「只数自己拉起的」⇒
// 同一台机器上任何其它进程/测试/容器命中同样的 cmdline 就会串味。
// 注释里已经记过一次前车之鉴(「我第一版就踩了」),但只修了 base
// 快照,**没解决全局匹配**这个根因。
//
// ## 缺陷 3:defer 清理依赖 pluginPid,失败就跳过
//
// gc4 的 defer:
// if pid := pluginPid(p); pid > 0 { syscall.Kill(-pid, SIGKILL) }
// 若 pluginPid 拿不到 pid(进程已退出 / 时序未到),清理**整个跳过**
// ⇒ 残留进程污染后续测试。
//
// 运行:go test ./internal/plugin/proc/ -run TestGrandchildTestDesign -v
const testFile = "grandchild_test.go"
func srcText(t *testing.T) string {
t.Helper()
b, err := os.ReadFile(testFile)
if err != nil {
t.Fatalf("读 %s: %v", testFile, err)
}
return string(b)
}
func TestGrandchildTestDesign_SurvivesTestUsesEscapingSource(t *testing.T) {
src := srcText(t)
// ★ 判据只问一件事:**那个用普通源的测试,名字是否还说「Survives」**。
//
// 不去解函数体、不去比对 spawn 参数 —— 那些都会随重构变,
// 而「名字 vs 语义」这层矛盾才是真正要防的回归。
//
// 修复前:func TestKillReturnsEvenWhenGrandchildSurvives → 用普通源 ⇒ 红
// 修复后:改名 DiesWithProcessGroup ⇒ 绿
const oldName = "func TestKillReturnsEvenWhenGrandchildSurvives("
const newName = "func TestKillReturnsWhenGrandchildDiesWithProcessGroup("
hasOld := strings.Contains(src, oldName)
hasNew := strings.Contains(src, newName)
switch {
case hasOld && hasNew:
t.Errorf("两个名字同时存在(%s 与 %s):改名没删干净,go vet 也会报重定义",
oldName, newName)
case hasOld:
// ★ 旧名还在:它 spawn 的是 grandchildPluginSource(sleep 400、
// **不**设 Setpgid、留在插件进程组内 ⇒ kill(-pgid) **能**杀掉它)
// ⇒ 孙进程不会「Survive」,与名字矛盾。
// 真正测脱组存活的是 TestKillReturnsWhenGrandchildEscapesProcessGroup
// (escapingGrandchildSource:sleep 401 + Setsid)。
t.Errorf("TestKillReturnsEvenWhenGrandchildSurvives 用了普通源 " +
"grandchildPluginSource(sleep 400、**不**设 Setpgid、留在进程组内、" +
"kill(-pgid) **能**杀掉它)⇒ 孙进程不会「Survive」,与测试名矛盾。\n" +
" 真正测脱组存活的是 TestKillReturnsWhenGrandchildEscapesProcessGroup" +
"(escapingGrandchildSource:sleep 401 + Setsid)。\n" +
" 修法:改名成 …WhenGrandchildDiesWithProcessGroup(名字与实现一致)," +
"或改用 escaping 源。")
}
}
func TestGrandchildTestDesign_CountersAreGlobalNotOwn(t *testing.T) {
src := srcText(t)
for _, c := range []struct{ fn, marker string }{
// 认新旧两个名字:修复后新增了带 ppid 限定的 …Under 变体
{"countShimGrandchildrenUnder", `"sleep 400"`},
{"countEscapingGrandchildren", `"sleep 401"`},
} {
i := strings.Index(src, "func "+c.fn+"(")
if i < 0 {
t.Errorf("找不到 %s", c.fn)
continue
}
j := strings.Index(src[i:], "\n}\n")
if j < 0 {
continue
}
body := src[i : i+j]
// 扫全系统 /proc 而不看父子关系 ⇒ 会数到别人的进程。
// 修复形态:函数体里有 procPPid(pid) 限定(或名字带 Under)。
hasPPidGate := strings.Contains(body, "procPPid(") ||
strings.Contains(body, "PPid")
if strings.Contains(body, "os.ReadDir(\"/proc\")") && !hasPPidGate {
t.Errorf("%s 扫全系统 /proc 找 %s,不区分父子关系。\n"+
" 同机任何命中同样 cmdline 的进程/容器都会串味,表现为"+
"「合跑红、单跑绿」。\n"+
" 修法:按 PPid 限定为**自己拉起的那几个**,或让插件把自己的孙进程 pid 报上来。",
c.fn, c.marker)
}
}
}
func TestGrandchildTestDesign_CleanupSkippedWhenPidMissing(t *testing.T) {
src := srcText(t)
i := strings.Index(src, "func TestKillReturnsWhenGrandchildDiesWithProcessGroup(")
if i < 0 {
i = strings.Index(src, "func TestKillReturnsEvenWhenGrandchildSurvives(")
}
if i < 0 {
t.Fatal("找不到该测试(新旧名都试过)")
}
j := strings.Index(src[i:], "\n}\n")
body := src[i : i+j]
// defer 里 `if pid := pluginPid(p); pid > 0 { Kill }` ⇒ 拿不到 pid 就整个跳过。
// 修复形态:body 里出现 cleanupSleepMarkers(兜底按 cmdline 清理)。
if strings.Contains(body, "cleanupSleepMarkers(") {
return
}
if strings.Contains(body, "pid > 0") &&
!strings.Contains(body, "else") && !strings.Contains(body, "fallback") {
t.Errorf("defer 清理是「pluginPid(p) > 0 才杀」,pid 拿不到就**整个跳过**清理" +
"⇒ 残留进程污染后续测试。\n" +
" 修法:pid 拿不到时也要兜底(如按唯一 cmdline 标记清理)," +
"或让插件启动时把孙进程 pid 报给宿主。")
}
}
// TestGrandchildTestDesign_CountersHaveSeparateNamespaces 记一条事实,
// 免得以后有人以为两个计数器是同一个。
func TestGrandchildTestDesign_CountersHaveSeparateNamespaces(t *testing.T) {
src := srcText(t)
if !strings.Contains(src, `"sleep 400"`) || !strings.Contains(src, `"sleep 401"`) {
t.Fatal("两个计数器的 sleep 标记应当不同(400 / 401),否则会互相数进去")
}
_ = filepath.Join // 保持 import 有用(若上面某条判据被删也不至于编译失败)
_ = strconv.Itoa
}