test: correct fixture helper comment on home-scoped task temp root - #22
Merged
Merged
Conversation
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.
Intent
Correct the test comment that says the temp folder is not home-scoped, as a text correction only with no behaviour change. Context: PR #20 (merged) moved each task's temp root from the shared /tmp/fm- to state/.tasktmp/ inside the spawning Firstmate home. The comment above fm_test_remove_spawn_launch_dirs in tests/fixture-tree-helpers.sh, added by PR #16, still says "The per-task /tmp/fm- root is not home-scoped, so it is not this helper's to remove; fm-spawn.sh's own cleanup owns it", which is now wrong.
What Changed
fm_test_remove_spawn_launch_dirsintests/fixture-tree-helpers.sh. It no longer says the per-task/tmp/fm-<id>root is not home-scoped. It now says the task temp root is atstate/<id>.tasktmp/inside the home, whichfm-spawn.sh's header defines, so it gets removed along with the fixture home. The helper itself only removes the launch directories staged in/tmp.Risk Assessment
✅ Low: The change only edits a comment, and the new wording matches the fm-spawn.sh header contract (state/.tasktmp/ inside the spawning home) and the helper body, which only removes /tmp/fm-*+ launch directories.
Testing
This is a comment-only edit to a test helper, so there is nothing to drive live. I confirmed the diff touches only comment lines and that the new text matches fm-spawn.sh's state/<id>.tasktmp/ temp root. A syntax check and the helper's own fixture suite (tests/fm-test-fixtures.test.sh) both passed, which shows the helper still behaves as before. Verdict is no-surface.Evidence: Change diff (comment-only)
Source: Change diff (comment-only)
Evidence: fm-test-fixtures.test.sh output
Source: fm-test-fixtures.test.sh output
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
git diff 3b9d1d8 96d0ac9: checked that every added or removed line is a#comment (a filter for non-comment changed lines matched nothing)grep -n tasktmp bin/fm-spawn.sh: checked the new comment's claim against fm-spawn.sh's header and its TASK_TMP assignmentbash -n tests/fixture-tree-helpers.sh: syntax check of the edited helperbash tests/fm-test-fixtures.test.sh: the helper's own suite, including the exit-cleanup tests that call fm_test_remove_spawn_launch_dirs (rc=0, all ok)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.