Re: [PATCH] tests: turn on network daemon tests by default

From: Jeff King <peff@peff.net>
Date: Feb 12, 2014, 22:12 UTC
Message-ID: <20140212221258.GB6799@sigill.intra.peff.net>
In-Reply-To: <xmqq61oknoak.fsf@gitster.dls.corp.google.com>

On Wed, Feb 12, 2014 at 11:06:43AM -0800, Junio C Hamano wrote:

Show 9 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > Agreed, but I think the only way to know the size of those fallouts is
> > to try it and see who complains.  I would not normally be so cavalier
> > with git itself, but I think for the test infrastructure, we have a
> > small, tech-savvy audience that can help us iterate on it without too
> > much pain.
> 
> There is another.

I somehow pictured this while reading your email: http://youtu.be/X4JVcvR7IM0

Show 11 quoted lines

> $ GIT_TEST_HTTPD=false sh t5537-fetch-shallow.sh 
> ok 1 - setup
> ok 2 - setup shallow clone
> ok 3 - clone from shallow clone
> ok 4 - fetch from shallow clone
> ok 5 - fetch --depth from shallow clone
> ok 6 - fetch --unshallow from shallow clone
> ok 7 - fetch something upstream has but hidden by clients shallow boundaries
> ok 8 - fetch that requires changes in .git/shallow is filtered
> ok 9 - fetch --update-shallow
> error: Can't use skip_all after running some tests

Ah, yeah, I did not notice that t5537 does its own special handling of GIT_TEST_HTTPD. I think it is wrong to do so, and it is already buggy in the case that GIT_TEST_HTTPD is set, but apache cannot be started.

E.g., with the current "master":

  $ sudo chmod -x `which apache2`
  $ GIT_TEST_HTTPD=1 ./t5537-fetch-shallow.sh
  ok 1 - setup
  ok 2 - setup shallow clone
  ok 3 - clone from shallow clone
  ok 4 - fetch from shallow clone
  ok 5 - fetch --depth from shallow clone
  ok 6 - fetch --unshallow from shallow clone
  ok 7 - fetch something upstream has but hidden by clients shallow boundaries
  ok 8 - fetch that requires changes in .git/shallow is filtered
  ok 9 - fetch --update-shallow
  error: Can't use skip_all after running some tests

lib-httpd was never designed to be included from anywhere except the beginning of the file. But that wouldn't be right for t5537, because it wants to run some of the tests, even if apache setup fails. The right way to do it is probably to have lib-httpd do all of its work in a lazy prereq. I don't know how clunky that will end up, though; it might be simpler to just move the shallow http test into one of the http-fetch scripts.

-Peff