git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Eric Sunshine <sunshine@sunshineco.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Git List <git@vger.kernel.org>
Subject: Re: [PATCH] tests: clean after SANITY tests
Date: Fri, 15 Jun 2018 15:57:13 -0400	[thread overview]
Message-ID: <CAPig+cQQEKjM43OqX9bT2=_yFjkiUMqjbZag5TWpsh6d_Hwn_g@mail.gmail.com> (raw)
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:
> 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.

      reply	other threads:[~2018-06-15 19:57 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-15 18:30 [PATCH] tests: clean after SANITY tests Junio C Hamano
2018-06-15 19:57 ` Eric Sunshine [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAPig+cQQEKjM43OqX9bT2=_yFjkiUMqjbZag5TWpsh6d_Hwn_g@mail.gmail.com' \
    --to=sunshine@sunshineco.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).