Re: [PATCH v3 02/12] test-lib: bring $remove_trash out of retirement
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 20, 2021, 22:28 UTC
- Message-ID
- <xmqqsg3k381c.fsf@gitster.g>
- In-Reply-To
- <patch-02.12-39759d00ad-20210420T121833Z-avarab@gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> We carry around the "$remove_trash" variable because if the directory > is unexpectedly gone at test_done time we'll hit the error about it > being unexpectedly gone added in df4c0d1a792 (test-lib: abort when > can't remove trash directory, 2017-04-20).
Makes sense, but then remove_trash should be initialized to a non-empty string at the beginning, to avoid it leaking from end-user environment, just like the way store_arg_to and opt_required_arg are protected?
Show 63 quoted lines
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 3dec266221..7522faf39f 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -1169,7 +1169,7 @@ test_done () {
> esac
> fi
>
> - if test -z "$debug"
> + if test -z "$debug" && test -n "$remove_trash"
> then
> test -d "$TRASH_DIRECTORY" ||
> error "Tests passed but trash directory already removed before test cleanup; aborting"
> @@ -1334,6 +1334,21 @@ then
> exit 1
> fi
>
> +# Are we running this test at all?
> +this_test=${0##*/}
> +this_test=${this_test%%-*}
> +if match_pattern_list "$this_test" $GIT_SKIP_TESTS
> +then
> + say_color info >&3 "skipping test $this_test altogether"
> + skip_all="skip all tests in $this_test"
> + test_done
> +fi
> +
> +# Last-minute variable setup
> +HOME="$TRASH_DIRECTORY"
> +GNUPGHOME="$HOME/gnupg-home-not-used"
> +export HOME GNUPGHOME
> +
> # Test repository
> rm -fr "$TRASH_DIRECTORY" || {
> GIT_EXIT_OK=t
> @@ -1341,10 +1356,7 @@ rm -fr "$TRASH_DIRECTORY" || {
> exit 1
> }
>
> -HOME="$TRASH_DIRECTORY"
> -GNUPGHOME="$HOME/gnupg-home-not-used"
> -export HOME GNUPGHOME
> -
> +remove_trash=t
> if test -z "$TEST_NO_CREATE_REPO"
> then
> test_create_repo "$TRASH_DIRECTORY"
> @@ -1356,15 +1368,6 @@ fi
> # in subprocesses like git equals our $PWD (for pathname comparisons).
> cd -P "$TRASH_DIRECTORY" || exit 1
>
> -this_test=${0##*/}
> -this_test=${this_test%%-*}
> -if match_pattern_list "$this_test" $GIT_SKIP_TESTS
> -then
> - say_color info >&3 "skipping test $this_test altogether"
> - skip_all="skip all tests in $this_test"
> - test_done
> -fi
> -
> if test -n "$write_junit_xml"
> then
> junit_xml_dir="$TEST_OUTPUT_DIRECTORY/out"