Repository navigation
Conversation
test/install/systemd.test.ts built a temp home but never pointed HOME at it, then called the real resolveTelePiInstallContext(). Every path it used was therefore the developer's own: the suite deleted ~/.config/systemd/user recursively (all units, not just telepi's, plus the *.target.wants symlinks), overwrote the live telepi.service with paths inside a temp dir that afterEach then removed, and ran a real 'systemctl --user daemon-reload/enable/restart'. - stub HOME to the per-test temp home (as test/install.test.ts already does) and restore process.env afterwards - do not delete a directory to prove auto-creation: point serviceUnitPath at a path under tempDir that does not exist yet, and assert it is under tempDir - mock node:child_process so reconcile/getStatus cannot drive the real service manager Fixes benedict2310#29
benedict2310
approved these changes
Sep 24, 2026
benedict2310
left a comment
Owner
There was a problem hiding this comment.
Reviewed the complete diff and added deterministic success-path and exact failure/status assertions in 77b830f. Independent local Codex review found no defects. Local build and coverage pass: 443 tests, 86.87% lines/statements, 81.01% branches, 90.27% functions. All 14 focused tests also pass in a network-isolated, non-root Podman container. The temporary HOME, non-destructive directory creation test, and child-process mock should stay together. Awaiting CI for the updated head before merge.
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.
Fixes #29.
test/install/systemd.test.tscreates a temp home undertmpdir()but never pointsHOMEat it, then calls the realresolveTelePiInstallContext(). Since that resolver derives its paths fromprocess.env.HOME || homedir()(src/paths.ts), every path the suite touches belongs to whoever runs the tests. Runningnpm testtherefore:~/.config/systemd/userrecursively —rmSync(path.dirname(ctx.serviceUnitPath), { recursive: true, force: true }), which removes every unit file the user has there, not just telepi's, along withdefault.target.wants/andtimers.target.wants/;telepi.servicewithExecStart/StandardOutputpaths inside the temp dir thatafterEachthen deletes, leaving the unit pointing at files that no longer exist;systemctl --user daemon-reload / enable / restart telepi.servicefrom thereconciletest.Changes
HOMEto the per-test temp home inbeforeEach, and restoreprocess.envinafterEach— the same patterntest/install.test.tsalready uses.serviceUnitPathat a not-yet-created path undertempDirand asserts it is undertempDir, so the destructive shape cannot return if the env stub is ever lost.node:child_processfor this file soreconcile/getStatuscannot drive the real service manager.Verification
npx vitest run test/install/systemd.test.ts— 13 passed.npm test— 32 files, 441 passed / 1 skipped.HOME, checksumming~/.config/systemd/userbefore and after: unchanged, and telepi'sExecMainStartTimestampdid not move (no restart).Happy to split the mock into its own commit, or to leave the
rmSynctest shape alone and rely on theHOMEstub only, if you prefer a more minimal change.