git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v3 2/9] t5520: ensure origin refs are updated

From
Junio C Hamano <gitster@pobox.com>
Date
May 13, 2015, 14:27 UTC
Message-ID
<xmqqegmkbliw.fsf@gitster.dls.corp.google.com>
In-Reply-To
<1431508136-15313-3-git-send-email-pyokagan@gmail.com>
Paul Tan <pyokagan@gmail.com> writes:
Show 8 quoted lines
> Should all of the tests before "setup for avoiding reapplying old
> patches" fail or be skipped, the repo "dst" will not have fetched the
> updated refs from origin. To be resilient against such failures, run
> "git fetch origin".
>
> Signed-off-by: Paul Tan <pyokagan@gmail.com>
> ---
> * Hmm, no reviews the last round?
It is not unusual when the change is trivially correct.

I do not think this hurts, but I do not think that this is vastly better, either. If you suspect that the previous one may fail but you at the same time are so trusting that the one before that one would succeed, yes, this will help in that case. But if you suspect the previous one may fail, the one before that may have also failed, in which case this may not be sufficient (the result of fetching may not match what this test expects to see).

It all depends on where our paranoia ends. The current code is very much trusting all previous ones equally, and accepts "upon the first error, all bets are off for the later ones". With this patch, it becomes slightly less trusting.

If everything else were equal, I would say this change is a "Meh" to me, but I think the change improves this test in a different way.

It begins with "Run 'git rebase --abort', just in case"; which is a signal that it does consider that the previous one may have failed and attempts to prepare for that possibility, while trusting the one before that would have succeeded. And under that assumption, what it currently does is _not_ consistent; the previous "pull --rebase" may have failed in the "rebase" phase, in which case "abort just in case" is a good measure to go back to the clean state, but it may have failed in the "fetch" phase, in which case "abort" does not help. And this patch is needed to fix that inconsistency.

If justified in that way in the log message, then I wouldn't have said "I do not think that this is vastly better", I think.

Show 15 quoted lines
>  t/t5520-pull.sh | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
> index 20ad373..14a9280 100755
> --- a/t/t5520-pull.sh
> +++ b/t/t5520-pull.sh
> @@ -339,6 +339,7 @@ test_expect_success 'git pull --rebase detects upstreamed changes' '
>  test_expect_success 'setup for avoiding reapplying old patches' '
>  	(cd dst &&
>  	 test_might_fail git rebase --abort &&
> +	 git fetch origin &&
>  	 git reset --hard origin/master
>  	) &&
>  	git clone --bare src src-replace.git &&
Previous: Paul TanNext: Paul Tan
Message 15 of 25 in “Improve git-pull test coverage”
  1. 0/9 Improve git-pull test coveragePaul Tan, May 13, 2015
  2. 1/9 t5520: fixup file contents comparisonsPaul Tan, May 13, 2015
  3. Junio C HamanoMay 13, 2015
  4. Junio C HamanoMay 13, 2015
  5. Michael BlumeMay 14, 2015
  6. Junio C HamanoMay 14, 2015
  7. Paul TanMay 15, 2015
  8. Junio C HamanoMay 15, 2015
  9. Junio C HamanoMay 15, 2015
  10. Paul TanMay 16, 2015
  11. Junio C HamanoMay 16, 2015
  12. Junio C HamanoMay 16, 2015
  13. Paul TanMay 17, 2015
  14. 2/9 t5520: ensure origin refs are updatedPaul Tan, May 13, 2015
  15. Junio C HamanoMay 13, 2015
  16. Paul TanMay 18, 2015
  17. 3/9 t5520: test no merge candidates casesPaul Tan, May 13, 2015
  18. 4/9 t5520: test for failure if index has unresolved entriesPaul Tan, May 13, 2015
  19. Matthieu MoyMay 13, 2015
  20. Paul TanMay 15, 2015
  21. 5/9 t5520: test work tree fast-forward when fetch updates headPaul Tan, May 13, 2015
  22. 6/9 t5520: test --rebase with multiple branchesPaul Tan, May 13, 2015
  23. 7/9 t5520: test --rebase failure on unborn branch with indexPaul Tan, May 13, 2015
  24. 8/9 t5521: test --dry-run does not make any changesPaul Tan, May 13, 2015
  25. 9/9 t5520: check reflog action in fast-forward mergePaul Tan, May 13, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.