Re: [PATCH] tests: clean after SANITY tests

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jun 15, 2018, 19:57 UTC
Message-ID: <CAPig+cQQEKjM43OqX9bT2=_yFjkiUMqjbZag5TWpsh6d_Hwn_g@mail.gmail.com>
In-Reply-To: <xmqqa7rv297r.fsf@gitster-ct.c.googlers.com>

On Fri, Jun 15, 2018 at 2:30 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 15 quoted lines

> Some of our tests try to make sure Git behaves sensibly in a
> read-only directory, by dropping 'w' permission bit before doing a
> test and then restoring it after it is done.  The latter is needed
> for the test framework to clean after itself without leaving a
> leftover directory that cannot be removed.
> [...]
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> diff --git a/t/t0001-init.sh b/t/t0001-init.sh
> @@ -287,6 +287,7 @@ test_expect_success 'init notices EEXIST (2)' '
>  test_expect_success POSIXPERM,SANITY 'init notices EPERM' '
> +       test_when_finished "chmod +w newdir" &&
>         rm -fr newdir &&
>         mkdir newdir &&
>         chmod -w newdir &&

When reading this, I was wondering if there was a "rm -fr newdir" at the end of the test which could be removed now that it uses test_when_finished(). Checking beyond the shown context, I see that it doesn't bother with removal since the next test removes the directory as a preparatory step. Even before the addition of test_when_finished() to restore write permission, the subsequent test's removal of the directory worked because this is test is only run when POSIXPERM is met, and POSIX allows for such an operation. Okay, looks good.