Re: [PATCH/RFC/GSoC 3/3] t0301: test credential-cache support of XDG_RUNTIME_DIR

From: 惠轶群 <huiyiqun@gmail.com>
Date: Mar 18, 2016, 04:35 UTC
Message-ID: <CAKqreuyirQdzjh3Un=ieJMKLicyG7onyy=YTjZ4zOTqfV06eew@mail.gmail.com>
In-Reply-To: <20160316171535.GC4039@sigill.intra.peff.net>

2016-03-17 1:15 GMT+08:00 Jeff King <peff@peff.net>:

Show 16 quoted lines

> On Wed, Mar 16, 2016 at 06:07:45PM +0800, Hui Yiqun wrote:
>
>> diff --git a/t/t0301-credential-cache.sh b/t/t0301-credential-cache.sh
>> index 82c8411..0718bb0 100755
>> --- a/t/t0301-credential-cache.sh
>> +++ b/t/t0301-credential-cache.sh
>> @@ -12,7 +12,32 @@ test -z "$NO_UNIX_SOCKETS" || {
>>  # don't leave a stale daemon running
>>  trap 'code=$?; git credential-cache exit; (exit $code); die' EXIT
>>
>> +test_expect_success 'set $XDG_RUNTIME_DIR' '
>> +     XDG_RUNTIME_DIR=$HOME/xdg_runtime/
>> +'
>
> Doesn't this need to export the variable so that credential-cache can
> see it?

I'm not sure, but it seems that a little clean up code added before send-email make the test fail. At that time, I run test without building. I've send PATCH v2 which runs well on my computer. However, $XDG_RUNTIME_DIR is still not exported, but that just works.

I will try to dig deeper into the bash script to see why.

Show 8 quoted lines

>
>> +
>> +helper_test cache
>> +
>
> This runs the full suite of tests twice (once here, and once for the
> original helper_test invocation you left below). Shouldn't we just do it
> once (making sure that $XDG_RUNTIME_DIR is respected)?

I'd like to test the behavior of git-credential-cache when $XDG_RUNTIME_DIR is unset.

In `t/t0302-credential-store.sh`, helper_test is also run multiple times. That's why I do so.

Show 15 quoted lines

>> +test_expect_success 'force git-credential-cache to exit so that socket disappear' '
>> +     git credential-cache exit &&
>> +     test_path_is_missing "$XDG_RUNTIME_DIR/git/credential-cache.sock" &&
>> +     unset XDG_RUNTIME_DIR
>> +'
>
> I wondered if this might be racy. credential-cache tells the daemon
> "exit", then waits for a response or EOF. The daemon sees "exit" and
> calls exit(0) immediately. We clean up the socket in an atexit()
> handler. So I think we are OK (the pipe will get closed when the process
> exits, and the atexit handler must have run by then).
>
> But that definitely was not designed, and is just how it happens to
> work. I'm not sure if it's worth commenting on that (here, or perhaps in
> the daemon code).

I'm still confused.

What do you mean by "pipe"? should it be "socket" instead?

What is not designed? cleanup being done, my tests passing or the synchronization?

>
> -Pef