Re: [RFC PATCH 1/1] config: learn the "hostname:" includeIf condition

[Junio C Hamano <gitster@pobox.com>]

Ignacio Encinas Rubio <ignacio@iencinas.com> writes:

>>> +test_expect_success 'conditional include, hostname' '
>>> +	echo "[includeIf \"hostname:$(hostname)a\"]path=bar12" >>.git/config &&
>>> +	echo "[test]twelve=12" >.git/bar12 &&
>>> +	test_must_fail git config test.twelve &&
>> 
>> Emulating other tests in this file that uses here document may make
>> it a bit easier to read?  E.g.,
>> 
>> 	cat >>.gitconfig <<-EOF &&
>> 	[includeIf "hostname:$(hostname)a"]
>> 		path = bar12
>> 	EOF
>
> Thanks for pointing that out. I just read the last tests from that file
> where they used the echo "..." >> approach. Do you think it is
> worthwhile rewriting those tests to use the approach you suggested?

I had an impression that existing ones do not have ugliness of
backslash-quoting and do not benefit from such a rewrite to use
here-document as much as the one you added does.  

If that is not the case, and existing ones would concretely improve
the readability with such a rewrite to use here-document, surely.
If we want to do that route, we should either do one of the two.

 - The patch [1/2] does such a "style update" only on existing tests
   to improve their readability, and then patch [2/2] then does your
   addition to the tests, together with the code change.

 - Or you do this patch alone, without touching existing tests, but
   with your tests added in an easier-to-read style.  And then after
   the dust settles, a separate "style udpate" patch clean the
   existing ones up.

> By the way, before contributing, I saw there is some work on moving to
> unit tests. I wasn't sure how to test this particular feature there, so
> I went with the "old" approach as it seemed more natural. Is this ok?

We are not really "moving to".  We did not have a test framework for
effective unit tests, so some tests that would have been easier to
manage as unit tests were instead written as an end-to-end
integration tests, which was what we had a framework for.  These
tests are moving to, but for a test like "the user uses the
'[includeIf X:Y] path = P' construct---does the git command really
shows the effect of include from P when condition X:Y holds?", the
unit testing framework would not be a better fit than the end-to-end
behaviour test, I would say.