From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: Jeff King <peff@peff.net>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: [PATCH] test-lib: make '--stress' more bisect-friendly
Date: Fri, 8 Feb 2019 19:23:19 +0100 [thread overview]
Message-ID: <20190208182319.GY10587@szeder.dev> (raw)
In-Reply-To: <20190208164732.GA23461@sigill.intra.peff.net>
On Fri, Feb 08, 2019 at 11:47:33AM -0500, Jeff King wrote:
> On Fri, Feb 08, 2019 at 12:50:45PM +0100, SZEDER Gábor wrote:
>
> > - Make it exit with failure if a failure is found.
> >
> > - Add the '--stress-limit=<N>' option to repeat the test script
> > at most N times in each of the parallel jobs, and exit with
> > success when the limit is reached.
> > [...]
> >
> > This is a case when an external stress script works better, as it can
> > easily check commits in the past... if someone has such a script,
> > that is.
>
> Heh, I literally just implemented this kind of max-count in my own
> "stress" script[1] to handle this recent t0025 testing. So certainly I
> think it is a good idea.
>
> Picking an <N> is tough. Too low and you get a false negative, too high
> and you can wait forever, especially if the script is long. But I don't
> think there's any real way to auto-scale it, except by seeing a few of
> the failing cases and watching how long they take.
So far I've chosen <N> like this: run the test script with --stress
3-5 times to trigger the failure, take the highest repetition count
that was necessary for the failure, multiply it by 4-6 to get a round
number, and that's a good ballpark for <N>. And once bisect came up
with the suspect commit, I double checked it by letting the test
script run with --stress on its parent commit for at least 5-10x <N>
repetitions.
Anyway, I doubt that auto-scaling <N> is worth the effort.
> > t/README | 5 +++++
> > t/test-lib.sh | 18 ++++++++++++++++--
> > 2 files changed, 21 insertions(+), 2 deletions(-)
>
> Patch looks good. A few observations:
>
> > @@ -237,8 +248,10 @@ then
> > exit 1
> > ' TERM INT
> >
> > - cnt=0
> > - while ! test -e "$stressfail"
> > + cnt=1
> > + while ! test -e "$stressfail" &&
> > + { test -z "$stress_limit" ||
> > + test $cnt -le $stress_limit ; }
> > do
> > $TEST_SHELL_PATH "$0" "$@" >"$TEST_RESULTS_BASE.stress-$job_nr.out" 2>&1 &
> > test_pid=$!
>
> You switch to 1-indexing the counts here. I think that makes sense,
> since otherwise --stress-limit=300 would end at "1.299", etc.
Yeah, that's exactly why I did it.
>
> > @@ -261,6 +274,7 @@ then
> >
> > if test -f "$stressfail"
> > then
> > + stress_exit=1
> > echo "Log(s) of failed test run(s):"
> > for failed_job_nr in $(sort -n "$stressfail")
> > do
>
> I think I'd argue that this missing stress_exit is a bug in the original
> script,
Well, yes, indeed.
Though being able to trigger an elusive test failure is a success in
my book ;)
> and somewhat orthogonal to the limit counter. But I don't think
> it's worth the trouble to split it out (and certainly the theme of "now
> you can run this via bisect" unifies the two changes).
>
> -Peff
next prev parent reply other threads:[~2019-02-08 18:23 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-08 11:50 [PATCH] test-lib: make '--stress' more bisect-friendly SZEDER Gábor
2019-02-08 16:47 ` Jeff King
2019-02-08 16:49 ` Jeff King
2019-02-08 18:33 ` SZEDER Gábor
2019-02-08 19:12 ` Jeff King
2019-02-08 18:23 ` SZEDER Gábor [this message]
2019-02-08 19:11 ` Jeff King
2019-02-11 19:58 ` [PATCH] test-lib: fix non-portable pattern bracket expressions SZEDER Gábor
2019-02-11 22:29 ` Junio C Hamano
2019-02-11 23:46 ` Jeff King
2019-02-12 0:30 ` SZEDER Gábor
2019-02-12 0:34 ` Jeff King
2019-02-12 10:47 ` Carlo Arenas
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190208182319.GY10587@szeder.dev \
--to=szeder.dev@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
/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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).