Jeff King [off-list ref] writes:
On Sat, Nov 26, 2016 at 02:01:36PM +0100, Johannes Schindelin wrote:
quoted
quoted
If you want to control it from outside the test script, you'd need
something like:
if test "$GIT_TEST_DIFFTOOL" = "builtin"
That is a bit magic. I first used "GIT_USE_BUILTIN_DIFFTOOL" and it did
not work. My name is arguably more correct (see also Jakub's note about
"naming is hard"), but yours works because there is a "TEST" substring in
it.
Yes. You are free to add an exception to the env list in test-lib.sh,
but we usually use GIT_TEST_* to avoid having to do so.
Perhaps
- The switch between "do I use builtin, or scripted?" mechanism in
1/2 can look at an environment (just like the old "am" rewrite
series did), instead of configuration. This would make the code
a lot more simppler (you do not have to worry about the
interaction between "setup" and .git/config).
- That environment variable can be named GIT_TEST_BUILTIN_DIFFTOOL;
after all, people are opting into helping to test the new shiny
to make/prove it ready sooner.
- The bulk of the existing test for difftool can be moved to a
dot-included file (in a way similar to t/annotate-tests are
usable to test both annotate and blame-imitating-annotate).
Existing PERL prerequisites can all be lost.
- Two tests can include that dot-included file; one would
explicitly unset that environment (and gives up without PERL
prerequisite), while the other explicitly sets it.
Hi,
On Mon, 28 Nov 2016, Junio C Hamano wrote:
Jeff King [off-list ref] writes:
quoted
On Sat, Nov 26, 2016 at 02:01:36PM +0100, Johannes Schindelin wrote:
quoted
quoted
If you want to control it from outside the test script, you'd need
something like:
if test "$GIT_TEST_DIFFTOOL" = "builtin"
That is a bit magic. I first used "GIT_USE_BUILTIN_DIFFTOOL" and it did
not work. My name is arguably more correct (see also Jakub's note about
"naming is hard"), but yours works because there is a "TEST" substring in
it.
Yes. You are free to add an exception to the env list in test-lib.sh,
but we usually use GIT_TEST_* to avoid having to do so.
Perhaps
- The switch between "do I use builtin, or scripted?" mechanism in
1/2 can look at an environment (just like the old "am" rewrite
series did), instead of configuration. This would make the code
a lot more simppler (you do not have to worry about the
interaction between "setup" and .git/config).
- That environment variable can be named GIT_TEST_BUILTIN_DIFFTOOL;
after all, people are opting into helping to test the new shiny
to make/prove it ready sooner.
- The bulk of the existing test for difftool can be moved to a
dot-included file (in a way similar to t/annotate-tests are
usable to test both annotate and blame-imitating-annotate).
Existing PERL prerequisites can all be lost.
- Two tests can include that dot-included file; one would
explicitly unset that environment (and gives up without PERL
prerequisite), while the other explicitly sets it.
If my main worry was the test suite, I would agree with this plan.
However, I have been bitten time and again by problems that occurred only
in production, our test suite (despite taking already waaaaaay too long to
be truly useful in my daily development) was simply not good enough.
So my plan was different: to let end users opt-in to test this new beast
thoroughly, more thoroughly than any review would.
And for that, environment variables are just not an option. I need
something that can be configured in a portable application, so that the
main Git for Windows installation is unaffected.
My original "create a file in libexec/git-core/" was simple, did the job
reliably, and worked also for testing.
It is a pity that you two gentlemen shot it down for being inelegant. And
ever since, we try to find a solution that is as simple, works as
reliably, also for testing, *and* appeases your tastes.
Ciao,
Dscho
Hello,
W dniu 28.11.2016 o 18:34, Johannes Schindelin pisze:
My original "create a file in libexec/git-core/" was simple, did the job
reliably, and worked also for testing.
It is a pity that you two gentlemen shot it down for being inelegant. And
ever since, we try to find a solution that is as simple, works as
reliably, also for testing, *and* appeases your tastes.
I just would like to note that existence of file is used for both
git-daemon and gitweb (the latter following the git-daemon example).
So there is a precedent for the use of this mechanism.
Best,
--
Jakub Narębski