Skip to content

fix(test): stop the systemd tests from deleting the real ~/.config/systemd/user - #30

Merged
benedict2310 merged 2 commits into
benedict2310:mainfrom
ytjohn:fix/systemd-test-real-home
Oct 20, 2026
Merged

benedict2310 merged 2 commits into
benedict2310:mainfrom
ytjohn:fix/systemd-test-real-home

Conversation

@ytjohn

@ytjohn ytjohn commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #29.

test/install/systemd.test.ts creates a temp home under tmpdir() but never points HOME at it, then calls the real resolveTelePiInstallContext(). Since that resolver derives its paths from process.env.HOME || homedir() (src/paths.ts), every path the suite touches belongs to whoever runs the tests. Running npm test therefore:

  1. deleted ~/.config/systemd/user recursively — rmSync(path.dirname(ctx.serviceUnitPath), { recursive: true, force: true }), which removes every unit file the user has there, not just telepi's, along with default.target.wants/ and timers.target.wants/;
  2. overwrote the live telepi.service with ExecStart/StandardOutput paths inside the temp dir that afterEach then deletes, leaving the unit pointing at files that no longer exist;
  3. ran real systemctl --user daemon-reload / enable / restart telepi.service from the reconcile test.

Changes

  • Stub HOME to the per-test temp home in beforeEach, and restore process.env in afterEach — the same pattern test/install.test.ts already uses.
  • Stop deleting a directory to prove auto-creation. The test now points serviceUnitPath at a not-yet-created path under tempDir and asserts it is under tempDir, so the destructive shape cannot return if the env stub is ever lost.
  • Mock node:child_process for this file so reconcile/getStatus cannot drive the real service manager.

Verification

  • npx vitest run test/install/systemd.test.ts — 13 passed.
  • Full npm test — 32 files, 441 passed / 1 skipped.
  • Ran the full suite with my real HOME, checksumming ~/.config/systemd/user before and after: unchanged, and telepi's ExecMainStartTimestamp did not move (no restart).

Happy to split the mock into its own commit, or to leave the rmSync test shape alone and rely on the HOME stub only, if you prefer a more minimal change.

ytjohn and others added 2 commits September 21, 2026 09:58
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 benedict2310 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

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.

@ytjohn

ytjohn commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

Hi - is there anything else needed before merge? Thank you.

@benedict2310

Copy link
Copy Markdown
Owner

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-20T14:56:44.844427Z 77b830f Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Delightful!

Reviewed commit: 77b830ffd2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@benedict2310
benedict2310 merged commit b3e19d6 into benedict2310:main Oct 20, 2026
2 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.

npm test deletes the user's real ~/.config/systemd/user directory (all units, not just telepi's)

2 participants