Skip to content

test(archive-props): pin that an absolute-named symlink keeps its target - #692

Merged
rominf merged 1 commit into
mainfrom
test/archive-props-absolute-named-symlink
Oct 15, 2026
Merged

rominf merged 1 commit into
mainfrom
test/archive-props-absolute-named-symlink

Conversation

@rominf

@rominf rominf commented Oct 15, 2026

Copy link
Copy Markdown
Collaborator

Follow-up to #684, for the automated review's non-blocking note there.

moved_one_level_down returns an entry with an absolute name untouched. It does this before it looks at a symlink target. No test combined an absolute name with a symlink, so a change that hoisted the target's ../ above that early return stayed green.

moving_an_entry_down_keeps_its_symlink_target_in_place now moves a symlink with each absolute name spelling (/x and @OUT/x) and a relative target. It asserts that both the name and the target are unchanged. With the ../ splice moved above the name check, the test fails with "target of /x was rewritten".

Test-only change: no user-observable CLI behaviour changes, so no Gherkin scenario is needed (AGENTS.md §3).

moved_one_level_down returns an entry with an absolute name untouched,
before it looks at a symlink target. No case combined an absolute name
with a symlink, so hoisting the target's ../ above that early return
stayed green. Add one per absolute spelling.

Signed-off-by: Roman Inflianskas <Roman.Inflianskas@amd.com>

@juhovainio juhovainio left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed the diff (via gh pr diff, no checkout). This is a small, test-only addition (+13/-0) that closes a Gherkin/e2e coverage gap left by #684. Both a line-by-line code review and a separate standards/spec pass came back clean - no blocking or non-blocking findings. The new coverage is scoped correctly and doesn't touch any production code paths.

Approving based on the diff read. CI had some GPU/hardware lanes still pending at last check - worth confirming those land green before merge.

@rominf
rominf added this pull request to the merge queue Oct 15, 2026
Merged via the queue into main with commit 7f11a70 Oct 15, 2026
31 checks passed
@rominf
rominf deleted the test/archive-props-absolute-named-symlink branch October 15, 2026 10:10
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.

2 participants