Repository navigation
test(cmux): add rapid spawn/kill IOSurface fixture - #4
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis change introduces a new stress test fixture Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
4a6e4ea to
78a38b0
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 78a38b0. Configure here.
| environment: [ | ||
| "CMUX_RAPID_SPAWN_KILL_APP_PATH": appURL.path, | ||
| "CMUX_RAPID_SPAWN_KILL_ITERATIONS": "3", | ||
| "CMUX_RAPID_SPAWN_KILL_FORCE_WINDOW": "1", |
There was a problem hiding this comment.
Unused force-window env var has no effect
Medium Severity
The CMUX_RAPID_SPAWN_KILL_FORCE_WINDOW environment variable is set by the test to force IOSurface allocation, but the rapid_spawn_kill.sh fixture and the cmux app don't consume it. This makes the variable inert, and the test's assertion for IOSurface allocation relies on default cmux behavior, which could be misleading or flaky.
Reviewed by Cursor Bugbot for commit 78a38b0. Configure here.
| local executable | ||
| executable="$(resolve_executable)" | ||
| mkdir -p "$TMP_ROOT" | ||
| trap 'rm -rf "$TMP_ROOT"' EXIT |
There was a problem hiding this comment.
User-supplied tmpdir wiped recursively on exit
Low Severity
When CMUX_RAPID_SPAWN_KILL_TMPDIR is provided, the script does mkdir -p on the supplied path and then unconditionally rm -rfs it on EXIT. Without $$ or another uniqueness suffix, any pre-existing contents of that directory (including data from concurrent runs sharing the same path) are destroyed. The default value is safe because it embeds the PID, but the user-overridable form is not, and the README only labels it as a "scratch directory" without warning about destructive cleanup.
Reviewed by Cursor Bugbot for commit 78a38b0. Configure here.


Summary
tests/fixtures/rapid_spawn_kill.shto rapidly launch, sample, terminate, and relaunch cmux app processes with no inter-iteration settle time.RapidSpawnKillFixtureTeststo run the fixture under Appleleaks --atExitand enforce a defaultVM: IOSurface <= 50 MBbudget.tests/fixtures/README.md.TDD / Verification
Expected executable fixture at .../tests/fixtures/rapid_spawn_kill.sh.shellcheck tests/fixtures/rapid_spawn_kill.sh.git diff --checkandgit diff --cached --check.PATH="/opt/homebrew/opt/zig@0.15/bin:$PATH" xcodebuild -quiet -project GhosttyTabs.xcodeproj -scheme cmux-unit -configuration Debug -destination 'platform=macOS,arch=arm64' -derivedDataPath /tmp/cmux-p2e-bash-derived -only-testing:cmuxTests/RapidSpawnKillFixtureTests/testRapidSpawnKillFixtureKeepsIOSurfaceFootprintUnderBudget test.12.60 MB; finalVM: IOSurface = 12.60 MB.Notes
EAGAINfromposix_spawn '/opt/homebrew/bin/git'while trying to diff unrelatednode_modules/undici*paths.No space left on device; temporary cmux DerivedData directories created during verification were removed and the final pass used the resolved DerivedData cache.Note
Medium Risk
Adds a new integration-style XCTest that shells out to
leaks/vmmapand repeatedly spawns/terminates the app, which can be timing- and environment-sensitive in CI.Overview
Adds an IOSurface regression harness that rapidly spawns and kills the
cmuxapp, samplesIOSurfacefootprint viavmmap, and reports a peakVM: IOSurface = <N> MBvalue (tests/fixtures/rapid_spawn_kill.sh).Introduces
RapidSpawnKillFixtureTests(wired into thecmuxTestsXcode target) to run the fixture underleaks --atExit, attach the combined output to the test result, and fail if the measured IOSurface footprint exceeds a configurable default budget (50 MB). Documentation for running and configuring the fixture is added undertests/fixtures/README.md.Reviewed by Cursor Bugbot for commit 78a38b0. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Tests
Documentation