[prev in list] [next in list] [prev in thread] [next in thread] 

List:       git
Subject:    Re: [PATCH v2 1/8] fetch: split out tests for output format
From:       SZEDER =?utf-8?B?R8OhYm9y?= <szeder.dev () gmail ! com>
Date:       2023-04-29 17:34:48
Message-ID: 20230429173448.GD3271 () szeder ! dev
[Download RAW message or body]

On Thu, Apr 27, 2023 at 01:13:08PM +0200, Patrick Steinhardt wrote:
> We're about to introduce a new porcelain mode for the output of
> git-fetch(1). As part of that we'll be introducing a set of new tests
> that only relate to the output of this command.
> 
> Split out tests that exercise the output format of git-fetch(1) so that
> it becomes easier to verify this functionality as a standalone unit. As
> the tests assume that the default branch is called "main" we set up the
> corresponding GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME environment variable
> accordingly.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  t/t5510-fetch.sh        | 53 ----------------------------------
>  t/t5574-fetch-output.sh | 63 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 63 insertions(+), 53 deletions(-)
>  create mode 100755 t/t5574-fetch-output.sh


> diff --git a/t/t5574-fetch-output.sh b/t/t5574-fetch-output.sh
> new file mode 100755
> index 0000000000..f91b654d38
> --- /dev/null
> +++ b/t/t5574-fetch-output.sh
> @@ -0,0 +1,63 @@
> +#!/bin/sh
> +
> +test_description='git fetch output format'
> +
> +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
> +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'fetch aligned output' '
> +	git clone . full-output &&
> +	test_commit looooooooooooong-tag &&
> +	(
> +		cd full-output &&
> +		git -c fetch.output=full fetch origin >actual 2>&1 &&

Why is that 2>&1 redirection used here?
If the output the test case is interested in goes to the command's
standard output, then it's unnecessary.  However, if it goes to
standard error, then why is standard output redirected as well?

I understand that this patch just moves existing test cases around
as-is, so this is not something you introduced, but I point it out
here, because later patches of this series add several new test cases
following this anti-pattern.

Since this series creates a new test script, perhaps this might be the
right time to clean this up.

> +		grep -e "->" actual | cut -c 22- >../actual
> +	) &&
> +	cat >expect <<-\EOF &&
> +	main                 -> origin/main
> +	looooooooooooong-tag -> looooooooooooong-tag
> +	EOF
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'fetch compact output' '
> +	git clone . compact &&
> +	test_commit extraaa &&
> +	(
> +		cd compact &&
> +		git -c fetch.output=compact fetch origin >actual 2>&1 &&
> +		grep -e "->" actual | cut -c 22- >../actual
> +	) &&
> +	cat >expect <<-\EOF &&
> +	main       -> origin/*
> +	extraaa    -> *
> +	EOF
> +	test_cmp expect actual
> +'
> +
> +test_expect_success '--no-show-forced-updates' '
> +	mkdir forced-updates &&
> +	(
> +		cd forced-updates &&
> +		git init &&
> +		test_commit 1 &&
> +		test_commit 2
> +	) &&
> +	git clone forced-updates forced-update-clone &&
> +	git clone forced-updates no-forced-update-clone &&
> +	git -C forced-updates reset --hard HEAD~1 &&
> +	(
> +		cd forced-update-clone &&
> +		git fetch --show-forced-updates origin 2>output &&

Oh, look, there are some good examples to follow here as well :)

> +		test_i18ngrep "(forced update)" output
> +	) &&
> +	(
> +		cd no-forced-update-clone &&
> +		git fetch --no-show-forced-updates origin 2>output &&
> +		test_i18ngrep ! "(forced update)" output
> +	)
> +'
> +
> +test_done
> -- 
> 2.40.1
> 


[prev in list] [next in list] [prev in thread] [next in thread]