{"thread":{"id":"58020","subject":"[PATCH] perf-lib: fix missing test titles in output","startedAt":"2022-06-16T07:09:37Z","lastAt":"2022-06-16T20:06:32Z","messageCount":3,"participants":["Jeff King","Derrick Stolee","Neeraj Singh (WINDOWS-SFS)"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"457361","messageId":"YqrXLO5oMYeOr2PB@coredump.intra.peff.net","threadId":"58020","inReplyTo":null,"subject":"[PATCH] perf-lib: fix missing test titles in output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-06-16T07:09:32Z","receivedAt":"2022-06-16T07:09:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Commit 5dccd9155f (t/perf: add iteration setup mechanism to perf-lib,\n2022-04-04) modified the parameter parsing of test_wrapper() such that\nthe test title was no longer in $1, and is instead in $test_title_.\n\nWe correctly pass the new variable to the code which outputs the title\nto the log, but missed the spot in test_wrapper() where the title is\nwritten to the \".descr\" file which is used to produce the final output\ntable. As a result, all of the titles are missing from that table (or\nworse, using whatever was left in $1):\n\n  $ ./p0000-perf-lib-sanity.sh\n  [...]\n  Test           this tree\n  ------------------------------\n  0000.1:        0.01(0.01+0.00)\n  0000.2:        0.01(0.00+0.01)\n  0000.4:        0.00(0.00+0.00)\n  0000.5: true   0.00(0.00+0.00)\n  0000.7:        0.00(0.00+0.00)\n  0000.8:        0.00(0.00+0.00)\n\nAfter this patch, we get the pre-5dccd9155f output:\n\n  Test                                                       this tree\n  --------------------------------------------------------------------------\n  0000.1: test_perf_default_repo works                       0.00(0.00+0.00)\n  0000.2: test_checkout_worktree works                       0.01(0.00+0.01)\n  0000.4: export a weird var                                 0.00(0.00+0.00)\n  0000.5: éḿíẗ ńöń-ÁŚĆÍÍ ćḧáŕáćẗéŕś                          0.00(0.00+0.00)\n  0000.7: important variables available in subshells         0.00(0.00+0.00)\n  0000.8: test-lib-functions correctly loaded in subshells   0.00(0.00+0.00)\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/perf/perf-lib.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh\nindex ab3687c28d..27c2801792 100644\n--- a/t/perf/perf-lib.sh\n+++ b/t/perf/perf-lib.sh\n@@ -219,7 +219,7 @@ test_wrapper_ () {\n \tthen\n \t\tbase=$(basename \"$0\" .sh)\n \t\techo \"$test_count\" >>\"$perf_results_dir\"/$base.subtests\n-\t\techo \"$1\" >\"$perf_results_dir\"/$base.$test_count.descr\n+\t\techo \"$test_title_\" >\"$perf_results_dir\"/$base.$test_count.descr\n \t\tbase=\"$perf_results_dir\"/\"$PERF_RESULTS_PREFIX$(basename \"$0\" .sh)\".\"$test_count\"\n \t\t\"$test_wrapper_func_\" \"$test_title_\" \"$@\"\n \tfi\n-- \n2.37.0.rc0.352.g10876ef154\n"},{"id":"457367","messageId":"6671fd2c-ffcb-6278-494f-2d3cfe4c36d1@github.com","threadId":"58020","inReplyTo":"YqrXLO5oMYeOr2PB@coredump.intra.peff.net","subject":"Re: [PATCH] perf-lib: fix missing test titles in output","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-06-16T13:03:37Z","receivedAt":"2022-06-16T13:03:45Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 6/16/2022 3:09 AM, Jeff King wrote:\n> Commit 5dccd9155f (t/perf: add iteration setup mechanism to perf-lib,\n> 2022-04-04) modified the parameter parsing of test_wrapper() such that\n> the test title was no longer in $1, and is instead in $test_title_.\n> \n> We correctly pass the new variable to the code which outputs the title\n> to the log, but missed the spot in test_wrapper() where the title is\n> written to the \".descr\" file which is used to produce the final output\n> table. As a result, all of the titles are missing from that table (or\n> worse, using whatever was left in $1):\n> \n>   $ ./p0000-perf-lib-sanity.sh\n>   [...]\n>   Test           this tree\n>   ------------------------------\n>   0000.1:        0.01(0.01+0.00)\n>   0000.2:        0.01(0.00+0.01)\n>   0000.4:        0.00(0.00+0.00)\n>   0000.5: true   0.00(0.00+0.00)\n>   0000.7:        0.00(0.00+0.00)\n>   0000.8:        0.00(0.00+0.00)\n\nYikes!\n \n> After this patch, we get the pre-5dccd9155f output:\n> \n>   Test                                                       this tree\n>   --------------------------------------------------------------------------\n>   0000.1: test_perf_default_repo works                       0.00(0.00+0.00)\n>   0000.2: test_checkout_worktree works                       0.01(0.00+0.01)\n>   0000.4: export a weird var                                 0.00(0.00+0.00)\n>   0000.5: éḿíẗ ńöń-ÁŚĆÍÍ ćḧáŕáćẗéŕś                          0.00(0.00+0.00)\n>   0000.7: important variables available in subshells         0.00(0.00+0.00)\n>   0000.8: test-lib-functions correctly loaded in subshells   0.00(0.00+0.00)\n\nGood catch. LGTM.\n\n-Stolee\n"},{"id":"457403","messageId":"DM5PR21MB154239CE797E7D50D0435213CCAC9@DM5PR21MB1542.namprd21.prod.outlook.com","threadId":"58020","inReplyTo":"6671fd2c-ffcb-6278-494f-2d3cfe4c36d1@github.com","subject":"RE: [PATCH] perf-lib: fix missing test titles in output","fromName":"Neeraj Singh (WINDOWS-SFS)","fromEmail":"neerajsi@microsoft.com","sentAt":"2022-06-16T19:51:44Z","receivedAt":"2022-06-16T20:06:32Z","isPatch":true,"sender":{"key":"neerajsi@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/78932899?v=4"},"body":"Apologies for the breakage and thanks for the fix.\nIt looks good to me!\n\n-Neeraj\n"}]}