| Message ID | 8326a9b5320e1e774caef568fcce2bfd2ec13cb1.1611161639.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | More index cleanups | expand |
On Wed, Jan 20, 2021 at 8:54 AM Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> wrote: > > From: Derrick Stolee <dstolee@microsoft.com> > > Most test cases can verify Git's behavior using input/output > expectations or changes to the .git directory. However, sometimes we > want to check that Git did or did not run a certain section of code. > This is particularly important for performance-only features that we > want to ensure have been enabled in certain cases. > > Add a new 'test_region' function that checks if a trace2 region was > entered and left in a given trace2 event log. Ooh, others do this too? Sounds like a helpful function to add, but just checking for entered and left means that... > > There is one existing test (t0500-progress-display.sh) that performs > this check already, so use the helper function instead. More uses will > be added in a later change. > > t6423-merge-rename-directories.sh also greps for region_enter lines, but > it verifies the number of such lines, which is not the same as an > existence check. ...yeah, won't cover the case that I added. That's fine, since it appears to be a one-off for now and we don't know of any other cases, current or planned, that want to do something like that yet. > > Signed-off-by: Derrick Stolee <dstolee@microsoft.com> > --- > t/t0500-progress-display.sh | 3 +-- > t/test-lib-functions.sh | 40 +++++++++++++++++++++++++++++++++++++ > 2 files changed, 41 insertions(+), 2 deletions(-) > > diff --git a/t/t0500-progress-display.sh b/t/t0500-progress-display.sh > index 1ed1df351cb..c461b89dfaf 100755 > --- a/t/t0500-progress-display.sh > +++ b/t/t0500-progress-display.sh > @@ -303,8 +303,7 @@ test_expect_success 'progress generates traces' ' > "Working hard" <in 2>stderr && > > # t0212/parse_events.perl intentionally omits regions and data. > - grep -e "region_enter" -e "\"category\":\"progress\"" trace.event && > - grep -e "region_leave" -e "\"category\":\"progress\"" trace.event && > + test_region category progress trace.event && Sidenote: Hmm...about 40% of my region labels in merge-ort.c and 90% in diffcore-rename.c have spaces in them. This function could still be used, but I'm curious if I should change the labels (but then again, they are testing logical regions rather than individual functions, and the spaces instead of underscores kind of convey that...) > grep "\"key\":\"total_objects\",\"value\":\"40\"" trace.event && > grep "\"key\":\"total_bytes\",\"value\":\"409600\"" trace.event > ' > diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh > index 999982fe4a9..c878db93013 100644 > --- a/t/test-lib-functions.sh > +++ b/t/test-lib-functions.sh > @@ -1655,3 +1655,43 @@ test_subcommand () { > grep "\[$expr\]" > fi > } > + > +# Check that the given command was invoked as part of the > +# trace2-format trace on stdin. > +# > +# test_region [!] <category> <label> git <command> <args>... > +# > +# For example, to look for trace2_region_enter("index", "do_read_index", repo) > +# in an invocation of "git checkout HEAD~1", run > +# > +# GIT_TRACE2_EVENT="$(pwd)/trace.txt" GIT_TRACE2_EVENT_NESTING=10 \ > +# git checkout HEAD~1 && > +# test_region index do_read_index <trace.txt > +# > +# If the first parameter passed is !, this instead checks that > +# the given region was not entered. > +# > +test_region () { > + local expect_exit=0 > + if test "$1" = "!" > + then > + expect_exit=1 > + shift > + fi > + > + grep -e "region_enter" -e "\"category\":\"$1\",\"label\":\"$2\"" "$3" > + exitcode=$? > + > + if test $exitcode != $expect_exit > + then > + return 1 > + fi > + > + grep -e "region_leave" -e "\"category\":\"$1\",\"label\":\"$2\"" "$3" > + exitcode=$? > + > + if test $exitcode != $expect_exit > + then > + return 1 > + fi > +} > -- > gitgitgadget This patch looks good to me.
On 1/20/2021 1:20 PM, Elijah Newren wrote: > On Wed, Jan 20, 2021 at 8:54 AM Derrick Stolee via GitGitGadget > <gitgitgadget@gmail.com> wrote: ... >> # t0212/parse_events.perl intentionally omits regions and data. >> - grep -e "region_enter" -e "\"category\":\"progress\"" trace.event && >> - grep -e "region_leave" -e "\"category\":\"progress\"" trace.event && >> + test_region category progress trace.event && > > Sidenote: Hmm...about 40% of my region labels in merge-ort.c and 90% > in diffcore-rename.c have spaces in them. This function could still > be used, but I'm curious if I should change the labels (but then > again, they are testing logical regions rather than individual > functions, and the spaces instead of underscores kind of convey > that...) You should be able to use test_region "category with spaces" "progress with spaces" trace but if not, then the test_region helper could be improved to match. I do think that it's better to avoid spaces in these identifiers. Thanks, -Stolee
diff --git a/t/t0500-progress-display.sh b/t/t0500-progress-display.sh index 1ed1df351cb..c461b89dfaf 100755 --- a/t/t0500-progress-display.sh +++ b/t/t0500-progress-display.sh @@ -303,8 +303,7 @@ test_expect_success 'progress generates traces' ' "Working hard" <in 2>stderr && # t0212/parse_events.perl intentionally omits regions and data. - grep -e "region_enter" -e "\"category\":\"progress\"" trace.event && - grep -e "region_leave" -e "\"category\":\"progress\"" trace.event && + test_region category progress trace.event && grep "\"key\":\"total_objects\",\"value\":\"40\"" trace.event && grep "\"key\":\"total_bytes\",\"value\":\"409600\"" trace.event ' diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 999982fe4a9..c878db93013 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -1655,3 +1655,43 @@ test_subcommand () { grep "\[$expr\]" fi } + +# Check that the given command was invoked as part of the +# trace2-format trace on stdin. +# +# test_region [!] <category> <label> git <command> <args>... +# +# For example, to look for trace2_region_enter("index", "do_read_index", repo) +# in an invocation of "git checkout HEAD~1", run +# +# GIT_TRACE2_EVENT="$(pwd)/trace.txt" GIT_TRACE2_EVENT_NESTING=10 \ +# git checkout HEAD~1 && +# test_region index do_read_index <trace.txt +# +# If the first parameter passed is !, this instead checks that +# the given region was not entered. +# +test_region () { + local expect_exit=0 + if test "$1" = "!" + then + expect_exit=1 + shift + fi + + grep -e "region_enter" -e "\"category\":\"$1\",\"label\":\"$2\"" "$3" + exitcode=$? + + if test $exitcode != $expect_exit + then + return 1 + fi + + grep -e "region_leave" -e "\"category\":\"$1\",\"label\":\"$2\"" "$3" + exitcode=$? + + if test $exitcode != $expect_exit + then + return 1 + fi +}