From: Phillip Wood <phillip.wood@talktalk.net>
Date: Wed, 21 Feb 2018 11:28:31 GMT
Subject: Re: [PATCH v2 5/9] t3701: add failing test for pathological context lines
Message-ID: <69548d34-3982-6eb7-2620-6e223ab80aaf@talktalk.net>
In-Reply-To: <20180219112910.24471-6-phillip.wood@talktalk.net>

On 19/02/18 11:29, Phillip Wood wrote:
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
> 
> When a hunk is skipped by add -i the offsets of subsequent hunks are
> not adjusted to account for any missing insertions due to the skipped
> hunk. Most of the time this does not matter as apply uses the context
> lines to apply the subsequent hunks in the correct place, however in
> pathological cases the context lines will match at the now incorrect
> offset and the hunk will be applied in the wrong place. The offsets of
> hunks following an edited hunk that has had the number of insertions
> or deletions changed also need to be updated in the same way. Add
> failing tests to demonstrate this.
> 
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
> 
> Notes:
>      changes since v1:
>       - changed edit test to use the existing fake editor and to strip
>         the hunk header and some context lines as well as deleting an
>         insertion
>       - style fixes
> 
>   t/t3701-add-interactive.sh | 31 +++++++++++++++++++++++++++++++
>   1 file changed, 31 insertions(+)
> 
> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
> index 70748393f28c93f4bc5d43b07bd96bd208aba3e9..06c4747f506a1b05ccad0f9387e1fbd69cfd7784 100755
> --- a/t/t3701-add-interactive.sh
> +++ b/t/t3701-add-interactive.sh
> @@ -511,4 +511,35 @@ test_expect_success 'status ignores dirty submodules (except HEAD)' '
>   	! grep dirty-otherwise output
>   '
>   
> +test_expect_success 'set up pathological context' '
> +	git reset --hard &&
> +	test_write_lines a a a a a a a a a a a >a &&
> +	git add a &&
> +	git commit -m a &&
> +	test_write_lines c b a a a a a a a b a a a a >a &&
> +	test_write_lines     a a a a a a a b a a a a >expected-1 &&
> +	test_write_lines   b a a a a a a a b a a a a >expected-2 &&
> +	# check editing can cope with missing header and deleted context lines
> +	# as well as changes to other lines
> +	test_write_lines +b " a" >patch
> +'
> +
> +test_expect_failure 'add -p works with pathological context lines' '
> +	git reset &&
> +	printf "%s\n" n y |
> +	git add -p &&
> +	git cat-file blob :a >actual &&
> +	test_cmp expected-1 actual
> +'
> +
> +test_expect_failure 'add -p patch editing works with pathological context lines' '
> +	git reset &&
> +	test_set_editor "$FAKE_EDITOR" &&
In light of the discussion of patch 2, I think this line should be deleted


> +	# n q q below is in case edit fails
> +	printf "%s\n" e y    n q q |
> +	git add -p &&
> +	git cat-file blob :a >actual &&
> +	test_cmp expected-2 actual
> +'
> +
>   test_done
>