All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Narebski <jnareb@gmail.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	git@vger.kernel.org, Jeff King <peff@peff.net>
Subject: Re: [PATCH v4 7/7] t/README: Document the do's and don'ts of tests
Date: Tue, 6 Jul 2010 15:19:50 +0200	[thread overview]
Message-ID: <201007061519.51632.jnareb@gmail.com> (raw)
In-Reply-To: <AANLkTikFOAWINvKINvPbbrqBNlmjDcn7uJVQ7doVbhem@mail.gmail.com>

On Tue, 6 Jul 2010, Ævar Arnfjörð Bjarmason wrote:
> 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.

>> 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

  reply	other threads:[~2010-07-06 13:20 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-07-02 14:59 [PATCH v4 0/7] Improvements for t/README Ævar Arnfjörð Bjarmason
2010-07-02 14:59 ` [PATCH v4 1/7] t/README: The trash is in 't/trash directory.$name' Ævar Arnfjörð Bjarmason
2010-07-02 14:59 ` [PATCH v4 2/7] t/README: Typo: paralell -> parallel Ævar Arnfjörð Bjarmason
2010-07-02 14:59 ` [PATCH v4 3/7] t/README: Document the prereq functions, and 3-arg test_* Ævar Arnfjörð Bjarmason
2010-07-02 14:59 ` [PATCH v4 4/7] t/README: Document test_external* Ævar Arnfjörð Bjarmason
2010-07-02 14:59 ` [PATCH v4 5/7] t/README: Document test_expect_code Ævar Arnfjörð Bjarmason
2010-07-02 14:59 ` [PATCH v4 6/7] t/README: Add a section about skipping tests Ævar Arnfjörð Bjarmason
2010-07-02 14:59 ` [PATCH v4 7/7] t/README: Document the do's and don'ts of tests Ævar Arnfjörð Bjarmason
2010-07-06  2:35   ` Junio C Hamano
2010-07-06  8:35     ` Jakub Narebski
2010-07-06 13:02       ` Ævar Arnfjörð Bjarmason
2010-07-06 13:19         ` Jakub Narebski [this message]
2010-07-06 12:50     ` Ævar Arnfjörð Bjarmason

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201007061519.51632.jnareb@gmail.com \
    --to=jnareb@gmail.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.