Skip to content

e2e: fix the TestKillMatching flake, wait for the child's command line - #3155

Merged
Tim Hockin (thockin) merged 1 commit into
agent-substrate:mainfrom
dims:e2e-kill-test-cmdline
Oct 23, 2026
Merged

Tim Hockin (thockin) merged 1 commit into
agent-substrate:mainfrom
dims:e2e-kill-test-cmdline

Conversation

@dims

@dims Davanum Srinivas (dims) commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Flake fix. TestKillMatching reads /proc/<pid>/cmdline right after Start. Start returns when the exec closes the child's status pipe, and the kernel does that before it sets up the new argv, so the file can still read empty and killMatching finds nothing: kill_test.go:41: killed 0, want 1 (2026-10-19, 2026-10-23).

Poll for the command line before calling killMatching.

Pinned to one CPU on Linux 6.8 (taskset -c 0 go test -count=3000), the old test failed 4 of 3000 runs with killed 0, the fixed one 0 of 3000.

@dims

Copy link
Copy Markdown
Collaborator Author

@dims
Davanum Srinivas (dims) marked this pull request as ready for review October 23, 2026 14:15
TestKillMatching read /proc/<pid>/cmdline right after Start. Start
returns when the exec closes the child's status pipe, and the kernel
closes it before it sets up the new argument area, so the file can
still read empty and killMatching finds nothing. Poll for the
command line first.

@thockin Tim Hockin (thockin) left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I did not believe this so I had to go test it. TIL

@thockin
Tim Hockin (thockin) added this pull request to the merge queue Oct 23, 2026
Merged via the queue into agent-substrate:main with commit 49d3d45 Oct 23, 2026
15 checks passed
@BenTheElder Benjamin Elder (BenTheElder) added the area/tests Enhancing / fixing test coverage. label Oct 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/tests Enhancing / fixing test coverage.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants