All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gummerer <t.gummerer@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Jeff King <peff@peff.net>
Subject: Re: [PATCH] perf-lib: remove old result files before running tests
Date: Wed, 20 Nov 2019 08:00:03 +0000	[thread overview]
Message-ID: <20191120080003.GA38901@cat> (raw)
In-Reply-To: <xmqq5zjfchix.fsf@gitster-ct.c.googlers.com>

On 11/20, Junio C Hamano wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
> 
> > @@ -178,10 +178,11 @@ test_wrapper_ () {
> >  	export test_prereq
> >  	if ! test_skip "$@"
> >  	then
> 
> > -		base=$(basename "$0" .sh)
> 
> So we used to use $base to hold the number and the filename here
> 
> > -		echo "$test_count" >>"$perf_results_dir"/$base.subtests
> > -		echo "$1" >"$perf_results_dir"/$base.$test_count.descr
> >  		base="$perf_results_dir"/"$PERF_RESULTS_PREFIX$(basename "$0" .sh)"."$test_count"
> 
> and then redefined it to be the results-prefix specific one.
> 
> 
> > +		rm -f "$base".*
> 
> you now remove those results-prefix specific one for the $test_count
> (I guess that is as specific you can go) before writing the count
> and the description.  
> 
> So this "rm -f" is a no-op when perf-results-prefix is not empty?

No, not quite.  It is a no-op the first time a particular test runs
for a specific prefix.  The prefix is usually set by the 't/perf/run'
script, and indicates the revision that is tested.

So if we were running for example

    ./run deadbeef... p0001-rev-list.sh

we'd have a file 'test-results/build_deadbeef....p0001-rev-list.1.times' 
in the t/perf directory.

Now we add a 'test_size' test before the first 'test_perf' test, and
run the tests again.  After this run we'd still have that original
file from the test results from the previous run, as well as a
'test-results/build_deadbeef....p0001-rev-list.1.size'.

This duplicate file at the matching "$base" is what we want to avoid.
The "rm -f" above would remove
'test-results/build_deadbeef....p0001-rev-list.1.times' so we now only
have the '.size' file left, and 'aggregate.perl' gives us the right
result.

> > +		no_prefix_base="$perf_results_dir"/$(basename "$0" .sh)
> > +		echo "$test_count" >>$no_prefix_base.subtests
> > +		echo "$1" >$no_prefix_base.$test_count.descr
> >  		"$test_wrapper_func_" "$@"
> >  	fi

  reply	other threads:[~2019-11-20  8:00 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-19 18:50 [PATCH] perf-lib: remove old result files before running tests Thomas Gummerer
2019-11-20  4:12 ` Junio C Hamano
2019-11-20  8:00   ` Thomas Gummerer [this message]
2019-11-21 10:20 ` Jeff King
2019-11-22  8:11   ` Thomas Gummerer
2019-11-25 14:09     ` Jeff King
2019-11-25 17:04       ` Thomas Gummerer

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=20191120080003.GA38901@cat \
    --to=t.gummerer@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.
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.