diff mbox series

[v2,18/21] t7900: mark pack-refs tests as REFFILES

Message ID ff3b67c84c412dafaa506355e83427a731b47623.1619519903.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series Prepare tests for reftable backend | expand

Commit Message

Han-Wen Nienhuys April 27, 2021, 10:38 a.m. UTC
From: Han-Wen Nienhuys <hanwen@google.com>

Reftable automatically compacts tables on writes. The functionality of
incremental compaction is unittested in reftable/stack_test.c
(test_reftable_stack_auto_compaction)

In addition, pack-refs triggers a full compaction of the entire stack. This is
exercised in t0031-reftable.sh.

Given that git-maintenance simply calls out git-pack-refs, it seems superfluous
to test this further for reftable.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 t/t7900-maintenance.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Ævar Arnfjörð Bjarmason May 20, 2021, 3:40 p.m. UTC | #1
On Tue, Apr 27 2021, Han-Wen Nienhuys via GitGitGadget wrote:

> From: Han-Wen Nienhuys <hanwen@google.com>
>
> Reftable automatically compacts tables on writes. The functionality of
> incremental compaction is unittested in reftable/stack_test.c
> (test_reftable_stack_auto_compaction)
>
> In addition, pack-refs triggers a full compaction of the entire stack. This is
> exercised in t0031-reftable.sh.
>
> Given that git-maintenance simply calls out git-pack-refs, it seems superfluous
> to test this further for reftable.
>
> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
> ---
>  t/t7900-maintenance.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
> index 2412d8c5c006..6f2f55a6c51d 100755
> --- a/t/t7900-maintenance.sh
> +++ b/t/t7900-maintenance.sh
> @@ -343,7 +343,7 @@ test_expect_success 'maintenance.incremental-repack.auto' '
>  	test_subcommand git multi-pack-index write --no-progress <trace-B
>  '
>  
> -test_expect_success 'pack-refs task' '
> +test_expect_success REFFILES 'pack-refs task' '
>  	for n in $(test_seq 1 5)
>  	do
>  		git branch -f to-pack/$n HEAD || return 1

I don't think it's superfluous to test what "git maintenance" does
anyway, i.e. the test ends with:

    test_subcommand git pack-refs --all --prune

Shouldn't we test that we .. don't run that, abort, warn, whatever?
Anyway, as with another earlier comment of mine we're going to have
chicken & egg problems with amending these tests and then actually
introducing reftable support, so maybe "not yet", but I worry in general
about the loss of coverage...
Han-Wen Nienhuys May 31, 2021, 2:08 p.m. UTC | #2
On Thu, May 20, 2021 at 5:41 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> > -test_expect_success 'pack-refs task' '
> > +test_expect_success REFFILES 'pack-refs task' '
> >       for n in $(test_seq 1 5)
> >       do
> >               git branch -f to-pack/$n HEAD || return 1
>
> I don't think it's superfluous to test what "git maintenance" does
> anyway, i.e. the test ends with:
>
>     test_subcommand git pack-refs --all --prune
>
> Shouldn't we test that we .. don't run that, abort, warn, whatever?

Sure. I've removed the check on the side effect for pack-refs instead.
diff mbox series

Patch

diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 2412d8c5c006..6f2f55a6c51d 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -343,7 +343,7 @@  test_expect_success 'maintenance.incremental-repack.auto' '
 	test_subcommand git multi-pack-index write --no-progress <trace-B
 '
 
-test_expect_success 'pack-refs task' '
+test_expect_success REFFILES 'pack-refs task' '
 	for n in $(test_seq 1 5)
 	do
 		git branch -f to-pack/$n HEAD || return 1