Re: [PATCH v4 4/4] t7800: run both builtin and scripted difftool, for now
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 9, 2017, 01:38 UTC
- Message-ID
- <xmqqtw99ypvq.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <0ae4a950a4cd2c8c4f05a6b46c60f127611580cf.1483373635.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
Show 5 quoted lines
> This is uglier than a simple > > touch "$GIT_EXEC_PATH/use-builtin-difftool" > > of course. But oh well.
That is totally irrelevant.
The more important point is that we do the same set of tests so instead of running just one, we run BOTH. If we did a bit more work, we could even say "even when there is no Perl installed, we run tests with only the new built-in one". This does not go that far yet, but that should not be too hard, I would think. See below.
Show 10 quoted lines
> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh > index e94910c563..273ab55723 100755 > --- a/t/t7800-difftool.sh > +++ b/t/t7800-difftool.sh > @@ -23,6 +23,20 @@ prompt_given () > test "$prompt" = "Launch 'test-tool' [Y/n]? branch" > } > > +for use_builtin_difftool in false true > +do
Instead of the above, I'd suggest to make the loop like so:
for use_builtin_difftool in $TESTED_VARIANTS do
and before the loop begins, set TESTED_VARIANTS to either "false true" or "true" by checking the PERL prerequisite.
Show 12 quoted lines
> +test_expect_success 'verify we are running the correct difftool' ' > + if test true = '$use_builtin_difftool' > + then > + test_must_fail ok=129 git difftool -h >help && > + grep "g, --gui" help > + else > + git difftool -h >help && > + grep "g|--gui" help > + fi > +' > + > # NEEDSWORK: lose all the PERL prereqs once legacy-difftool is retired.
And then we can lose this comment. As all existing tests have PERL prereq that can go away with the above suggested change, we'd need to touch quite a many lines with "s/_success PERL /_success /".
It may be a good time to indent these existing tests to make it clear that they are inside the new "for use_builtin_difftool" loop.
Show 10 quoted lines
> +test true != $use_builtin_difftool || break > + > +test_expect_success 'tear down for re-run' ' > + rm -rf * .[a-z]* && > + git init > +' > + > +# run as builtin difftool now > +GIT_CONFIG_PARAMETERS="'difftool.usebuiltin=true'" > +export GIT_CONFIG_PARAMETERS
... and indentation would extend to these newly added lines, of course.
> +done > + > test_done