Hi Martin
On 06/06/2025 12:31, Martin Ågren wrote:
On Tue, 20 May 2025 at 11:27, Phillip Wood <phillip.wood123@xxxxxxxxx> wrote:
+test_expect_success 'stash -p <pathspec> stash and restores the file' '
+ cat file >expect-file &&
+ echo changed-file >file &&
+ echo changed-other-file >other-file &&
+ echo a | git stash -p file &&
+ test_cmp expect-file file &&
+ echo changed-other-file >expect &&
+ test_cmp expect other-file &&
+ git stash pop &&
+ test_cmp expect other-file &&
+ echo changed-file >expect &&
+ test_cmp expect file
+'
This only exercises the patch machinery fairly trivially: all hunks are
added. The implementation under test could miss `-p` completely and
behave as `git stash push -- file` or some variant of it, and this test
would continue to pass. (Confirmed by editing the test to not use `-p`
and seeing it run successfully.)
It might be worthwhile to set up some more elaborate scenario where you
pick only some hunks, e.g., this (whitespace-damaged) diff:
I avoided doing this because we already have tests that check "git stash
push -p" works correctly when staging a selection of hunks and so I
didn't think it was worth the extra complexity here when I was only
interested it whether we parsed '-p' correctly. However you're right
that the test passes if we ignore '-p' completely so I agree it is worth
changing it. I'll send a re-roll.
Thanks for your thoughtful review
Phillip
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index d24559a328..3b28504126 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -1178,16 +1178,19 @@ test_expect_success 'stash -- <pathspec>
stashes and restores the file' '
'
test_expect_success 'stash -p <pathspec> stash and restores the file' '
- cat file >expect-file &&
- echo changed-file >file &&
+ test_write_lines b c >file &&
+ git commit -m "a few lines" -- file &&
+ test_write_lines a b c d >file &&
+ test_write_lines b c d >expect-file &&
echo changed-other-file >other-file &&
- echo a | git stash -p file &&
+ test_write_lines s y n | git stash -p file &&
test_cmp expect-file file &&
echo changed-other-file >expect &&
test_cmp expect other-file &&
+ test_write_lines b c >file &&
git stash pop &&
test_cmp expect other-file &&
- echo changed-file >expect &&
+ test_write_lines a b c >expect &&
test_cmp expect file
'
Martin
