> Change the test library to insert non-alphanumeric ASCII characters > into the TRASH_DIRECTORY name, that's the directory the test library > creates, chdirs to and runs each individual test from. > > Unless test_fails_on_unusual_directory_names=1 is declared before > importing test-lib.sh (and if perl isn't available on the system), the > trash directory will contain every non-alphanumeric character in > ASCII, in order. At the very least there must be an easier way to disable this, e.g. a command line option. This change is sure effective in smoking out bugs, but it's a major annoyance during development when it comes to debugging a test. At first I could not even cd into the trash directory, because TAB completing the directory name with all those non-printable characters didn't work (this may be a bug in the bash-completion package). And simply copy-pasting the dirname didn't work either, because 'ls' displayed it as trash directory.t9902-completion.??????????????????????????????? !"#$%&'()*+,-:;<=>?@[\]^_`{|}~? After some headscratching, Sunday night may be my excuse, I figured out that 'cd tr*' works... only to be greeted with the ugliest-ever three-line(!) shell prompt. Therefore I would say that this should not even be enabled by default in test-lib.sh, so at least running a test directly from the command line as ./t1234-foo.sh would considerately give us an easily accessible trash directory even without any command line options. We could enable it for 'make test' by default via GIT_TEST_OPTS in t/Makefile, though. > This includes all the control characters, !, [], {} etc. the "." > character isn't included because it's already in the directory name, > and nor is "/" for obvious reasons, although that would actually work, > we'd just create a subdirectory, which would make the tests harder to > inspect when they fail.i 1. Heh. How an additional subdirectory would make the tests harder to inspect is nothing compared to the effect of all the other characters. 2. s/i$// > This change is inspired by the "submodule: prevent backslash expantion > in submodule names" patch[1]. If we'd had backslashes in the > TRASH_DIRECTORY all along that bug would have been fixed a long time > ago. This will flag such issues by marking tests that currently fail > with "test_fails_on_unusual_directory_names=1", ensure that new tests > aren't added unless a discussion is had about why the code can't > handle unusual pathnames, and prevent future regressions. > > 1. <20170407172306.172673-1-bmwill@xxxxxxxxxx> > --- > t/README | 12 ++++++++++++ > t/test-lib.sh | 4 ++++ > 2 files changed, 16 insertions(+) > > diff --git a/t/README b/t/README > index ab386c3681..314dd40221 100644 > --- a/t/README > +++ b/t/README > @@ -345,6 +345,18 @@ assignment to variable 'test_description', like this: > This test registers the following structure in the cache > and tries to run git-ls-files with option --frotz.' > > +By default the tests will be run from a directory with a highly > +unusual filename that includes control characters, a newline, various > +punctuation etc., this is done to smoke out any bugs related to path > +handling. If for whatever reason the tests can't deal with such > +unusual path names, set: > + > + test_fails_on_unusual_directory_names=1 > + > +Before sourcing 'test-lib.sh' as described below. This option is > +mainly intended to grandfather in existing broken tests & code, and > +should usually not be used in new code, instead your tests or code > +probably need fixing. > > Source 'test-lib.sh' > -------------------- > diff --git a/t/test-lib.sh b/t/test-lib.sh > index 13b5696822..089ff5ac7d 100644 > --- a/t/test-lib.sh > +++ b/t/test-lib.sh > @@ -914,6 +914,10 @@ fi > > # Test repository > TRASH_DIRECTORY="trash directory.$(basename "$0" .sh)" > +if test -z "$test_fails_on_unusual_directory_names" -a "$(perl -e 'print 1+1' 2>/dev/null)" = "2" > +then > + TRASH_DIRECTORY="$TRASH_DIRECTORY.$(perl -e 'print join q[], grep { /[^[:alnum:]]/ and !m<[./]> } map chr, 0x01..0x7f')" > +fi > test -n "$root" && TRASH_DIRECTORY="$root/$TRASH_DIRECTORY" > case "$TRASH_DIRECTORY" in > /*) ;; # absolute path is good > -- > 2.11.0