git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Han-Wen Nienhuys <hanwenn@gmail.com>,
	Han-Wen Nienhuys <hanwen@google.com>
Subject: Re: [PATCH v2 07/11] t1405: mark test for 'git pack-refs' as REFFILES
Date: Fri, 23 Jul 2021 10:28:10 -0700	[thread overview]
Message-ID: <xmqqsg05ndn9.fsf@gitster.g> (raw)
In-Reply-To: <05dead16f1cbc8d967edf101e170049bbe121cb4.1626989327.git.gitgitgadget@gmail.com> (Han-Wen Nienhuys via GitGitGadget's message of "Thu, 22 Jul 2021 21:28:43 +0000")

"Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Han-Wen Nienhuys <hanwen@google.com>
>
> The outcome of the pack-refs operation is not checked. This was apparently
> forgotten in the commit introducing this test: 16feb99d (Mar 26 2017, "t1405:
> some basic tests on main ref store").
>
> I tried adding the obvious check this, but it seems to fail on the freebsd_12
> builder.

These two paragraphs may state the truth, but readers would
appreciate if it is clarified that they are both side notes that do
not necessarily help understanding the change being made with the
patch.  Something like this

    The number of loose ref files under .git/refs/ directory before
    and after running the "git pack-refs" command only matter when
    the files ref-backend is in use, so hide the test behind the
    REFFILES prerequisite.

would be needed before these sidenotes.

With a larger context, as you said, 

    diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh
    index 92b0487324..3517f54961 100755
    --- a/t/t1405-main-ref-store.sh
    +++ b/t/t1405-main-ref-store.sh
    @@ -9,10 +9,14 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME

     RUN="test-tool ref-store main"

    -test_expect_success 'pack_refs(PACK_REFS_ALL | PACK_REFS_PRUNE)' '
    -	test_commit one &&
    +
    +test_expect_success 'setup' '
    +	test_commit one
    +'
    +
    +test_expect_success REFFILES 'pack_refs(PACK_REFS_ALL | PACK_REFS_PRUNE)' '
            N=`find .git/refs -type f | wc -l` &&
            test "$N" != 0 &&
            $RUN pack-refs 3 &&
            N=`find .git/refs -type f | wc -l`
     '

we do count the number of regular files in .git/refs hierarchy after
running "test-tool ref-store main pack-refs 3" (whatever that means).

I think your "obvious check" would have been something like the
attached?  How would that break?  It is not the "'wc -l' pads the
output with extra spaces" we often see on macOS, is it?

Thanks.

 t/t1405-main-ref-store.sh | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git c/t/t1405-main-ref-store.sh w/t/t1405-main-ref-store.sh
index 3517f54961..9dc88d7580 100755
--- c/t/t1405-main-ref-store.sh
+++ w/t/t1405-main-ref-store.sh
@@ -15,10 +15,11 @@ test_expect_success 'setup' '
 '
 
 test_expect_success REFFILES 'pack_refs(PACK_REFS_ALL | PACK_REFS_PRUNE)' '
-	N=`find .git/refs -type f | wc -l` &&
-	test "$N" != 0 &&
+	N=$(find .git/refs -type f | wc -l) &&
+	test $N -ne 0 &&
 	$RUN pack-refs 3 &&
-	N=`find .git/refs -type f | wc -l`
+	N=$(find .git/refs -type f | wc -l) &&
+	test $N -eq 0
 '
 
 test_expect_success 'create_symref(FOO, refs/heads/main)' '

  reply	other threads:[~2021-07-23 17:28 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-19 18:07 [PATCH 0/6] Fix direct filesystem access in various test files Han-Wen Nienhuys via GitGitGadget
2021-07-19 18:07 ` [PATCH 1/6] t6050: use git-update-ref rather than filesystem access Han-Wen Nienhuys via GitGitGadget
2021-07-19 18:07 ` [PATCH 2/6] t1503: mark symlink test as REFFILES Han-Wen Nienhuys via GitGitGadget
2021-07-19 18:07 ` [PATCH 3/6] t6120: use git-update-ref rather than filesystem access Han-Wen Nienhuys via GitGitGadget
2021-07-19 18:07 ` [PATCH 4/6] t7509: " Han-Wen Nienhuys via GitGitGadget
2021-07-19 21:35   ` Junio C Hamano
2021-07-19 18:07 ` [PATCH 5/6] t3320: use git-symbolic-ref " Han-Wen Nienhuys via GitGitGadget
2021-07-19 18:07 ` [PATCH 6/6] t2402: use ref-store test helper to create broken symlink Han-Wen Nienhuys via GitGitGadget
2021-07-22 21:28 ` [PATCH v2 00/11] Fix direct filesystem access in various test files Han-Wen Nienhuys via GitGitGadget
2021-07-22 21:28   ` [PATCH v2 01/11] t6050: use git-update-ref rather than filesystem access Han-Wen Nienhuys via GitGitGadget
2021-07-22 21:28   ` [PATCH v2 02/11] t1503: mark symlink test as REFFILES Han-Wen Nienhuys via GitGitGadget
2021-07-22 21:28   ` [PATCH v2 03/11] t6120: use git-update-ref rather than filesystem access Han-Wen Nienhuys via GitGitGadget
2021-07-22 21:28   ` [PATCH v2 04/11] t3320: use git-symbolic-ref " Han-Wen Nienhuys via GitGitGadget
2021-07-22 21:28   ` [PATCH v2 05/11] t2402: use ref-store test helper to create broken symlink Han-Wen Nienhuys via GitGitGadget
2021-07-22 21:28   ` [PATCH v2 06/11] t1405: use 'git reflog exists' to check reflog existence Han-Wen Nienhuys via GitGitGadget
2021-07-23 17:15     ` Junio C Hamano
2021-07-22 21:28   ` [PATCH v2 07/11] t1405: mark test for 'git pack-refs' as REFFILES Han-Wen Nienhuys via GitGitGadget
2021-07-23 17:28     ` Junio C Hamano [this message]
2021-07-22 21:28   ` [PATCH v2 08/11] t1410: mark test " Han-Wen Nienhuys via GitGitGadget
2021-07-23 17:32     ` Junio C Hamano
2021-07-22 21:28   ` [PATCH v2 09/11] t7064: use update-ref -d to remove upstream branch Han-Wen Nienhuys via GitGitGadget
2021-07-23 17:37     ` Junio C Hamano
2021-07-22 21:28   ` [PATCH v2 10/11] t6500: use "ls -1" to snapshot ref database state Han-Wen Nienhuys via GitGitGadget
2021-07-22 21:28   ` [PATCH v2 11/11] t6001: avoid direct file system access Han-Wen Nienhuys via GitGitGadget
2021-07-23 17:40     ` Junio C Hamano
2021-07-23 17:44   ` [PATCH v2 00/11] Fix direct filesystem access in various test files Junio C Hamano
2021-08-02 13:38     ` Han-Wen Nienhuys
2021-08-02 16:27       ` Junio C Hamano
2021-08-02 16:28       ` Junio C Hamano
2021-08-02 16:53         ` Han-Wen Nienhuys
2021-08-02 16:53   ` [PATCH v3 " Han-Wen Nienhuys via GitGitGadget
2021-08-02 16:53     ` [PATCH v3 01/11] t6050: use git-update-ref rather than filesystem access Han-Wen Nienhuys via GitGitGadget
2021-08-02 16:53     ` [PATCH v3 02/11] t1503: mark symlink test as REFFILES Han-Wen Nienhuys via GitGitGadget
2021-08-02 16:53     ` [PATCH v3 03/11] t6120: use git-update-ref rather than filesystem access Han-Wen Nienhuys via GitGitGadget
2021-08-02 16:53     ` [PATCH v3 04/11] t3320: use git-symbolic-ref " Han-Wen Nienhuys via GitGitGadget
2021-08-02 16:53     ` [PATCH v3 05/11] t2402: use ref-store test helper to create broken symlink Han-Wen Nienhuys via GitGitGadget
2021-08-02 16:53     ` [PATCH v3 06/11] t1405: use 'git reflog exists' to check reflog existence Han-Wen Nienhuys via GitGitGadget
2021-08-02 16:53     ` [PATCH v3 07/11] t1405: mark test for 'git pack-refs' as REFFILES Han-Wen Nienhuys via GitGitGadget
2021-08-02 16:53     ` [PATCH v3 08/11] t1410: mark test " Han-Wen Nienhuys via GitGitGadget
2021-08-02 16:53     ` [PATCH v3 09/11] t7064: use update-ref -d to remove upstream branch Han-Wen Nienhuys via GitGitGadget
2021-08-02 16:53     ` [PATCH v3 10/11] t6500: use "ls -1" to snapshot ref database state Han-Wen Nienhuys via GitGitGadget
2021-08-02 16:53     ` [PATCH v3 11/11] t6001: avoid direct file system access Han-Wen Nienhuys via GitGitGadget
2021-08-02 20:07     ` [PATCH v3 00/11] Fix direct filesystem access in various test files Junio C Hamano
2021-08-02 21:32       ` Junio C Hamano
2021-08-03  9:37         ` Han-Wen Nienhuys
2021-07-22 22:24 ` [PATCH 0/6] " Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqsg05ndn9.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=hanwen@google.com \
    --cc=hanwenn@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).