Re: [PATCH v4 7/7] t/README: Document the do's and don'ts of tests

From: Jakub Narebski <jnareb@gmail.com>
Date: Jul 6, 2010, 13:19 UTC
Message-ID: <201007061519.51632.jnareb@gmail.com>
In-Reply-To: <AANLkTikFOAWINvKINvPbbrqBNlmjDcn7uJVQ7doVbhem@mail.gmail.com>

On Tue, 6 Jul 2010, Ævar Arnfjörð Bjarmason wrote:

Show 25 quoted lines

> On Tue, Jul 6, 2010 at 08:35, Jakub Narebski <jnareb@gmail.com> wrote:
>> Junio C Hamano <gitster@pobox.com> writes:
>>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>>>
>>>> +Do:
>>>> +
>>>> + - Put as much code as possible inside test_expect_success and other
>>>> +   assertions.
>>>> +
>>>> +   Even code that isn't a test per se, but merely some setup code
>>>> +   should be inside a test assertion if at all possible. Test scripts
>>>> +   should only have trivial code outside of their assertions.
>>>
>>> Let's make it even stronger; "should only have trivial" -> "shouldn't have
>>> any ... unless there is a good reason."
>>
>> I think that the only thing that can and *should* be put outside
>> test_expect_* is creating helper file: test vectors ('expect' files),
>> configuration files, files that are to be arguments to commands, etc.
>> Is it covered by "there is a good reason"?  Isn't it too severe?
> 
> Personally I'd put `..>expect && ..>actual && test_cmp' inside
> test_expect_* too if they're only going to be used by that test, but
> outside them at the top of the file if they're files that are used by
> multiple tests for the duration of the test run.

I agree with putting e.g. `echo "sth" >expect` inside test_expect_*. It is also obvious that `.. >actual` should be inside test_expect_*.

What I was thinking about was generating larger files, by using e.g.

  cat >expected <<\EOF
  
  ...
  EOF

Putting them inside test_expect_* would make it IMHO less clear, less readable.

Sidenote: we should probably describe <<\EOF vs <<EOF here-docs and
when to use one or another in t/README.

Show 6 quoted lines

>> There probably should be description when to put creating such files
>> in test script, and when to put them as pre-made files in tXXXX/
>> subdirectory (non US-ASCII is one reason to put it as pre-made file).
> 
> I don't know which one would be preferrable. We have a lot of things
> in t/t*/* that could be generated by a test, and vice-versa.

Probably because those tests were written by diferent people, and there were no clear policy / guidelines description in t/README :-)

Thanks a lot for your work!

-- 
Jakub Narebski
Poland