Re: [PATCH 1/2] t1300: put git invocations inside test function

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 19, 2011, 07:04 UTC
Message-ID: <7vzkgxcviz.fsf@alter.siamese.dyndns.org>
In-Reply-To: <4E9E7115.60303@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> writes:

Show 11 quoted lines

> This innocently looking hunk fails on Windows, because the preceding tests
> are skipped, and .git/config does not exist. I was tempted to just change
> this to 'rm -f'. But there are a few other instances of 'rm .git/config'
> in this file that were *not* moved inside the test function.
>
> How would you like this solved?
>
> - Move this one out again
> - Add -f to just this one
> - Add -f everywhere
> - a combination of the above?

Probably "rm -f .git/config" to all tests would be the most appropriate, as the desire to start from an empty config file is a sign that each test wants to be as independent as possible from previous tests.

An alternative approach to achieve the independence would be to make each test responsible for cleaning up after itself, with test_when_finished, but that won't work for the first one, as the test framework would give you the original one. Besides, that's trying to be cooperative when each test can fail in an unexpected way (after all the purpose of tests is to fail when things go wrong). In order to achieve isolation, each test protecting themselves from others, as long as it can be easily and reasonably done (e.g. a single "rm -f"), is probably far more preferrable than each test trying to clean after itself, risking possible failure to do so.