From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 15 Aug 2025 18:40:06 GMT
Subject: Re: [GSoC PATCH v10 2/5] repo: add the field references.format
Message-ID: <xmqqfrdsqvi1.fsf@gitster.g>
In-Reply-To: <20250815135521.44044-3-lucasseikioshiro@gmail.com>

Lucas Seiki Oshiro <lucasseikioshiro@gmail.com> writes:

> diff --git a/t/t1900-repo.sh b/t/t1900-repo.sh
> new file mode 100755
> index 0000000000..dca4023a00
> --- /dev/null
> +++ b/t/t1900-repo.sh
> @@ -0,0 +1,50 @@
> +#!/bin/sh
> +
> +test_description='test git repo-info'
> +
> +. ./test-lib.sh
> +
> +# Test whether a key-value pair is correctly returned
> +#
> +# Usage: test_repo_info <label> <init command> <repo_name> <key> <expected value>
> +#
> +# Arguments:
> +#   label: the label of the test
> +#   init_command: a command which creates a repository
> +#   repo_name: the name of the repository that will be created in init_command
> +#   key: the key of the field that is being tested
> +#   expected_value: the value that the field should contain
> +test_repo_info () {
> +	label=$1
> +	init_command=$2
> +	repo_name=$3
> +	key=$4
> +	expected_value=$5
> +
> +	test_expect_success "$label" '
> +		eval "$init_command $repo_name" &&
> +		echo "$key=$expected_value" >expected &&
> +		git -C $repo_name repo info "$key" >actual &&

Most tests compare "expect" and "actual" by convention, so this is a
bit unusual, but I'll let it go.

I know it is tempting to write a helper function like this that
looks overly generic, but I've seen that we end up either tests that
are too hard to modify (because such helpers are not flexible enough)
or we end up adding more helpers to cover different cases, and
either case the presense of the helper does not help the resulting
tests easier to read or modify.

If this were "test repo-info for ref.format only" that takes a
single parameter (format), that might have been more palatable, but
I'll withhold my judgement until we have tests to handle more keys
in later steps.

> +		test_cmp expected actual
> +	'
> +}
> +
> +test_repo_info 'ref format files is retrieved correctly' \
> +	'git init --ref-format=files' 'format-files' 'references.format' 'files'
> +
> +test_repo_info 'ref format reftable is retrieved correctly' \
> +	'git init --ref-format=reftable' 'format-reftable' 'references.format' 'reftable'
> +
> +test_expect_success 'git-repo-info fails if an invalid key is requested' '
> +	echo "error: key ${SQ}foo${SQ} not found" >expected_err &&
> +	test_must_fail git repo info foo 2>actual_err &&
> +	test_cmp expected_err actual_err
> +'

Also, unless we capture both standard output (in actual) and
standard error (in actual_err) from the command being tested, and
instead are interested in testing only what comes out of the error
stream, it is more common to use the usual "expect vs actual".

> +
> +test_expect_success 'git-repo-info outputs data even if there is an invalid field' '
> +	echo "references.format=$(test_detect_ref_format)" >expected &&
> +	test_must_fail git repo info foo references.format bar >actual &&
> +	test_cmp expected actual
> +'
> +
> +test_done