From: Remi Galan Alfonso <remi.galan-alfonso@ensimag.grenoble-inp.fr>
Date: Tue, 26 Jul 2016 15:18:49 GMT
Subject: Re: [PATCH v1 2/3] convert: modernize tests
Message-ID: <323759193.1122705.1469546329160.JavaMail.zimbra@ensimag.grenoble-inp.fr>
In-Reply-To: <20160722154900.19477-3-larsxschneider@gmail.com>

Hi Lars,

Sorry, minor nit that I noticed a couple of days ago but didn't
comment on the moment and forgot until now.

Lars Schneider <larsxschneider@gmail.com> wrote:
> Use `test_config` to set the config, check that files are empty with
> `test_must_be_empty`, compare files with `test_cmp`, and remove spaces
> after ">".

Considering how close it is to your patch, you might also want to
remove spaces after '<'.

There is only one occurrence in this file and it's in a line you are
already modifying.

See below:

>  test_expect_success check '
>  
> -        cmp test.o test &&
> -        cmp test.o test.t &&
> +        test_cmp test.o test &&
> +        test_cmp test.o test.t &&
>  
>          # ident should be stripped in the repository
>          git diff --raw --exit-code :test :test.i &&
> @@ -47,10 +47,10 @@ test_expect_success check '
>          embedded=$(sed -ne "$script" test.i) &&
>          test "z$id" = "z$embedded" &&
>  
> -        git cat-file blob :test.t > test.r &&
> +        git cat-file blob :test.t >test.r &&
>  
> -        ./rot13.sh < test.o > test.t &&
> -        cmp test.r test.t
> +        ./rot13.sh < test.o >test.t &&

Here.

> +        test_cmp test.r test.t
>  '

Thanks,
Rémi