All of lore.kernel.org
 help / color / mirror / Atom feed
From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: Jeff King <peff@peff.net>
Cc: Stefan Beller <sbeller@google.com>, Johannes Sixt <j6t@kdbg.org>,
	Junio C Hamano <gitster@pobox.com>,
	David Turner <dturner@twosigma.com>,
	Git mailing list <git@vger.kernel.org>
Subject: Re: [PATCH] t6500: don't run detached auto gc at the end of the test script
Date: Thu, 13 Apr 2017 21:03:26 +0200	[thread overview]
Message-ID: <CAM0VKjmK0ib+sjf0sMPeiK7DrwHq1dJ58ZhDdp6HZsuQdb-eRw@mail.gmail.com> (raw)
In-Reply-To: <20170413175701.5ogpe7qbflbkgljm@sigill.intra.peff.net>

On Thu, Apr 13, 2017 at 7:57 PM, Jeff King <peff@peff.net> wrote:
> On Thu, Apr 13, 2017 at 10:55:08AM -0700, Stefan Beller wrote:
>
>> On Thu, Apr 13, 2017 at 9:37 AM, Jeff King <peff@peff.net> wrote:
>> > Ah, OK, that makes more sense. I can detect it reliably by just checking
>> >
>> >   ! test -d $root/trash*
>> >
>> > in my stress-test after the test successfully completes.
>>
>> Would we want to report such an error from the test suite itself?
>> (My line of thinking is that this is a similar issue to broken && chains,
>> which we do report).

A broken &&-chain can harm the test's correctness by hiding errors.
The failing 'rm -rf $trash' during housekeeping, after all the tests
in the test script has been run and their results reported... not
really, I would think, though arguably it's a sign that something is
fishy.

> Yeah, I had a similar thought. I can't think of any reason why it would
> trigger a false positive, as long as we account for test-failure and the
> --debug flag properly. I don't think we can just drop the "-f" from the
> final "rm", because it may be overriding funny permissions left by
> tests.

FWIW, I used the following rather simple change during stress-testing
these patches (barring white-space damage):

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 13b569682..d7fa15a69 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -763,7 +763,7 @@ test_done () {

                test -d "$remove_trash" &&
                cd "$(dirname "$remove_trash")" &&
-               rm -rf "$(basename "$remove_trash")"
+               rm -rf "$(basename "$remove_trash")" || exit 1

                test_at_end_hook_

  reply	other threads:[~2017-04-13 19:03 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-10 12:59 [PATCH] t6500: don't run detached auto gc at the end of the test script SZEDER Gábor
2017-04-10 13:58 ` Jeff King
2017-04-10 16:31   ` SZEDER Gábor
2017-04-10 16:35     ` Jeff King
2017-04-10 16:56       ` SZEDER Gábor
2017-04-10 17:01         ` Jeff King
2017-04-11 21:32           ` Johannes Sixt
2017-04-12  0:27             ` SZEDER Gábor
2017-04-12  0:50               ` Jeff King
2017-04-12 22:03                 ` SZEDER Gábor
2017-04-12 22:07                   ` [PATCHv2] " SZEDER Gábor
2017-04-13 10:31                     ` [PATCHv2.1] t6500: wait for " SZEDER Gábor
2017-04-13 16:06                       ` David Turner
2017-04-13 16:44                       ` Jeff King
2017-04-13 18:08                         ` SZEDER Gábor
2017-04-13 18:12                           ` Jeff King
2017-04-13 16:37                   ` [PATCH] t6500: don't run " Jeff King
2017-04-13 17:55                     ` Stefan Beller
2017-04-13 17:57                       ` Jeff King
2017-04-13 19:03                         ` SZEDER Gábor [this message]
2017-04-13 19:12                           ` Jeff King
2017-04-13 19:35                             ` SZEDER Gábor
2017-04-14 20:08                               ` Jeff King
2017-04-20 16:42                                 ` SZEDER Gábor
2017-04-20 16:45                                   ` Jeff King
2017-04-20 16:52                                   ` [PATCH] test-lib: abort when can't remove trash directory SZEDER Gábor
2017-04-20 19:06                                     ` Jeff King
2017-04-21  0:48                                     ` Junio C Hamano
2017-04-21 20:06                                       ` SZEDER Gábor
2017-04-21 20:15                                     ` Jeff King
2017-04-24  0:14                                       ` Junio C Hamano
2017-04-24  1:43                                         ` Jeff King
2017-04-24  2:58                                           ` Junio C Hamano
2017-04-24  4:02                                             ` Junio C Hamano
2017-04-24  7:52                                               ` Jeff King
2017-04-24  9:39                                                 ` Torsten Bögershausen
2017-04-24  9:46                                                   ` Jeff King
2017-04-25  2:31                                                   ` Junio C Hamano
2017-04-25  6:05                                                 ` Junio C Hamano
2017-04-25  6:07                                                   ` Jeff King
2017-04-25  6:31                                                     ` Junio C Hamano

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=CAM0VKjmK0ib+sjf0sMPeiK7DrwHq1dJ58ZhDdp6HZsuQdb-eRw@mail.gmail.com \
    --to=szeder.dev@gmail.com \
    --cc=dturner@twosigma.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    --cc=peff@peff.net \
    --cc=sbeller@google.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.