Re: [PATCH v2 1/3] stash: add test to ensure reflog --rewrite --updatref behavior
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Feb 23, 2022, 08:54 UTC
- Message-ID
- <220223.864k4q6jpr.gmgdl@evledraar.gmail.com>
- In-Reply-To
- <6e136b62ca4588cc58f2cb59b635eeaf14e6e20d.1645554652.git.gitgitgadget@gmail.com>
On Tue, Feb 22 2022, John Cai via GitGitGadget wrote:
Show 5 quoted lines
> From: John Cai <johncai86@gmail.com> > > There is missing test coverage to ensure that the resulting reflogs > after a git stash drop has had its old oid rewritten if applicable, and > if the refs/stash has been updated if applicable.
This test looks good, and if 3/3 is applied and either of the flags you're passing is omitted they'll fail, so we know we have the missing coverage here.
Show 23 quoted lines
> Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Signed-off-by: John Cai <johncai86@gmail.com>
> ---
> t/t3903-stash.sh | 25 ++++++++++++++++++++++++-
> 1 file changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> index b149e2af441..ec9cc5646d6 100755
> --- a/t/t3903-stash.sh
> +++ b/t/t3903-stash.sh
> @@ -185,10 +185,33 @@ test_expect_success 'drop middle stash by index' '
> test 1 = $(git show HEAD:file)
> '
>
> +test_expect_success 'drop stash reflog updates refs/stash' '
> + git reset --hard &&
> + git rev-parse refs/stash >expect &&
> + echo 9 >file &&
> + git stash &&
> + git stash drop stash@{0} &&
> + git rev-parse refs/stash >actual &&
> + test_cmp expect actual
> +'This one will be portable to the reftable backend.
> +test_expect_success 'drop stash reflog updates refs/stash with rewrite' '
But as I noted in <220222.86fsob88h7.gmgdl@evledraar.gmail.com> (but it was easy to miss) this test will need to depend on REFFILES. So just changing this line to:
test_expect_success REFFILES 'drop stash[...]'
Show 11 quoted lines
> + git reset --hard &&
> + echo 9 >file &&
> + git stash &&
> + oid="$(git rev-parse stash@{0})" &&
> + git stash drop stash@{1} &&
> + cut -d" " -f1-2 .git/logs/refs/stash >actual &&
> + cat >expect <<-EOF &&
> + $(test_oid zero) $oid
> + EOF
> + test_cmp expect actual
> +'Then:
Show 8 quoted lines
> test_expect_success 'stash pop' ' > git reset --hard && > git stash pop && > - test 3 = $(cat file) && > + test 9 = $(cat file) && > test 1 = $(git show :file) && > test 1 = $(git show HEAD:file) && > test 0 = $(git stash list | wc -l)
This test was already a bit broken in needing the preceding tests, but it will break now if REFFILES isn't true, which you can reproduce e.g. with:
./t3903-stash.sh --run=1-16,18-50 -vixd
Perhaps the least sucky solution to that is:
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh index ec9cc5646d6..1d11c9bda20 100755 --- a/t/t3903-stash.sh +++ b/t/t3903-stash.sh @@ -205,13 +205,19 @@ test_expect_success 'drop stash reflog updates refs/stash with rewrite' ' cat >expect <<-EOF && $(test_oid zero) $oid EOF - test_cmp expect actual + test_cmp expect actual && + >dropped-stash ' test_expect_success 'stash pop' ' git reset --hard && git stash pop && - test 9 = $(cat file) && + if test -e dropped-stash + then + test 9 = $(cat file) + else + test 3 = $(cat file) + fi && test 1 = $(git show :file) && test 1 = $(git show HEAD:file) && test 0 = $(git stash list | wc -l)