On Wed, Jun 16 2021, Patrick Steinhardt wrote: > When the TEST_OUTPUT_DIRECTORY is defined, then all test data will be > written in that directory instead of the default directory located in Is the timing of this patch a coincidence, or did you run into this related to the other patches related to this variable now, i.e. https://lore.kernel.org/git/20210609170520.67014-1-felipe.contreras@xxxxxxxxx/ and related. > Fix the issue by adding a `--results-dir` parameter to "aggregate.perl" > which identifies the directory where results are and by making the "run" > script awake of the TEST_OUTPUT_DIRECTORY variable. Makes sense. > [...] > my (@dirs, %dirnames, %dirabbrevs, %prefixes, @tests, > - $codespeed, $sortby, $subsection, $reponame); > + $codespeed, $sortby, $subsection, $reponame, $resultsdir); > > Getopt::Long::Configure qw/ require_order /; > > my $rc = GetOptions("codespeed" => \$codespeed, > "reponame=s" => \$reponame, > + "results-dir=s" => \$resultsdir, > "sort-by=s" => \$sortby, > "subsection=s" => \$subsection); > usage() unless $rc; > @@ -137,7 +139,9 @@ sub sane_backticks { > @tests = glob "p????-*.sh"; > } > > -my $resultsdir = "test-results"; > +if (not $resultsdir) { > + $resultsdir = "test-results"; > +} Works, but FWIW in git.git's perl scripts it's usual to do: my $resultsdir = "test-results"; GetOptions(...); Which serves the same purpose. You can also inline this in the GetOptions call: GetOptions(..., "results-dir=s" => \(my $resultsdir = "test-results"), ); But maybe that's too obscurely Perl-ish, i.e. declaring a variable for our scope inline in another function's argument list, and we should just use the first idiom we use in most other places. > if test -z "$GIT_PERF_AGGREGATING_LATER"; then > - ( cd "$TEST_DIRECTORY"/perf && ./aggregate.perl $(basename "$0") ) > + ( cd "$TEST_DIRECTORY"/perf && ./aggregate.perl --results-dir="$TEST_RESULTS_DIR" $(basename "$0") ) > fi > } Makes sense to split up the overly long line at this point probably... >[...] > -mkdir -p test-results > -get_subsections "perf" >test-results/run_subsections.names > +if test -n "$TEST_OUTPUT_DIRECTORY" > +then > + TEST_RESULTS_DIR="$TEST_OUTPUT_DIRECTORY/test-results" > +else > + TEST_RESULTS_DIR=test-results > +fi Indending with spaces?
