2016-03-18 13:00 GMT+08:00 Jeff King <peff@xxxxxxxx>:
> On Fri, Mar 18, 2016 at 12:34:04PM +0800, 惠轶群 wrote:
>
>> >> +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.
>
> I suspect it is because you have $XDG_RUNTIME_DIR defined in your
> environment, which causes the shell to automatically export it. I don't,
> so an explicit "export" is required to for the variable to make it to
> its children.
Yes, that's the problem. the explicit "export" is new knowledge for me, thanks.
> I think we should actually be unsetting it in test-lib.sh for all tests,
> as we do for XDG_CONFIG_HOME. That makes sure the tests are running with
> a known state.
Well, I seems a good choice.
> For the non-XDG_RUNTIME_DIR tests, does this mean we are creating the
> socket in /tmp? I'm not entirely happy with that, as we usually try to
> have the test suite avoid touching anything outside of its trash
> directories.
>
>> > 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.
>
> OK. My main concern was just that the tests would take too long, but the
> slow one is the cache test at the end, which is not repeated. So I think
> this is fine.
>
>> > 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?
>
> Sorry, yes, I used "pipe" and "socket" interchangeably there.
>
>> What is not designed? cleanup being done, my tests passing or the
>> synchronization?
>
> The synchronization. If the daemon were implemented as:
>
> if (!strcmp(action.buf, "exit")) {
> /* acknowledge that we got command */
> fclose(out);
> exit(0);
> }
>
> for example, then the client would exit at the same that the daemon is
> cleaning up the socket, and we may or may not call test_path_is_missing
> before the cleanup is done.
>
> I think it's OK to rely on that, but we may want to put a comment to
> that effect in the daemon code so that it doesn't get changed.
The current implementation is natural for me. But having additional comment
is better.
>
> -Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html