Re: [PATCH v3 3/4] t7800: modernize tests

[John Keeping <john@keeping.me.uk>]

On Fri, Mar 22, 2013 at 08:13:46AM +0100, Johannes Sixt wrote:
> Am 3/21/2013 8:41, schrieb Johannes Sixt:
> > Am 3/20/2013 23:59, schrieb David Aguilar:
> >> I started digging in and the @worktree_files (aka @worktree above)
> >> is populated from the output of "git diff --raw ...".
> >>
> >> Seeing the "output" filename in "diff --raw" implies that one of the
> >> tests added "output" to the index somehow.  I do not see that
> >> happening anywhere, though, so I do not know how it would end up in
> >> the @worktree array if it is not reported by "diff --raw".
> >>
> >>
> >> My current understanding of how it could possibly be open twice:
> >>
> >> 1. via the >output redirect
> >> 2. via the copy() perl code which is fed by @worktree
> >>
> >> So I'm confused.  Why would we get different results on Windows?
> > 
> > I tracked down the difference between Windows and Linux, and it is...
> > 
> > 	for my $file (@worktree) {
> > 		next if $symlinks && -l "$b/$file";
> > 
> > ... this line in sub dir_diff. On Linux, we take the short-cut, but on
> > Windows we proceed through the rest of the loop,
> 
> And that is likely by design. From the docs:
> 
> --symlinks
> --no-symlinks
> 
>     git difftool's default behavior is create symlinks to the working
>     tree when run in --dir-diff mode.
> 
>     Specifying `--no-symlinks` instructs 'git difftool' to create
>     copies instead.  `--no-symlinks` is the default on Windows.
> 
> And indeed, we have this initialization:
> 
> 	my %opts = (
> 		...
> 		symlinks => $^O ne 'cygwin' &&
> 				$^O ne 'MSWin32' && $^O ne 'msys',
> 		...
> 	);
> 
> Can the --dir-diff tests case pass on Cygwin when neither --symlinks nor
> --no-symlinks is passed?
> 
> Perhaps the right solution is this:

We already have tests that explicitly pass '--symlinks'.  I wonder if it
would be better to change "output" to ".git/output", which should avoid
the problem by moving the output file out of the working tree.

> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
> index c6d6b1c..19238f6 100755
> --- a/t/t7800-difftool.sh
> +++ b/t/t7800-difftool.sh
> @@ -328,14 +328,16 @@ test_expect_success PERL 'setup change in subdirectory' '
>  	git commit -m "modified both"
>  '
>  
> -test_expect_success PERL 'difftool -d' '
> -	git difftool -d --extcmd ls branch >output &&
> +# passing --symlinks helps Cygwin, which defaults to --no-symlinks
> +
> +test_expect_success PERL,SYMLINKS 'difftool -d' '
> +	git difftool -d --symlinks --extcmd ls branch >output &&
>  	stdin_contains sub <output &&
>  	stdin_contains file <output
>  '
>  
> -test_expect_success PERL 'difftool --dir-diff' '
> -	git difftool --dir-diff --extcmd ls branch >output &&
> +test_expect_success PERL,SYMLINKS 'difftool --dir-diff' '
> +	git difftool --dir-diff --symlinks --extcmd ls branch >output &&
>  	stdin_contains sub <output &&
>  	stdin_contains file <output
>  '
> @@ -362,16 +364,16 @@ test_expect_success PERL,SYMLINKS 'difftool --dir-diff --symlink without unstage
>  	test_cmp actual expect
>  '
>  
> -test_expect_success PERL 'difftool --dir-diff ignores --prompt' '
> -	git difftool --dir-diff --prompt --extcmd ls branch >output &&
> +test_expect_success PERL,SYMLINKS 'difftool --dir-diff ignores --prompt' '
> +	git difftool --dir-diff --symlinks --prompt --extcmd ls branch >output &&
>  	stdin_contains sub <output &&
>  	stdin_contains file <output
>  '
>  
> -test_expect_success PERL 'difftool --dir-diff from subdirectory' '
> +test_expect_success PERL,SYMLINKS 'difftool --dir-diff from subdirectory' '
>  	(
>  		cd sub &&
> -		git difftool --dir-diff --extcmd ls branch >output &&
> +		git difftool --dir-diff --symlinks --extcmd ls branch >output &&
>  		stdin_contains sub <output &&
>  		stdin_contains file <output
>  	)
> 
> (Only tested on MinGW, which skips the tests.) I leave it to you
> to write --no-symlinks tests.
> 
> -- Hannes