Re: [PATCH v3 2/2] bundle-uri: add test for bundle-uri clones with tags
From: Toon Claes <hidden>
Date: 2025-04-14 12:19:39
Taylor Blau [off-list ref] writes:
quoted hunk ↗ jump to hunk
I think we should be testing that all of the refs we expect to have made it over actually did so. This diff (applied on top of your series) does that:--- 8< --- diff --git a/t/t5558-clone-bundle-uri.sh b/t/t5558-clone-bundle-uri.sh index b1276ba295..9b211a626b 100755 --- a/t/t5558-clone-bundle-uri.sh +++ b/t/t5558-clone-bundle-uri.sh@@ -128,13 +128,12 @@ test_expect_success 'create bundle with tags' ' test_expect_success 'clone with tags bundle' ' git clone --bundle-uri="clone-from-tags/ALL.bundle" \ clone-from-tags clone-tags-path && - git -C clone-tags-path for-each-ref --format="%(refname)" >refs && - grep "refs/bundles/tags/" refs >actual && - cat >expect <<-\EOF && - refs/bundles/tags/A - refs/bundles/tags/B - refs/bundles/tags/tag-A - EOF + + git -C clone-from-tags for-each-ref --format="%(refname:lstrip=1)" \ + >expect && + git -C clone-tags-path for-each-ref --format="%(refname:lstrip=2)" \ + refs/bundles >actual && + test_cmp expect actual ' --- >8 ---While writing the above, I wasn't quite sure how to follow the test setup. It looks like it creates the following structure: $ git log --oneline --graph * d9df450 (HEAD -> base, tag: B) B * 0ddfaf1 (tag: tag-A, tag: A) A , which we could do with just: test_commit A && test_commit B But even then, I don't think we really need to have more than one tag here to exercise this functionality. So I think it would be fine to simplify the test to just create a single tag, which a simple "test_commit A" should do.
Hi Scott, Are you planning to pick up this patch series again? I think it would be really valuable to get this merged. The patch by Taylor above might be worth integrating, other than that I think it should be good to go. Let me know if I can provide any help. -- Toon