Thread (5 messages) 5 messages, 3 authors, 2022-02-25

Re: [PATCH v2 1/3] stash: add test to ensure reflog --rewrite --updatref behavior

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2022-02-23 21:54:53

Possibly related (same subject, not in this thread)

On Wed, Feb 23 2022, Junio C Hamano wrote:
Ævar Arnfjörð Bjarmason [off-list ref] writes:
quoted
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
 '
If "git stash drop", invoked in earlier part of this test before the
precontext, fails, then test_cmp would fail and we leave
dropped-stash untouched, even though we did run "git stash drop"
already.
Yes, that's an edge case that's exposed here, but which I thought wasn't
worth bothering with. I.e. if you get such a failure on test N getting
N+1 failing as well isn't that big of a deal.

The big deal is rather that we know we're adding a REFFILES dependency
to this, which won't run this at all, which will make the "stash pop"
below fail.
Why does the next test need to depend on what has happened earlier?
They don't need to, and ideally wouldn't, but most of our test suite has
this issue already. Try e.g. running it with:

    prove t[0-9]*.sh :: --run=10-20 --immediate

And for this particular file running e.g. this on master:

    ./t3903-stash.sh --run=1-10,30-40

Will fail 7 tests in the 30-40 range.

So while it's ideal that we can combine tests with arbitrary --run
parameters, i.e. all tests would tear down fully, not depend on earlier
tests etc. we're really far from that being the case in practice.

So insisting on some general refactoring of this file as part of this
series seems a bit overzelous, which is why I'm suggesting the bare
minimum to expect and work around the inevitable REFFILES failure, as
Han-Wen is actively working in that area.
quoted
 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)
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help