diff mbox series

[v2,07/11] t1405: mark test for 'git pack-refs' as REFFILES

Message ID 05dead16f1cbc8d967edf101e170049bbe121cb4.1626989327.git.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series Fix direct filesystem access in various test files. | expand

Commit Message

Han-Wen Nienhuys July 22, 2021, 9:28 p.m. UTC 
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.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 t/t1405-main-ref-store.sh | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Junio C Hamano July 23, 2021, 5:28 p.m. UTC | #1 
"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)' '
diff mbox series

Patch

diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh
index 92b04873247..3517f54961d 100755
--- a/t/t1405-main-ref-store.sh
+++ b/t/t1405-main-ref-store.sh
@@ -9,8 +9,12 @@  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 &&