threads / patch / 58020

patchperf-lib: fix missing test titles in output

Subject: [PATCH] perf-lib: fix missing test titles in output

## tl;dr

3 messages between Jun 16, 2022 and Jun 16, 2022. Diffs are folded; open one to read it.

replies: 2people: 3as markdown or json

Jeff King· Jun 16, 2022, 07:09 UTC · lore

Commit 5dccd9155f (t/perf: add iteration setup mechanism to perf-lib, 2022-04-04) modified the parameter parsing of test_wrapper() such that the test title was no longer in $1, and is instead in $test_title_.

We correctly pass the new variable to the code which outputs the title to the log, but missed the spot in test_wrapper() where the title is written to the ".descr" file which is used to produce the final output table. As a result, all of the titles are missing from that table (or worse, using whatever was left in $1):

  $ ./p0000-perf-lib-sanity.sh
  [...]
  Test           this tree
  ------------------------------
  0000.1:        0.01(0.01+0.00)
  0000.2:        0.01(0.00+0.01)
  0000.4:        0.00(0.00+0.00)
  0000.5: true   0.00(0.00+0.00)
  0000.7:        0.00(0.00+0.00)
  0000.8:        0.00(0.00+0.00)
After this patch, we get the pre-5dccd9155f output:
  Test                                                       this tree
  --------------------------------------------------------------------------
  0000.1: test_perf_default_repo works                       0.00(0.00+0.00)
  0000.2: test_checkout_worktree works                       0.01(0.00+0.01)
  0000.4: export a weird var                                 0.00(0.00+0.00)
  0000.5: éḿíẗ ńöń-ÁŚĆÍÍ ćḧáŕáćẗéŕś                          0.00(0.00+0.00)
  0000.7: important variables available in subshells         0.00(0.00+0.00)
  0000.8: test-lib-functions correctly loaded in subshells   0.00(0.00+0.00)
Signed-off-by: Jeff King <peff@peff.net>
---
 t/perf/perf-lib.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to t/perf/perf-lib.sh +1 −1
diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
index ab3687c28d..27c2801792 100644
--- a/t/perf/perf-lib.sh
+++ b/t/perf/perf-lib.sh
@@ -219,7 +219,7 @@ test_wrapper_ () {
 	then
 		base=$(basename "$0" .sh)
 		echo "$test_count" >>"$perf_results_dir"/$base.subtests
-		echo "$1" >"$perf_results_dir"/$base.$test_count.descr
+		echo "$test_title_" >"$perf_results_dir"/$base.$test_count.descr
 		base="$perf_results_dir"/"$PERF_RESULTS_PREFIX$(basename "$0" .sh)"."$test_count"
 		"$test_wrapper_func_" "$test_title_" "$@"
 	fi
-- 
2.37.0.rc0.352.g10876ef154
Derrick Stolee· Jun 16, 2022, 13:03 UTC · re: Jeff King · lore

Re: [PATCH] perf-lib: fix missing test titles in output

On 6/16/2022 3:09 AM, Jeff King wrote:
Show 20 quoted lines
> Commit 5dccd9155f (t/perf: add iteration setup mechanism to perf-lib,
> 2022-04-04) modified the parameter parsing of test_wrapper() such that
> the test title was no longer in $1, and is instead in $test_title_.
> 
> We correctly pass the new variable to the code which outputs the title
> to the log, but missed the spot in test_wrapper() where the title is
> written to the ".descr" file which is used to produce the final output
> table. As a result, all of the titles are missing from that table (or
> worse, using whatever was left in $1):
> 
>   $ ./p0000-perf-lib-sanity.sh
>   [...]
>   Test           this tree
>   ------------------------------
>   0000.1:        0.01(0.01+0.00)
>   0000.2:        0.01(0.00+0.01)
>   0000.4:        0.00(0.00+0.00)
>   0000.5: true   0.00(0.00+0.00)
>   0000.7:        0.00(0.00+0.00)
>   0000.8:        0.00(0.00+0.00)
Yikes!
 
Show 10 quoted lines
> After this patch, we get the pre-5dccd9155f output:
> 
>   Test                                                       this tree
>   --------------------------------------------------------------------------
>   0000.1: test_perf_default_repo works                       0.00(0.00+0.00)
>   0000.2: test_checkout_worktree works                       0.01(0.00+0.01)
>   0000.4: export a weird var                                 0.00(0.00+0.00)
>   0000.5: éḿíẗ ńöń-ÁŚĆÍÍ ćḧáŕáćẗéŕś                          0.00(0.00+0.00)
>   0000.7: important variables available in subshells         0.00(0.00+0.00)
>   0000.8: test-lib-functions correctly loaded in subshells   0.00(0.00+0.00)
Good catch. LGTM.
-Stolee

← back to recent threads