Re: [PATCH v6 1/6] t0021: keep filter log files on comparison

[Junio C Hamano <gitster@pobox.com>]

Lars Schneider <larsxschneider@gmail.com> writes:

> The filter log files are modified on comparison. Write the modified log files
> to temp files for comparison to fix this.

The phrase "to fix this" implies that it is _wrong_ to modify after
comparing it, but it is unclear _why_ you think is wrong.  After
all, the purpose of this comparison helper is to see if these two
are the same with cruft removed, and once the helper finds the
answer to that question, the current users of the comparison helper
do not reuse these files, so from _their_ point of view, there is
nothing to "fix", is there?

It would become a problem _if_ we want future users of this helper
to reuse the same expect (or actual) multiple times and start from
an unmodified one.  There may be some other reason why you do not
want the comparison to smudge these files.  Please state what that
reason is before saying "fix this".

Thanks.

>
> This is useful for the subsequent patch 'convert: add "status=delayed" to
> filter process protocol'.
>
> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
> ---
>  t/t0021-conversion.sh | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
> index 161f560446..ff2424225b 100755
> --- a/t/t0021-conversion.sh
> +++ b/t/t0021-conversion.sh
> @@ -42,10 +42,10 @@ test_cmp_count () {
>  	for FILE in "$expect" "$actual"
>  	do
>  		sort "$FILE" | uniq -c |
> -		sed -e "s/^ *[0-9][0-9]*[ 	]*IN: /x IN: /" >"$FILE.tmp" &&
> -		mv "$FILE.tmp" "$FILE" || return
> +		sed -e "s/^ *[0-9][0-9]*[ 	]*IN: /x IN: /" >"$FILE.tmp"
>  	done &&
> -	test_cmp "$expect" "$actual"
> +	test_cmp "$expect.tmp" "$actual.tmp" &&
> +	rm "$expect.tmp" "$actual.tmp"
>  }
>  
>  # Compare two files but exclude all `clean` invocations because Git can
> @@ -56,10 +56,10 @@ test_cmp_exclude_clean () {
>  	actual=$2
>  	for FILE in "$expect" "$actual"
>  	do
> -		grep -v "IN: clean" "$FILE" >"$FILE.tmp" &&
> -		mv "$FILE.tmp" "$FILE"
> +		grep -v "IN: clean" "$FILE" >"$FILE.tmp"
>  	done &&
> -	test_cmp "$expect" "$actual"
> +	test_cmp "$expect.tmp" "$actual.tmp" &&
> +	rm "$expect.tmp" "$actual.tmp"
>  }
>  
>  # Check that the contents of two files are equal and that their rot13 version