Re: [PATCH 1/2] gc: add tests for --cruft and friends

[Junio C Hamano <gitster@pobox.com>]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

>> +test_expect_success 'gc --cruft generates a cruft pack' '
>> +	git init crufts &&
>> +	test_when_finished "rm -fr crufts" &&
>
> Let's "test_when_finished" first, then "git init", the point is to clean
> up the directory if we fail.

Good advice.

We say "rm -fr" not "rm -r" there because we do not want to see a
failure to remove if "git init" failed before it manages to create
the directory ;-)

>
>> +	(
>> +		cd crufts &&
>> +		test_commit base &&
>> +
>> +		test_commit --no-tag foo &&
>> +		test_commit --no-tag bar &&
>> +		git reset HEAD^^ &&
>> +
>> +		git gc --cruft &&
>> +
>> +		cruft=$(basename $(ls .git/objects/pack/pack-*.mtimes) .mtimes) &&
>> +		test_path_is_file .git/objects/pack/$cruft.pack
>> +	)
>> +'
>
> ...this...
>
>> +test_expect_success 'gc.cruftPacks=true generates a cruft pack' '
>> +	git init crufts &&
>> +	test_when_finished "rm -fr crufts" &&
>> +	(
>> +		cd crufts &&
>> +		test_commit base &&
>> +
>> +		test_commit --no-tag foo &&
>> +		test_commit --no-tag bar &&
>> +		git reset HEAD^^ &&
>> +
>> +		git -c gc.cruftPacks=true gc &&
>> +
>> +		cruft=$(basename $(ls .git/objects/pack/pack-*.mtimes) .mtimes) &&
>> +		test_path_is_file .git/objects/pack/$cruft.pack
>> +	)
>> +'
>> +
>
> ...whole thing seems to be copy/pasted aside from the git options.
>
> If so let's factor this into a trivial helper that invokes git "$@",
> then call it with "gc --cruft" and "-c ..."?

With shell, passing "here is a series of commands to be run in the
middle of a boilerplate sequence" is indeed easy to write, but it
gets harder to follow and quote correctly, which is why I'd rather
not see that pattern overused.

A pair of helper functions, one of which prepares a sample history
to be used, and the other checks if we created one (or more) cruft
packs, may achieve the same conciseness while remaining to be more
readable.  I.e.

    test_when_finished "rm -fr crufts" &&
    git init crufts &&
    (
	cd crufts &&
	prepare_history &&

	git -c gc.cruftPacks=true gc &&

	cruft_packs_exist
    )

perhaps?