fetch: increase test coverage of fetches
When using git-fetch(1) with the `--atomic` flag the expectation is that either all of the references are updated, or alternatively none are in case the fetch fails. While we already have tests for this, we do not have any tests which exercise atomicity either when pruning deleted refs or when backfilling tags. This gap in test coverage hides that we indeed don't handle atomicity correctly for both of these cases. Add test cases which cover these testing gaps to demonstrate the broken behaviour. Note that tests are not marked as `test_expect_failure`: this is done to explicitly demonstrate the current known-wrong behaviour, and they will be fixed up as soon as we fix the underlying bugs. While at it this commit also adds another test case which demonstrates that backfilling of tags does not return an error code in case the backfill fails. This bug will also be fixed by a subsequent commit. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Patrick Steinhardt committed
Feb 17, 2022 at 14:04 UTC
2a0cafd464709cfa22fe7249290c644a2a26c520
2 files changed
+114
t/t5503-tagfollow.sh
+81
@@ -160,4 +160,85 @@ test_expect_success 'new clone fetch main and tags' '
160
test_cmp expect actual
161
'
162
163
+test_expect_success 'atomic fetch with failing backfill' '
164
+ git init clone3 &&
165
+
166
+ # We want to test whether a failure when backfilling tags correctly
167
+ # aborts the complete transaction when `--atomic` is passed: we should
168
+ # neither create the branch nor should we create the tag when either
169
+ # one of both fails to update correctly.
170
+ #
171
+ # To trigger failure we simply abort when backfilling a tag.
172
+ write_script clone3/.git/hooks/reference-transaction <<-\EOF &&
173
+ while read oldrev newrev reference
174
+ do
175
+ if test "$reference" = refs/tags/tag1
176
+ then
177
+ exit 1
178
+ fi
179
+ done
180
+ EOF
181
+
182
+ test_must_fail git -C clone3 fetch --atomic .. $B:refs/heads/something &&
183
+
184
+ # Creation of the tag has failed, so ideally refs/heads/something
185
+ # should not exist. The fact that it does demonstrates that there is
186
+ # a bug in the `--atomic` flag.
187
+ test $B = "$(git -C clone3 rev-parse --verify refs/heads/something)"
188
+'
189
+
190
+test_expect_success 'atomic fetch with backfill should use single transaction' '
191
+ git init clone4 &&
192
+
193
+ # Fetching with the `--atomic` flag should update all references in a
194
+ # single transaction, including backfilled tags. We thus expect to see
195
+ # a single reference transaction for the created branch and tags.
196
+ cat >expected <<-EOF &&
197
+ prepared
198
+ $ZERO_OID $B refs/heads/something
199
+ $ZERO_OID $S refs/tags/tag2
200
+ committed
201
+ $ZERO_OID $B refs/heads/something
202
+ $ZERO_OID $S refs/tags/tag2
203
+ prepared
204
+ $ZERO_OID $T refs/tags/tag1
205
+ committed
206
+ $ZERO_OID $T refs/tags/tag1
207
+ EOF
208
+
209
+ write_script clone4/.git/hooks/reference-transaction <<-\EOF &&
210
+ ( echo "$*" && cat ) >>actual
211
+ EOF
212
+
213
+ git -C clone4 fetch --atomic .. $B:refs/heads/something &&
214
+ test_cmp expected clone4/actual
215
+'
216
+
217
+test_expect_success 'backfill failure causes command to fail' '
218
+ git init clone5 &&
219
+
220
+ write_script clone5/.git/hooks/reference-transaction <<-EOF &&
221
+ while read oldrev newrev reference
222
+ do
223
+ if test "\$reference" = refs/tags/tag1
224
+ then
225
+ # Create a nested tag below the actual tag we
226
+ # wanted to write, which causes a D/F conflict
227
+ # later when we want to commit refs/tags/tag1.
228
+ # We cannot just `exit 1` here given that this
229
+ # would cause us to die immediately.
230
+ git update-ref refs/tags/tag1/nested $B
231
+ exit \$!
232
+ fi
233
+ done
234
+ EOF
235
+
236
+ # Even though we fail to create refs/tags/tag1 the below command
237
+ # unexpectedly succeeds.
238
+ git -C clone5 fetch .. $B:refs/heads/something &&
239
+ test $B = $(git -C clone5 rev-parse --verify refs/heads/something) &&
240
+ test $S = $(git -C clone5 rev-parse --verify tag2) &&
241
+ test_must_fail git -C clone5 rev-parse --verify tag1
242
+'
243
+
244
test_done
t/t5510-fetch.sh
+33
@@ -343,6 +343,39 @@ test_expect_success 'fetch --atomic --append appends to FETCH_HEAD' '
343
test_cmp expected atomic/.git/FETCH_HEAD
344
'
345
346
+test_expect_success 'fetch --atomic --prune executes a single reference transaction only' '
347
+ test_when_finished "rm -rf \"$D\"/atomic" &&
348
+
349
+ cd "$D" &&
350
+ git branch scheduled-for-deletion &&
351
+ git clone . atomic &&
352
+ git branch -D scheduled-for-deletion &&
353
+ git branch new-branch &&
354
+ head_oid=$(git rev-parse HEAD) &&
355
+
356
+ # Fetching with the `--atomic` flag should update all references in a
357
+ # single transaction. It is currently missing coverage of pruned
358
+ # references though, and as a result those may be committed to disk
359
+ # even if updating references fails later.
360
+ cat >expected <<-EOF &&
361
+ prepared
362
+ $ZERO_OID $ZERO_OID refs/remotes/origin/scheduled-for-deletion
363
+ committed
364
+ $ZERO_OID $ZERO_OID refs/remotes/origin/scheduled-for-deletion
365
+ prepared
366
+ $ZERO_OID $head_oid refs/remotes/origin/new-branch
367
+ committed
368
+ $ZERO_OID $head_oid refs/remotes/origin/new-branch
369
+ EOF
370
+
371
+ write_script atomic/.git/hooks/reference-transaction <<-\EOF &&
372
+ ( echo "$*" && cat ) >>actual
373
+ EOF
374
+
375
+ git -C atomic fetch --atomic --prune origin &&
376
+ test_cmp expected atomic/actual
377
+'
378
+
379
test_expect_success '--refmap="" ignores configured refspec' '
380
cd "$TRASH_DIRECTORY" &&
381
git clone "$D" remote-refs &&