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

Ignacio Encinas Rubio [off-list ref] writes:

quoted

quoted

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