Re: [GSoC][PATCH v4 4/4] credential-cache: add tests for XDG functionality

2017-03-16 Thread Devin Lehmacher
> > > +test_expect_success 'credential-cache --socket option overrides default 
> > > location' '
> > > + test_when_finished "rm -rf \"$HOME\"/dir/" &&
> > > + check approve "cache --socket \"$HOME/dir/socket\"" <<-\EOF &&
> > > + protocol=https
> > > + host=example.com
> > > + username=store-user
> > > + password=store-pass
> > > + EOF
> > > + test -S "$HOME/dir/socket" &&
> > > + git credential-cache exit
> > > +'
>
> This is almost right, except:
>
>   - the "exit" needs to be told which socket to use
>
>   - we should do the "exit" even when the test fails early (so in
> test_when_finished)
>
>   - the test_when_finished block will interpolate $HOME when setting up
> the block, which will break if it contains double-quotes or other
> special characters. It should use \$HOME.
>
> I suspect the "check" invocation needs to do so as well, though it
> is even trickier (we shove it into a single-quoted "-c" argument).
>
> I think you could get by in both cases with relative paths.

Using a relative path won't work since `git credential-cache --socket`
only accepts absolute paths. It shouldn't be too hard to escape the
string though.

-Devin


Re: [GSoC][PATCH v4 4/4] credential-cache: add tests for XDG functionality

2017-03-16 Thread Jeff King
On Thu, Mar 16, 2017 at 09:29:58AM -0700, Junio C Hamano wrote:

> Devin Lehmacher  writes:
> 
> > @@ -20,4 +21,67 @@ helper_test_timeout cache --timeout=1
> >  # our socket, leaving us with no way to access the daemon.
> >  git credential-cache exit
> >  
> > +# we need to use rm -rf here since sometimes the daemon hasn't finished
> > +# cleaning up after itself and rmdir fails
> 
> H.  Peff, do you have ideas on better ways to do this (or
> explanation why this is the best we could do)?

If you call "git credential-cache exit", that should be deterministic.
The client program won't exit until the other side closes the
descriptor, which it won't do until it has cleaned up the socket. See
the comment at line 130 of credential-cache--daemon.c.

So here:

> > +test_expect_success 'credential-cache --socket option overrides default 
> > location' '
> > +   test_when_finished "rm -rf \"$HOME\"/dir/" &&
> > +   check approve "cache --socket \"$HOME/dir/socket\"" <<-\EOF &&
> > +   protocol=https
> > +   host=example.com
> > +   username=store-user
> > +   password=store-pass
> > +   EOF
> > +   test -S "$HOME/dir/socket" &&
> > +   git credential-cache exit
> > +'

This is almost right, except:

  - the "exit" needs to be told which socket to use

  - we should do the "exit" even when the test fails early (so in
test_when_finished)

  - the test_when_finished block will interpolate $HOME when setting up
the block, which will break if it contains double-quotes or other
special characters. It should use \$HOME.

I suspect the "check" invocation needs to do so as well, though it
is even trickier (we shove it into a single-quoted "-c" argument).

I think you could get by in both cases with relative paths.

So all together, probably:

  test_when_finished "
git credential-cache --socket dir/socket &&
rmdir dir
  " &&
  check approve "cache --socket dir/socket" <<-\EOF &&
  ...
  EOF
  test -S dir/socket

The final "test -S" should be OK in practice. We're assuming that the
cache-daemon is still running, but it has a 900 second timeout by
default (and besides, all the other tests have exact same race).

-Peff


Re: [GSoC][PATCH v4 4/4] credential-cache: add tests for XDG functionality

2017-03-16 Thread Junio C Hamano
Devin Lehmacher  writes:

> @@ -20,4 +21,67 @@ helper_test_timeout cache --timeout=1
>  # our socket, leaving us with no way to access the daemon.
>  git credential-cache exit
>  
> +# we need to use rm -rf here since sometimes the daemon hasn't finished
> +# cleaning up after itself and rmdir fails

H.  Peff, do you have ideas on better ways to do this (or
explanation why this is the best we could do)?

> +test_expect_success 'credential-cache --socket option overrides default 
> location' '
> + test_when_finished "rm -rf \"$HOME\"/dir/" &&
> + check approve "cache --socket \"$HOME/dir/socket\"" <<-\EOF &&
> + protocol=https
> + host=example.com
> + username=store-user
> + password=store-pass
> + EOF
> + test -S "$HOME/dir/socket" &&
> + git credential-cache exit
> +'
> +
> +XDG_CACHE_HOME="$HOME/xdg"
> +export XDG_CACHE_HOME
> +# test behavior when XDG_CACHE_HOME is set
> +helper_test cache
> +
> +test_expect_success "use custom XDG_CACHE_HOME if set and default sockets 
> are not created" '
> + test -S "$XDG_CACHE_HOME/git/credential/socket" &&
> + test_path_is_missing "$HOME/.git-credential-cache/socket" &&
> + test_path_is_missing "$HOME/.cache/git/credential/socket" &&
> + git credential-cache exit
> +'
> +unset XDG_CACHE_HOME
> +
> +test_expect_success "use custom XDG_CACHE_HOME even if xdg socket exists" '
> + check approve cache <<-\EOF &&
> + protocol=https
> + host=example.com
> + username=store-user
> + password=store-pass
> + EOF
> + test -S "$HOME/.cache/git/credential/socket" &&
> + XDG_CACHE_HOME="$HOME/xdg" &&
> + export XDG_CACHE_HOME &&
> + check approve cache <<-\EOF &&
> + protocol=https
> + host=example.com
> + username=store-user
> + password=store-pass
> + EOF
> + test -S "$HOME/xdg/git/credential/socket" &&
> + git credential-cache exit &&
> + unset XDG_CACHE_HOME

This unset will not run if any of the above steps since it was set
and exported fails.  It probably should be in test_when_finished and
should use safe_unset shell function instead.

> +'
> +
> +# we need to use rm -rf here since sometimes the daemon hasn't finished
> +# cleaning up after itself and rmdir fails
> +test_expect_success 'use user socket if user directory exists' '
> + test_when_finished "rm -rf \"$HOME/.git-credential-cache/\"" &&
> + mkdir -p -m 700 "$HOME/.git-credential-cache/" &&
> + check approve cache <<-\EOF &&
> + protocol=https
> + host=example.com
> + username=store-user
> + password=store-pass
> + EOF
> + test -S "$HOME/.git-credential-cache/socket" &&

We should also test that the XDG location is not touched at the same
time that the traditional directory was used for the socket.

> + git credential-cache exit
> +'

This last test should be replicated to also check the case where we
have a symbolic link at ~/.git-credential-cache that points at a
directory.

>  test_done

Thanks.