Re: [PATCH v5 02/10] config tests: add "NULL" tests for *_get_value_multi()

From: Glen Choo <chooglen@google.com>
Date: Feb 9, 2023, 04:00 UTC
Message-ID: <kl6llel7trlw.fsf@chooglen-macbookpro.roam.corp.google.com>
In-Reply-To: <patch-v5-02.10-91a44456327-20230207T154000Z-avarab@gmail.com>

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

Show 22 quoted lines

> +		# Value-less in the middle of a list
> +		cat >"$config" <<-\EOF &&
> +		[a]key=x
> +		[a]key
> +		[a]key=y
> +		EOF
> +		case "$op" in
> +		*_multi)
> +			cat >expect <<-\EOF
> +			x
> +			(NULL)
> +			y
> +			EOF
> +			;;
> +		*)
> +			cat >expect <<-\EOF
> +			y
> +			EOF
> +			;;
> +		esac &&
> +		test-tool config "$op" a.key $file >actual &&
> +		test_cmp expect actual &&

This extra test case makes me feel a bit better about making this DRY, though the extra "case" statement detracts from the readability a bit.

Maybe if we split the _multi and non-multi cases?

	test_expect_success "multi" '
    # tmp config things

		# Value-less in the middle of a list
		cat >"$config" <<-\EOF &&
		[a]key=x
		[a]key
		[a]key=y
		EOF
    cat >expect <<-\EOF
    x
    (NULL)
    y
    EOF
		test-tool config "$op" a.key $file >actual &&
		test_cmp expect actual &&

		# Value-less at the end of a least (probable typo)
		cat >"$config" <<-\EOF &&
		[a]key=x
		[a]key=y
		[a]key
		EOF
    cat >expect <<-\EOF
    x
    y
    (NULL)
    EOF
		test-tool config "$op" a.key $file >actual &&
		test_cmp expect actual
	'

	test_expect_success "single" '
    # tmp config things

		# Value-less in the middle of a list
		cat >"$config" <<-\EOF &&
		[a]key=x
		[a]key
		[a]key=y
		EOF
    cat >expect <<-\EOF
    y
    EOF
		test-tool config "$op" a.key $file >actual &&
		test_cmp expect actual &&

		# Value-less at the end of a least (probable typo)
		cat >"$config" <<-\EOF &&
		[a]key=x
		[a]key=y
		[a]key
		EOF
    cat >expect <<-\EOF
    (NULL)
    EOF
		test-tool config "$op" a.key $file >actual &&
		test_cmp expect actual
	'

Idk. It does read a bit clearer to me, but I don't feel strongly about it.

> +		# Value-less at the end of a least

s/least/list