On 4/21/2014 3:24 PM, Felipe Contreras wrote:
> Ilya Bobyr wrote:
>> On Mon, Apr 21, 2014 at 2:35 PM, Felipe Contreras <
>> felipe.contre...@gmail.com> wrote:
>>> Ilya Bobyr wrote:
>>>> test_expect_success 'setup' "
>>>> mkdir -p .git/hooks &&
>>>> cat > .git/hooks/update-branch <<-\\EOF &&
>>>> echo \$@ > .git/update-branch.args
>>>> chmod +x .git/hooks/update-branch &&
>>>> echo one > content &&
>>>> git add content &&
>>>> git commit -a -m one
>>> That is not maintainable at all.
>> Maybe you could explain how is this less maintainable, compared to a separate
> Do I really have to explain that manually escaping a shell script is not
This is rude.
Here is how you can do it without escaping:
test_expect_success 'setup' '
mkdir -p .git/hooks &&
cat > .git/hooks/update-branch <<-\EOF &&
echo $@ > .git/update-branch.args
chmod +x .git/hooks/update-branch &&
echo one > content &&
git add content &&
git commit -a -m one
It is not different from most of the tests, I think.
>> This is how it is suggested by t/README and how it is done in the other
>> test suites.
>> I can not see how your case is different, but I might be missing something.
> Let's take a cursoy look at `git grep -l "'EOF'" t`.
So the point is that some existing tests violate best practices?
I do not think this is a good justification to do the same for new tests.
> In fact my version is actually cleaner than these, because the code that is
> outside the cage is clearly delimited by a function.
It depends on the perspective.
If it fails, the failure would be missed regardless of if it is in a
function or not.
Most examples that you quoted only create files outside test_expect_success.
Even that is not necessary.
I am not telling you how you should write it.
I am just saying that you are breaking one of the recommendations on how
to write tests.
There are different options that adhere to the suggestions in t/README.
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html