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.
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.
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.
-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