Re: [PATCH v6 1/6] t0021: keep filter log files on comparison
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jun 25, 2017, 22:12 UTC
- Message-ID
- <xmqqtw337m4b.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20170625182125.6741-2-larsxschneider@gmail.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.
Show 41 quoted lines
>
> 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