Re: [PATCH] t1308: do not get fooled by symbolic links to the source tree

From: Jeff King <peff@peff.net>
Date: Jun 3, 2016, 01:07 UTC
Message-ID: <20160603010717.GA16311@sigill.intra.peff.net>
In-Reply-To: <CAGZ79kY1ygqd4VmwMj4AGTxo2bO6HFKoShzv6S2MxFq6QNjM1w@mail.gmail.com>

On Thu, Jun 02, 2016 at 04:23:00PM -0700, Stefan Beller wrote:

Show 20 quoted lines

> > To prevent this in the future, I switched my default --root= to point to
> > a symlink. I wonder if we could do something in the test suite, though,
> > as we did long ago by introducing "trash directory" with a space to
> > catch corner cases.
> >
> > I guess it would be something like:
> >
> >   if test_have_prereq SYMLINKS
> 
> IIUC this would need each test to be marked with SYMLINKS
> when testing with symlinks is desired. Marking a test with that
> is easily forgotten, so I'd rather do it by default as:
> 
> if (system supports symlinks):
> >   then
> >         mkdir "$TRASH_DIRECTORY.real" &&
> >         ln -s "$TRASH_DIRECTORY.real" "$TRASH_DIRECTORY"
> >   else
> >         mkdir "$TRASH_DIRECTORY"
> >   fi

I'm not sure I understand. My intent was to say "does the system support symlinks" in test-lib.sh when setting up the trash directory. The test_have_prereq function asks that, AFAIK (the "test_" prefix is not "does _this_ test require it" but just "this function is in the test namespace").

> I like the idea of testing with symlinks. (Does it have performance issues
> when everything goes through symlinks?)

I didn't notice any on my system (running with --root pointed at a directory, and at a symlink to that directory). It would be extra work whenever we determine a canonical absolute path, but I suspect that is drowned out in the noise of the rest of the test suite. Plus some minor extra work in the `ln` and `rm` calls during test setup/teardown, but likewise that should be a small part of the total cost.

> On the other hand if we do tests by default in a symlinked path, we could
> introduce errors more easily in non-symlinked path, but that is what developers
> use for developing (I guess), so it's not as likely?

True, but I'd be surprised if there are bugs that show up in a non-symlinked path that _do_ show up in a symlinked one.

I'm not sure if we've seen a case where this would find an actual bug in git, though. The cases I remember were mostly about the test suite being picky (i.e., git canonicalizes but the expected output doesn't, or vice versa).

-Peff