Skip to content

test(runtime): validate Windows process lifecycle - #2465

Merged
jackwener merged 1 commit into
apache:mainfrom
liugddx:test/windows-process-lifecycle
Aug 8, 2026
Merged

test(runtime): validate Windows process lifecycle#2465
jackwener merged 1 commit into
apache:mainfrom
liugddx:test/windows-process-lifecycle

Conversation

@liugddx

@liugddx liugddx commented Aug 8, 2026

Copy link
Copy Markdown
Member

Summary

  • add a database-free PTY driver test that validates live PID publication, output readiness, forced termination, and exit acknowledgement on Windows ConPTY
  • make detached-descendant abort coverage shell-independent so Windows exercises the real taskkill /T /F path
  • retain the same direct-process test on POSIX for process-group and topology cleanup coverage

Validation

  • npm --workspace @maka/runtime run build
  • node --test --test-reporter=spec packages/runtime/dist/__tests__/child-process-lifecycle.test.js packages/runtime/dist/__tests__/pty-process-driver.test.js (4 pass)
  • focused shell-exec timeout, abort tree cleanup, and taskkill failure tests (3 pass)
  • npm --workspace @maka/runtime run typecheck
  • npx biome check packages/runtime/src/__tests__/pty-process-driver.test.ts packages/runtime/src/__tests__/shell-exec.test.ts
  • git diff --check
  • npm run windows:inventory

Baseline note

The existing ShellRun ConPTY descendant assertion also passes on Windows, but its test file exits non-zero during temporary SQLite cleanup with the separately tracked EBUSY lifecycle issue. This PR does not mix that database cleanup work into process lifecycle validation.

Closes the process-lifecycle validation item in #2142.

@Astro-Han Astro-Han 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.

Thanks for separating process-lifecycle validation from the unrelated SQLite cleanup failure. The database-free PTY test and shell-independent detached-descendant test are focused, use the real production seams, and keep the Windows and POSIX contracts aligned.

P2 — The PID assertion does not yet prove that the published PID belongs to the live child.

The test currently checks only that driver.pid is a positive safe integer. A wrapper or another positive PID would satisfy that assertion as well. Since this test specifically claims to validate ConPTY’s live child PID publication, could the child print its own process.pid and have the test compare that exact value with driver.pid?

This is a focused test-strengthening suggestion rather than a production behavior concern, so I am approving the PR.

Non-blocking follow-up: the new Windows contracts are currently supported by the author’s focused Windows run but are not executed by the repository’s Windows baseline workflow. They could be promoted into a focused blocking Windows lane later.

简体中文

感谢把进程生命周期验证从无关的 SQLite 清理失败中拆出来。这个不依赖数据库的 PTY 测试和与 shell 无关的 detached descendant 测试都比较聚焦,直接使用生产 seam,也保持了 Windows 与 POSIX 契约一致。

P2 — 当前 PID 断言还不能证明发布的 PID 属于真实存活的子进程。

测试目前只检查 driver.pid 是正的安全整数;wrapper PID 或其他正数也能满足这个断言。既然测试明确要验证 ConPTY 发布真实子进程 PID,建议让子进程输出自己的 process.pid,然后与 driver.pid 做精确比较。

这是增强测试证据的建议,不是生产行为问题,因此我仍然 Approve 这条 PR。

非阻塞后续建议:这些新增 Windows 契约目前由作者的聚焦 Windows 运行提供证据,但仓库的 Windows baseline workflow 尚未执行它们。后续可以将其晋升到一个聚焦的阻塞式 Windows lane。

@jackwener
jackwener merged commit c0789df into apache:main Aug 8, 2026
11 checks passed
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.

3 participants