Re: [PATCH v2 22/27] userdiff tests: test hunk headers on accumulated files
- From
Johannes Sixt <j6t@kdbg.org>
- Date
- Feb 15, 2021, 18:29 UTC
- Message-ID
- <eebdd570-ba21-037c-c608-ec0989956bfa@kdbg.org>
- In-Reply-To
- <20210215154427.32693-23-avarab@gmail.com>
Am 15.02.21 um 16:44 schrieb Ævar Arnfjörð Bjarmason:
Show 16 quoted lines
> The existing tests in "t/t4018/" are unrealistic in that they're all > setting up small few-line isolated test cases with one thing we could > match as a hunk header, right above the one change in the file. > > Expand those tests by accumulating changes within the same file type > in the "test_diff_funcname" function. So e.g. for "bash" we'll end up > a "bash.acc" file with 15 s/ChangeMe/IWasChanged/ changes. > > This stress tests whether the hunk header selection will "jump across" > to an earlier change because the match for that is greedier. > > As it turns out we had one false positive in "t/t4018/cpp.sh" and > "t4018/matlab.sh" because of how the tests were structured, we must > always give the "ChangeMe" line at least one line of separation from > the header, since it was at the end of those tests we'd select the > "wrong" header. Let's adjust the spacing to compensate.
I can't wrap my head around this. The ChangeMe lines are one line away from the RIGHT lines. Why would we need a line below ChangeMe?
Or is this just a fall-out from this change itself? Then I'd appreciate to describe it as "This change now triggers false positives in... because... Let's adjust the spacing compensate.".
Show 95 quoted lines
>
> So in the end we found nothing of interest here, regardless, I think
> it is useful to continue to test in this mode. It's likely to aid in
> finding bugs in combinations of our positive and negative matching as
> we add more built-in patterns.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> t/t4018-diff-funcname.sh | 19 +++++++++++++++++++
> t/t4018/cpp.sh | 1 +
> t/t4018/matlab.sh | 3 +++
> 3 files changed, 23 insertions(+)
>
> diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
> index 2efe4e5bdd..8b4500037f 100755
> --- a/t/t4018-diff-funcname.sh
> +++ b/t/t4018-diff-funcname.sh
> @@ -65,12 +65,26 @@ test_diff_funcname () {
> do_change_me "$what"
> ' &&
>
> + test_expect_success "setup: $desc (accumulated)" '
> + cat arg.test >>arg.tests &&
> + cp arg.tests "$what".acc &&
> + git add "$what".acc &&
> + do_change_me "$what".acc
> + ' &&
> +
> test_expect_success "$desc" '
> git diff -U1 "$what" >diff &&
> last_diff_context_line diff >actual &&
> test_cmp expected actual
> ' &&
>
> + test_expect_success "$desc (accumulated)" '
> + git diff -U1 "$what".acc >diff &&
> + last_diff_context_line diff >actual.lines &&
> + tail -n 1 actual.lines >actual &&
> + test_cmp expected actual
> + ' &&
> +
> test_expect_success "teardown: $desc" '
> # In case any custom config was set immediately before
> # the test itself in the test file
> @@ -93,6 +107,11 @@ do
> echo "$what" >arg.what
> ' &&
>
> + test_expect_success "setup: hunk header for $what (accumulated)" '
> + >arg.tests &&
> + echo "$what.acc diff=$what" >>.gitattributes
> + ' &&
> +
> . "$test"
> done
>
> diff --git a/t/t4018/cpp.sh b/t/t4018/cpp.sh
> index 185d40d5ef..e0ab749316 100755
> --- a/t/t4018/cpp.sh
> +++ b/t/t4018/cpp.sh
> @@ -206,6 +206,7 @@ void wrong()
> struct RIGHT_iterator_tag {};
>
> int ChangeMe;
> +
> EOF_TEST
>
> test_diff_funcname 'cpp: template function definition' \
> diff --git a/t/t4018/matlab.sh b/t/t4018/matlab.sh
> index f62289148e..fba410e6f5 100755
> --- a/t/t4018/matlab.sh
> +++ b/t/t4018/matlab.sh
> @@ -31,6 +31,7 @@ EOF_HUNK
> %%% RIGHT section
> # this is octave script
> ChangeMe = 1;
> +
> EOF_TEST
>
> test_diff_funcname 'matlab: octave section 2' \
> @@ -40,6 +41,7 @@ EOF_HUNK
> ## RIGHT section
> # this is octave script
> ChangeMe = 1;
> +
> EOF_TEST
>
> test_diff_funcname 'matlab: section' \
> @@ -49,4 +51,5 @@ EOF_HUNK
> %% RIGHT section
> % this is understood by both matlab and octave
> ChangeMe = 1;
> +
> EOF_TEST
> -- Hannes