ci: add cross-platform test workflow (ubuntu + windows × Node 22 & 24)#12
Merged
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the lifecycle tests in test/harperLifecycle.test.ts to support Windows environments by guarding POSIX-specific signal and process-group assertions with an isPosix check, and skipping tests that rely on POSIX-only features. A review comment suggests reducing the graceMs timeout on Windows to 100ms to prevent an unnecessary 2-second delay during test execution.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
heskew
added a commit
that referenced
this pull request
Jun 5, 2026
Per Gemini review on #12: on Windows killHarper's SIGTERM-equivalent (taskkill without /F) doesn't stop a background node process, so the "terminates and waits for exit" test would always wait the full 2000ms grace before force-killing. Use graceMs 100 on Windows; POSIX keeps 2000ms (and still asserts a clean sub-grace SIGTERM exit). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…per tests - .github/workflows/ci.yml: `npm ci && npm test` on ubuntu+windows x Node 22 & 24 (fail-fast off, actions pinned to SHAs). - Regenerate package-lock.json with npm 11 so `npm ci` is in sync on both npm 10 and 11 (CI surfaced missing optional native deps; additive, no dep changes). - Guard the killHarper signal assertions to POSIX (Windows has no real POSIX signals and uses taskkill); short SIGKILL grace on Windows. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
c45eaf9 to
a983753
Compare
Ethan-Arrowood
approved these changes
Jun 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The repo's first CI:
npm ci && npm teston ubuntu + windows × Node 22 & 24 (actions pinned to SHAs).Stacked on #9 because the test suite lives there — I'll retarget to
mainonce #9 merges.killHarpertests OS-aware (Windows has no real POSIX signals and usestaskkill). This is the first real validation of fix: idle-based startup readiness and port-safe teardown recycle #9's Windows kill path — green on both Node versions.package-lock.jsonthat CI surfaced (npm cifailed on missing optional native deps). Regenerated with npm 11 so both npm 10 and 11 accept it — additive only, no dependency changes.Not gated yet:
npm run check/npm run build, which fail on pre-existing type errors (run.ts,targz.ts,@types/node) unrelated to this PR — a good follow-up.🤖 Generated with Claude Code