All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, chakrabortyabhradeep79@gmail.com,
	Taylor Blau <me@ttaylorr.com>,
	Derrick Stolee <derrickstolee@github.com>
Subject: Re: [PATCH v2 1/2] t7700: check post-condition in kept-pack test
Date: Fri, 25 Mar 2022 10:07:04 -0700	[thread overview]
Message-ID: <xmqqmthearlz.fsf@gitster.g> (raw)
In-Reply-To: <f2f8d12929bcbd630b2de3ce770a6763989ffcff.1648146897.git.gitgitgadget@gmail.com> (Derrick Stolee via GitGitGadget's message of "Thu, 24 Mar 2022 18:34:56 +0000")

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> +get_sorted_objects_from_packs () {
> +	git show-index <$(cat) >raw &&
> +	cut -d" " -f2 raw | sort
> +}

As pointed out by Taylor, this "the standard input gives us the name
of a file to be read" looks strange.  It may work, and it may even
give the easiest interface to use by all the callers, but if we were
designing a more generic helper function suitable to be added to the
test-lib*.sh, we wouldn't design it like so---instead it would be
either "we read the contents of the .idx file from the standard
input" or "the first argument is the name of the .idx file".

>  test_expect_success '--write-midx -b packs non-kept objects' '
> +	git init repo &&
> +	test_when_finished "rm -fr repo" &&
> +	(
> +		cd repo &&
> +
> +		# Create a kept pack-file
> +		test_commit base &&
> +		git repack -ad &&
> +		find $objdir/pack -name "*.idx" >before &&
> +		>$objdir/pack/$(basename $(cat before) .idx).keep &&

We probably want to sanity check "repack -a" by insisting "before"
has found exactly one .idx file, before using it this way.

		test_line_count = 1 before &&
		before=$(cat before) &&
		>"${before%.idx}.keep"

> +		# Create a non-kept pack-file
> +		test_commit other &&
> +		git repack &&
> +
> +		# Create loose objects
> +		test_commit loose &&
> +
> +		# Repack everything
> +		git repack --write-midx -a -b -d &&
> +
> +		# There should be two pack-files now, the
> +		# old, kept pack and the new, non-kept pack.
> +		find $objdir/pack -name "*.idx" | sort >after &&
> +		test_line_count = 2 after &&

OK.  "after" gets sorted because we will pass it to "comm" later.

> +		find $objdir/pack -name "*.keep" >kept &&
> +		test_line_count = 1 kept &&

Since we've made sure "before" is a one-liner earlier, we could just
say

		test_cmp before kept &&

instead, no?

> +		# Get object list from the kept pack.
> +		get_sorted_objects_from_packs \
> +			<before \
> +			>old.objects &&
		
OK.

> +		# Get object list from the one non-kept pack-file
> +		comm -13 before after >new-pack &&

OK.  This should give only one line of output but that is merely
assumed.  We know after has 2 and before has 1, but haven't made
sure that before is a subset of after.

		test_line_count = 1 new-pack &&

> +		get_sorted_objects_from_packs \
> +			<new-pack \
> +			>new.objects &&

OK.

> +		# None of the objects in the new pack should
> +		# exist within the kept pack.
> +		comm -12 old.objects new.objects >shared.objects &&

Great.

> +		test_must_be_empty shared.objects
> +	)
>  '
>  
>  test_expect_success TTY '--quiet disables progress' '

  parent reply	other threads:[~2022-03-25 17:07 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-21 20:34 [PATCH] test-lib-functions: fix test_subcommand_inexact Derrick Stolee via GitGitGadget
2022-03-22 15:17 ` Derrick Stolee
2022-03-23 14:53   ` Junio C Hamano
2022-03-23 14:55     ` Derrick Stolee
2022-03-23 21:45       ` Taylor Blau
2022-03-23 23:10         ` Junio C Hamano
2022-03-24 15:42           ` Derrick Stolee
2022-03-24 16:02             ` Taylor Blau
2022-03-24 16:39               ` Derrick Stolee
2022-03-24 16:38             ` Junio C Hamano
2022-03-24 18:10     ` Abhradeep Chakraborty
2022-03-25  0:33       ` Junio C Hamano
2022-03-25  8:13         ` Abhradeep Chakraborty
2022-03-24 18:34 ` [PATCH v2 0/2] " Derrick Stolee via GitGitGadget
2022-03-24 18:34   ` [PATCH v2 1/2] t7700: check post-condition in kept-pack test Derrick Stolee via GitGitGadget
2022-03-24 18:58     ` Taylor Blau
2022-03-25 13:55       ` Derrick Stolee
2022-03-25 17:07     ` Junio C Hamano [this message]
2022-03-25 17:23       ` Derrick Stolee
2022-03-25 17:36         ` Taylor Blau
2022-03-25 18:22           ` Junio C Hamano
2022-03-24 18:34   ` [PATCH v2 2/2] test-lib-functions: fix test_subcommand_inexact Derrick Stolee via GitGitGadget
2022-03-24 18:49     ` Taylor Blau
2022-03-24 20:48     ` Junio C Hamano
2022-03-25 14:03       ` Derrick Stolee
2022-03-25 17:25         ` Junio C Hamano
2022-03-25 19:02   ` [PATCH v3 0/2] " Derrick Stolee via GitGitGadget
2022-03-25 19:02     ` [PATCH v3 1/2] t7700: check post-condition in kept-pack test Derrick Stolee via GitGitGadget
2022-03-25 19:02     ` [PATCH v3 2/2] test-lib-functions: remove test_subcommand_inexact Derrick Stolee via GitGitGadget
2022-03-30  2:44     ` [PATCH v3 0/2] test-lib-functions: fix test_subcommand_inexact Taylor Blau

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=xmqqmthearlz.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=chakrabortyabhradeep79@gmail.com \
    --cc=derrickstolee@github.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=me@ttaylorr.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.