The lazy prerequisite ULIMIT_STACK_SIZE is used only in t7004 so far.
Move it to test-lib.sh so that it can be used in other tests (which it will
be in a follow-up commit).
Signed-off-by: Michael J Gruber <git@grubix.eu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We already set ASAN_OPTIONS to abort if it finds any errors.
As we start to experiment with LSAN, the leak sanitizer,
it's convenient if we give it the same treatment.
Note that ASAN is actually a superset of LSAN and can do the
leak detection itself. So this only has an effect if you
specifically build with "make SANITIZE=leak" (leak detection
but not the rest of ASAN). Building with just LSAN results
in a build that runs much faster. That makes the
build-test-fix cycle more pleasant.
In the long run, once we've fixed or suppressed all the
leaks, it will probably be worth turning leak-detection on
for ASAN and just using that (to check both leaks _and_
memory errors in a single test run). But there's still a lot
of work before we get there.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The --verbose test option cannot be used with test harnesses
like "prove". Instead, you must use --verbose-log.
Since the --valgrind option implies --verbose, that means
that it cannot be used with prove. I.e., this does not work:
prove t0000-basic.sh :: --valgrind
You'd think it could be fixed by doing:
prove t0000-basic.sh :: --valgrind --verbose-log
but that doesn't work either, because the implied --verbose
takes precedence over --verbose-log. If the user has given
us a specific option, we should prefer that.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
sum(1) is a command for calculating checksums of the contents of files.
It was part of early editions of Unix ("Research Unix", 1972/1973, [1]).
cksum(1) appeared in 4.4BSD (1993) as a replacement [2], and became part
of POSIX.1-2008 [3]. OpenBSD 5.6 (2014) removed sum(1).
We only use sum(1) in t1002 to check for changes in three files. On
MinGW we use md5sum(1) instead. We could switch to the standard command
cksum(1) for all platforms; MinGW comes with GNU coreutils now, which
provides sum(1), cksum(1) and md5sum(1). Use our standard method for
checking for file changes instead: test_cmp.
It's more convenient because it shows differences nicely, it's faster on
MinGW because we have a special implementation there based only on
shell-internal commands, it's simpler as it allows us to avoid stripping
out unnecessary entries from the checksum file using grep(1), and it's
more consistent with the rest of the test suite.
We already compare changed files with their expected new contents using
diff(1), so we don't need to check with "test_must_fail test_cmp" if
they differ from their original state. A later patch could convert the
direct diff(1) calls to test_cmp as well.
With all sum(1) calls gone, remove the MinGW-specific implementation
from test-lib.sh as well.
[1] http://minnie.tuhs.org/cgi-bin/utree.pl?file=V3/man/man1/sum.1
[2] http://minnie.tuhs.org/cgi-bin/utree.pl?file=4.4BSD/usr/share/man/cat1/cksum.0
[3] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/cksum.html
Signed-off-by: René Scharfe <l.s.r@web.de>
Reviewed-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Long ago in 628522ec14 (sha1-lookup: more memory efficient
search in sorted list of SHA-1, 2007-12-29) we added
sha1_entry_pos(), a binary search that uses the uniform
distribution of sha1s to scale the selection of mid-points.
As this was a performance experiment, we tied it to the
GIT_USE_LOOKUP environment variable and never enabled it by
default.
This code was successful in reducing the number of steps in
each search. But the overhead of the scaling ends up making
it slower when the cache is warm. Here are best-of-five
timings for running rev-list on linux.git, which will have
to look up every object:
$ time git rev-list --objects --all >/dev/null
real 0m35.357s
user 0m35.016s
sys 0m0.340s
$ time GIT_USE_LOOKUP=1 git rev-list --objects --all >/dev/null
real 0m37.364s
user 0m37.045s
sys 0m0.316s
The USE_LOOKUP version might have more benefit on a cold
cache, as the time to fault in each page would dominate. But
that would be for a single lookup. In practice, most
operations tend to look up many objects, and the whole pack
.idx will end up warm.
It's possible that the code could be better optimized to
compete with a naive binary search for the warm-cache case,
and we could have the best of both worlds. But over the
years nobody has done so, and this is largely dead code that
is rarely run outside of the test suite. Let's drop it in
the name of simplicity.
This lets us remove sha1_entry_pos() entirely, as the .idx
lookup code was the only caller. Note that sha1-lookup.c
still contains sha1_pos(), which differs from
sha1_entry_pos() in two ways:
- it has a different interface; it uses a function pointer
to access sha1 entries rather than a size/offset pair
describing the table's memory layout
- it only scales the initial selection of "mi", rather
than each iteration of the search
We can't get rid of this function, as it's called from
several places. It may be that we could replace it with a
simple binary search, but that's out of scope for this patch
(and would need benchmarking).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The build procedure has been improved to allow building and testing
Git with address sanitizer more easily.
* jk/build-with-asan:
Makefile: disable unaligned loads with UBSan
Makefile: turn off -fomit-frame-pointer with sanitizers
Makefile: add helper for compiling with -fsanitize
test-lib: turn on ASan abort_on_error by default
test-lib: set ASAN_OPTIONS variable before we run git
By default, ASan will exit with code 1 when it sees an
error. This means we'll notice a problem when we expected
git to succeed, but not in a test_must_fail block.
Let's ask it to actually raise SIGABRT instead. That will
give us a signal death that test_must_fail will notice. As a
bonus, it may also leave a coredump, which can be handy for
digging into a failure.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We turn off ASan's leak detection by default in the test
suite because it's too noisy. But we don't do so until
part-way through test-lib. This is before we've run any
tests, but after we do our initial "./git" to see if the
binary has even been built.
When built with clang, this seems to work fine. However,
using "gcc -fsanitize=address", the leak checker seems to
complain more aggressively:
$ ./git
...
==5352==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 2 byte(s) in 1 object(s) allocated from:
#0 0x7f120e7afcf8 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc1cf8)
#1 0x559fc2a3ce41 in do_xmalloc /home/peff/compile/git/wrapper.c:60
#2 0x559fc2a3cf1a in do_xmallocz /home/peff/compile/git/wrapper.c:100
#3 0x559fc2a3d0ad in xmallocz /home/peff/compile/git/wrapper.c:108
#4 0x559fc2a3d0ad in xmemdupz /home/peff/compile/git/wrapper.c:124
#5 0x559fc2a3d0ad in xstrndup /home/peff/compile/git/wrapper.c:130
#6 0x559fc274535a in main /home/peff/compile/git/common-main.c:39
#7 0x7f120dabd2b0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0)
This is a leak in the sense that we never free it, but it's
in a global that is meant to last the whole program. So it's
not really interesting or in need of fixing. And at any
rate, mentioning leaks outside of the test_expect blocks is
certainly unwelcome, as it pollutes stderr.
Let's bump the setting of ASAN_OPTIONS higher in test-lib.sh
to catch our initial "can we even run git?" test. While
we're at it, we can add a comment to make it a bit less
inscrutable.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Update "perl-compatible regular expression" support to enable JIT
and also allow linking with the newer PCRE v2 library.
* ab/pcre-v2:
grep: add support for PCRE v2
grep: un-break building with PCRE >= 8.32 without --enable-jit
grep: un-break building with PCRE < 8.20
grep: un-break building with PCRE < 8.32
grep: add support for the PCRE v1 JIT API
log: add -P as a synonym for --perl-regexp
grep: skip pthreads overhead when using one thread
grep: don't redundantly compile throwaway patterns under threading
The internal implementation of "git grep" has seen some clean-up.
* ab/grep-preparatory-cleanup: (31 commits)
grep: assert that threading is enabled when calling grep_{lock,unlock}
grep: given --threads with NO_PTHREADS=YesPlease, warn
pack-objects: fix buggy warning about threads
pack-objects & index-pack: add test for --threads warning
test-lib: add a PTHREADS prerequisite
grep: move is_fixed() earlier to avoid forward declaration
grep: change internal *pcre* variable & function names to be *pcre1*
grep: change the internal PCRE macro names to be PCRE1
grep: factor test for \0 in grep patterns into a function
grep: remove redundant regflags assignments
grep: catch a missing enum in switch statement
perf: add a comparison test of log --grep regex engines with -F
perf: add a comparison test of log --grep regex engines
perf: add a comparison test of grep regex engines with -F
perf: add a comparison test of grep regex engines
perf: emit progress output when unpacking & building
perf: add a GIT_PERF_MAKE_COMMAND for when *_MAKE_OPTS won't do
grep: add tests to fix blind spots with \0 patterns
grep: prepare for testing binary regexes containing rx metacharacters
grep: add a test helper function for less verbose -f \0 tests
...
Add support for v2 of the PCRE API. This is a new major version of
PCRE that came out in early 2015[1].
The regular expression syntax is the same, but while the API is
similar, pretty much every function is either renamed or takes
different arguments. Thus using it via entirely new functions makes
sense, as opposed to trying to e.g. have one compile_pcre_pattern()
that would call either PCRE v1 or v2 functions.
Git can now be compiled with either USE_LIBPCRE1=YesPlease or
USE_LIBPCRE2=YesPlease, with USE_LIBPCRE=YesPlease currently being a
synonym for the former. Providing both is a compile-time error.
With earlier patches to enable JIT for PCRE v1 the performance of the
release versions of both libraries is almost exactly the same, with
PCRE v2 being around 1% slower.
However after I reported this to the pcre-dev mailing list[2] I got a
lot of help with the API use from Zoltán Herczeg, he subsequently
optimized some of the JIT functionality in v2 of the library.
Running the p7820-grep-engines.sh performance test against the latest
Subversion trunk of both, with both them and git compiled as -O3, and
the test run against linux.git, gives the following results. Just the
/perl/ tests shown:
$ GIT_PERF_REPEAT_COUNT=30 GIT_PERF_LARGE_REPO=~/g/linux GIT_PERF_MAKE_COMMAND='grep -q LIBPCRE2 Makefile && make -j8 USE_LIBPCRE2=YesPlease CC=~/perl5/installed/bin/gcc NO_R_TO_GCC_LINKER=YesPlease CFLAGS=-O3 LIBPCREDIR=/home/avar/g/pcre2/inst LDFLAGS=-Wl,-rpath,/home/avar/g/pcre2/inst/lib || make -j8 USE_LIBPCRE=YesPlease CC=~/perl5/installed/bin/gcc NO_R_TO_GCC_LINKER=YesPlease CFLAGS=-O3 LIBPCREDIR=/home/avar/g/pcre/inst LDFLAGS=-Wl,-rpath,/home/avar/g/pcre/inst/lib' ./run HEAD~5 HEAD~ HEAD p7820-grep-engines.sh
[...]
Test HEAD~5 HEAD~ HEAD
-----------------------------------------------------------------------------------------------------------------
7820.3: perl grep 'how.to' 0.31(1.10+0.48) 0.21(0.35+0.56) -32.3% 0.21(0.34+0.55) -32.3%
7820.7: perl grep '^how to' 0.56(2.70+0.40) 0.24(0.64+0.52) -57.1% 0.20(0.28+0.60) -64.3%
7820.11: perl grep '[how] to' 0.56(2.66+0.38) 0.29(0.95+0.45) -48.2% 0.23(0.45+0.54) -58.9%
7820.15: perl grep '(e.t[^ ]*|v.ry) rare' 1.02(5.77+0.42) 0.31(1.02+0.54) -69.6% 0.23(0.50+0.54) -77.5%
7820.19: perl grep 'm(ú|u)lt.b(æ|y)te' 0.38(1.57+0.42) 0.27(0.85+0.46) -28.9% 0.21(0.33+0.57) -44.7%
See commit ("perf: add a comparison test of grep regex engines",
2017-04-19) for details on the machine the above test run was executed
on.
Here HEAD~2 is git with PCRE v1 without JIT, HEAD~ is PCRE v1 with
JIT, and HEAD is PCRE v2 (also with JIT). See previous commits of mine
mentioning p7820-grep-engines.sh for more details on the test setup.
For ease of readability, a different run just of HEAD~ (PCRE v1 with
JIT v.s. PCRE v2), again with just the /perl/ tests shown:
[...]
Test HEAD~ HEAD
----------------------------------------------------------------------------------------
7820.3: perl grep 'how.to' 0.21(0.42+0.52) 0.21(0.31+0.58) +0.0%
7820.7: perl grep '^how to' 0.25(0.65+0.50) 0.20(0.31+0.57) -20.0%
7820.11: perl grep '[how] to' 0.30(0.90+0.50) 0.23(0.46+0.53) -23.3%
7820.15: perl grep '(e.t[^ ]*|v.ry) rare' 0.30(1.19+0.38) 0.23(0.51+0.51) -23.3%
7820.19: perl grep 'm(ú|u)lt.b(æ|y)te' 0.27(0.84+0.48) 0.21(0.34+0.57) -22.2%
I.e. the two are either neck-to-neck, but PCRE v2 usually pulls ahead,
when it does it's around 20% faster.
A brief note on thread safety: As noted in pcre2api(3) & pcre2jit(3)
the compiled pattern can be shared between threads, but not some of
the JIT context, however the grep threading support does all pattern &
JIT compilation in separate threads, so this code doesn't need to
concern itself with thread safety.
See commit 63e7e9d8b6 ("git-grep: Learn PCRE", 2011-05-09) for the
initial addition of PCRE v1. This change follows some of the same
patterns it did (and which were discussed on list at the time),
e.g. mocking up types with typedef instead of ifdef-ing them out when
USE_LIBPCRE2 isn't defined. This adds some trivial memory use to the
program, but makes the code look nicer.
1. https://lists.exim.org/lurker/message/20150105.162835.0666407a.en.html
2. https://lists.exim.org/lurker/thread/20170419.172322.833ee099.en.html
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A recent update to t5545-push-options.sh started skipping all the
tests in the script when a web server testing is disabled or
unavailable, not just the ones that require a web server. Non HTTP
tests have been salvaged to always run in this script.
* jc/skip-test-in-the-middle:
t5545: enhance test coverage when no http server is installed
test: allow skipping the remainder
Add a PTHREADS prerequisite which is false when git is compiled with
NO_PTHREADS=YesPlease.
There's lots of custom code that runs when threading isn't available,
but before this prerequisite there was no way to test it.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Change the internal USE_LIBPCRE define, & build options flag to use a
naming convention ending in PCRE1, without changing the long-standing
USE_LIBPCRE Makefile flag which enables this code.
This is for preparation for libpcre2 support where having things like
USE_LIBPCRE and USE_LIBPCRE2 in any more places than we absolutely
need to for backwards compatibility with old Makefile arguments would
be confusing.
In some ways it would be better to change everything that now uses
USE_LIBPCRE to use USE_LIBPCRE1, and to make specifying
USE_LIBPCRE (or --with-pcre) an error. This would impose a one-time
burden on packagers of git to s/USE_LIBPCRE/USE_LIBPCRE1/ in their
build scripts.
However I'd like to leave the door open to making
USE_LIBPCRE=YesPlease eventually mean USE_LIBPCRE2=YesPlease,
i.e. once PCRE v2 is ubiquitous enough that it makes sense to make it
the default.
This code and the USE_LIBPCRE Makefile argument was added in commit
63e7e9d8b6 ("git-grep: Learn PCRE", 2011-05-09). At the time there was
no indication that the PCRE project would release an entirely new &
incompatible API around 3 years later.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Rename the LIBPCRE prerequisite to PCRE. This is for preparation for
libpcre2 support, where having just "LIBPCRE" would be confusing as it
implies v1 of the library.
None of these tests are incompatible between versions 1 & 2 of
libpcre, it's less confusing to give them a more general name to make
it clear that they work on both library versions.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Because TAP output does not like to see the remainder of the test
getting skipped after running one or more tests, bf4b7219
("test-lib.sh: Add check for invalid use of 'skip_all' facility",
2012-09-01) made sure that test_done errors out when this happens.
Instead, loosen the check so that we only pretend that the rest of
the test script did not exist in such a case. We'd lose a bit of
information (i.e. TAP does not notice that we are skipping some
tests), but not very much (i.e. TAP wasn't told how many tests are
skipped anyway).
This will allow inclusion of lib-httpd.sh in the middle of a test,
which will skip the remainder of the test scripts when tests that
involve web server are declined with GIT_TEST_HTTPD=false, for
example.
Acked-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Some platforms have ulong that is smaller than time_t, and our
historical use of ulong for timestamp would mean they cannot
represent some timestamp that the platform allows. Invent a
separate and dedicated timestamp_t (so that we can distingiuish
timestamps and a vanilla ulongs, which along is already a good
move), and then declare uintmax_t is the type to be used as the
timestamp_t.
* js/larger-timestamps:
archive-tar: fix a sparse 'constant too large' warning
use uintmax_t for timestamps
date.c: abort if the system time cannot handle one of our timestamps
timestamp_t: a new data type for timestamps
PRItime: introduce a new "printf format" for timestamps
parse_timestamp(): specify explicitly where we parse timestamps
t0006 & t5000: skip "far in the future" test when time_t is too limited
t0006 & t5000: prepare for 64-bit timestamps
ref-filter: avoid using `unsigned long` for catch-all data type
Attempt to allow us notice "fishy" situation where we fail to
remove the temporary directory used during the test.
* dt/gc-ignore-old-gc-logs:
test-lib: retire $remove_trash variable
test-lib.sh: do not barf under --debug at the end of the test
test-lib: abort when can't remove trash directory
The convention "$remove_trash is set to the trash directory that is
used during the test, so that it will be removed at the end, but
under --debug option we set the varilable to empty string to
preserve the directory" made sense back when it was introduced, as
there was no $TRASH_DIRECTORY variable. These days, since no tests
looks at the variable, it is obscure and even risks that by mistake
the variable gets used for something else (e.g. remove_trash=yes)
and cause us misbehave. Worse yet, remove_trash was not initialized
to an empty string at the beginning, so a stray environment variable
the user has could have affected the logic when "--debug" is in use.
Rewrite the clean-up sequence in test_done helper to explicitly
check the $debug condition and remove the trash directory using
the $TRASH_DIRECTORY variable.
Note that "go to the directory one level above the trash and then
remove it" is kept and this is deliverate; test_at_end_hook_ will
keep running from the expected location, and also some platforms may
not like a directory that is serving as the $cwd of a still-active
process removed.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The original did "does $remove_trash exist? Then go one level above
and remove it". There was no problem under "--debug", where
the variable is left empty, as the first "test -d $remove_trash" would
have said "No, it doesn't".
With the check implemented in the previous step, we'd always get an
error under "--debug".
Noticed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We had two similar bugs in the tests sporadically triggering error
messages during the removal of the trash directory, see commits
bb05510e5 (t5510: run auto-gc in the foreground, 2016-05-01) and
ef09036cf (t6500: wait for detached auto gc at the end of the test
script, 2017-04-13). The test script succeeded nonetheless, because
these errors are ignored during housekeeping in 'test_done'.
However, such an error is a sign that something is fishy in the test
script. Print an error message and abort the test script when the
trash directory can't be removed successfully or is already removed,
because that's unexpected and we would prefer somebody notice and
figure out why.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Git's source code refers to timestamps as unsigned long, which is
ill-defined, as there is no guarantee about the number of bits that
data type has.
In preparation of switching to another data type that is large enough
to hold "far in the future" dates, we need to prepare the t0006-date.sh
script for the case where we *still* cannot format those dates if the
system library uses 32-bit time_t.
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Git's source code refers to timestamps as unsigned longs. On 32-bit
platforms, as well as on Windows, unsigned long is not large enough to
capture dates that are "absurdly far in the future".
It is perfectly valid by the C standard, of course, for the `long` data
type to refer to 32-bit integers. That is why the `time_t` data type
exists: so that it can be 64-bit even if `long` is 32-bit. Git's source
code simply uses an incorrect data type for timestamps, is all.
The earlier quick fix 6b9c38e14c (t0006: skip "far in the future" test
when unsigned long is not long enough, 2016-07-11) papered over this
issue simply by skipping the respective test cases on platforms where
they would fail due to the data type in use.
This quick fix, however, tests for *long* to be 64-bit or not. What we
need, though, is a test that says whether *whatever data type we use for
timestamps* is 64-bit or not.
The same quick fix was used to handle the similar problem where Git's
source code uses `unsigned long` to represent size, instead of `size_t`,
conflating the two issues.
So let's just add another prerequisite to test specifically whether
timestamps are represented by a 64-bit data type or not. Later, after we
switch to a larger data type, we can flip that prerequisite to test
`time_t` instead of `long`.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We found a few run-away here documents that are started with an
end-of-here-doc marker that is incorrectly spelled, e.g.
git some command >actual &&
cat <<EOF >expect
...
EOF &&
test_cmp expect actual
which ends up slurping the entire remainder of the script as if it
were the data. Often the command that gets misused like this exits
without failure (e.g. "cat" in the above example), which makes the
command appear to work, without ever executing the remainder of the
test.
Piggy-back on the test that catches &&-chain breakage to detect this
case as well.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The 'debug' test helper is supposed to facilitate debugging by running
a command of the test suite under gdb. Unfortunately, its usefulness
is severely limited, because that gdb session is not interactive,
since the test's, and thus gdb's standard input is redirected from
/dev/null (for a good reason, see 781f76b15 (test-lib: redirect stdin
of tests, 2011-12-15)).
Redirect gdb's standard file descriptors from/to the test
environment's stdin, stdout and stderr in the 'debug' helper, thus
creating an interactive gdb session (even in non-verbose mode), which
is much, much more useful.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Eric Wong reported that while FreeBSD has a /usr/bin/unzip, it uses
different semantics from those that are needed by Git's tests: When
passing the -a option to Info-Zip, it heeds the text attribute of the
.zip file's central directory, while FreeBSD's unzip ignores that
attribute.
The common work-around is to install Info-Zip on FreeBSD, into
/usr/local/bin/.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Tested-by: Eric Wong <e@80x24.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Update to the test framework made in 2.9 timeframe broke running
the tests under valgrind, which has been fixed.
* nd/test-helpers:
valgrind: support test helpers
Update to the test framework made in 2.9 timeframe broke running
the tests under valgrind, which has been fixed.
* nd/test-helpers:
valgrind: support test helpers
The Travis CI configuration we ship ran the tests with --verbose
option but this risks non-TAP output that happens to be "ok" to be
misinterpreted as TAP signalling a test that passed. This resulted
in unnecessary failure. This has been corrected by introducing a
new mode to run our tests in the test harness to send the verbose
output separately to the log file.
* jk/tap-verbose-fix:
test-lib: bail out when "-v" used under "prove"
travis: use --verbose-log test option
test-lib: add --verbose-log option
test-lib: handle TEST_OUTPUT_DIRECTORY with spaces
Tests run with --valgrind call git commands through a wrapper script
that invokes valgrind on them. This script (valgrind.sh) is in turn
invoked through symlinks created for each command in t/valgrind/bin/.
Since e6e7530d (test helpers: move test-* to t/helper/ subdirectory)
these symlinks have been broken for test helpers -- they point to the
old locations in the root of the build directory. Fix that by teaching
the code for creating the links about the new location of the binaries,
and do the same in the wrapper script to allow it to find its payload.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The Travis CI configuration we ship ran the tests with --verbose
option but this risks non-TAP output that happens to be "ok" to be
misinterpreted as TAP signalling a test that passed. This resulted
in unnecessary failure. This has been corrected by introducing a
new mode to run our tests in the test harness to send the verbose
output separately to the log file.
* jk/tap-verbose-fix:
test-lib: bail out when "-v" used under "prove"
travis: use --verbose-log test option
test-lib: add --verbose-log option
test-lib: handle TEST_OUTPUT_DIRECTORY with spaces
When there is a TAP harness consuming the output of our test
scripts, the "--verbose" breaks the output by mingling
test command output with TAP. Because the TAP::Harness
module used by "prove" is fairly lenient, this _usually_
works, but it violates the spec, and things get very
confusing if the commands happen to output a line that looks
like TAP (e.g., the word "ok" on its own line).
Let's detect this situation and complain. Just calling
error() isn't great, though; prove will tell us that the
script failed, but the message doesn't make it through to
the user. Instead, we can use the special TAP signal "Bail
out!". This not only shows the message to the user, but
instructs the harness to stop running the tests entirely.
This is exactly what we want here, as the problem is in the
command-line options, and every test script would produce
the same error.
The result looks like this (the first "Bailout called" line
is in red if prove uses color on your terminal):
$ make GIT_TEST_OPTS='--verbose --tee'
rm -f -r 'test-results'
*** prove ***
Bailout called. Further testing stopped: verbose mode forbidden under TAP harness; try --verbose-log
FAILED--Further testing stopped: verbose mode forbidden under TAP harness; try --verbose-log
Makefile:39: recipe for target 'prove' failed
make: *** [prove] Error 255
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "--verbose" option redirects output from arbitrary
test commands to stdout. This is useful for examining the
output manually, like:
./t5547-push-quarantine.sh -v | less
But it also means that the output is intermingled with the
TAP directives, which can confuse a TAP parser like "prove".
This has always been a potential problem, but became an
issue recently when one test happened to output the word
"ok" on a line by itself, which prove interprets as a test
success:
$ prove t5547-push-quarantine.sh :: -v
t5547-push-quarantine.sh .. 1/? To dest.git
* [new branch] HEAD -> master
To dest.git
! [remote rejected] reject -> reject (pre-receive hook declined)
error: failed to push some refs to 'dest.git'
fatal: git cat-file d08c8eba97f4e683ece08654c7c8d2ba0c03b129: bad file
t5547-push-quarantine.sh .. Failed -1/4 subtests
Test Summary Report
-------------------
t5547-push-quarantine.sh (Wstat: 0 Tests: 5 Failed: 0)
Parse errors: Tests out of sequence. Found (2) but expected (3)
Tests out of sequence. Found (3) but expected (4)
Tests out of sequence. Found (4) but expected (5)
Bad plan. You planned 4 tests but ran 5.
Files=1, Tests=5, 0 wallclock secs ( 0.01 usr + 0.01 sys = 0.02 CPU)
Result: FAIL
One answer is "if it hurts, don't do it", but that's not
quite the whole story. The Travis tests use "--verbose
--tee" so that they can get the benefit of prove's parallel
options, along with a verbose log in case there is a
failure. We just need the verbose output to go to the log,
but keep stdout clean.
Getting this right turns out to be surprisingly difficult.
Here's the progression of alternatives I considered:
1. Add an option to write verbose output to stderr. This is
hard to capture, though, because we want each test to
have its own log (because they're all run in parallel
and the jumbled output would be useless).
2. Add an option to write verbose output to a file in
test-results. This works, but the log is missing all of
the non-verbose output, which gives context.
3. Like (2), but teach say_color() to additionally output
to the log. This mostly works, but misses any output
that happens outside of the say() functions (which isn't
a lot, but is a potential maintenance headache).
4. Like (2), but make the log file the same as the "--tee"
file. That almost works, but now we have two processes
opening the same file. That gives us two separate
descriptors, each with their own idea of the current
position. They'll each start writing at offset 0, and
overwrite each other's data.
5. Like (4), but in each case open the file for appending.
That atomically positions each write at the end of the
file.
It's possible we may still get sheared writes between
the two processes, but this is already the case when
writing to stdout. It's not a problem in practice
because the test harness generally waits for snippets to
finish before writing the TAP output.
We can ignore buffering issues with tee, because POSIX
mandates that it does not buffer. Likewise, POSIX
specifies "tee -a", so it should be available
everywhere.
This patch implements option (5), which seems to work well
in practice.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We are careful in test_done to handle a results directory
with a space in it, but the "--tee" code path does not.
Doing:
export TEST_OUTPUT_DIRECTORY='/tmp/path with spaces'
./t000-init.sh --tee
results in errors. Let's consistently double-quote our path
variables so that this works.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
JGit can show a fake ref "capabilities^{}" to "git fetch" when it
does not advertise any refs, but "git fetch" was not prepared to
see such an advertisement. When the other side disconnects without
giving any ref advertisement, we used to say "there may not be a
repository at that URL", but we may have seen other advertisement
like "shallow" and ".have" in which case we definitely know that a
repository is there. The code to detect this case has also been
updated.
* jt/accept-capability-advertisement-when-fetching-from-void:
connect: advertized capability is not a ref
connect: tighten check for unexpected early hang up
tests: move test_lazy_prereq JGIT to test-lib.sh
Update a few tests that used to use GIT_CURL_VERBOSE to use the
newer GIT_TRACE_CURL.
* ep/use-git-trace-curl-in-tests:
t5551-http-fetch-smart.sh: use the GIT_TRACE_CURL environment var
t5550-http-fetch-dumb.sh: use the GIT_TRACE_CURL environment var
test-lib.sh: preserve GIT_TRACE_CURL from the environment
t5541-http-push-smart.sh: use the GIT_TRACE_CURL environment var
Update a few tests that used to use GIT_CURL_VERBOSE to use the
newer GIT_TRACE_CURL.
* ep/use-git-trace-curl-in-tests:
t5551-http-fetch-smart.sh: use the GIT_TRACE_CURL environment var
t5550-http-fetch-dumb.sh: use the GIT_TRACE_CURL environment var
test-lib.sh: preserve GIT_TRACE_CURL from the environment
t5541-http-push-smart.sh: use the GIT_TRACE_CURL environment var
This enables JGIT to be used as a prereq in invocations of
test_expect_success (and other functions) in other test scripts.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Turning on this variable can be useful when debugging http
tests. It can break a few tests in t5541 if not set
to an absolute path but it is not a variable
that the user is likely to have enabled accidentally.
Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Each test run generates a "count" file in t/test-results
that stores the number of successful, failed, etc tests.
If you run "t1234-foo.sh", that file is named as
"t/test-results/t1234-foo-$$.count"
The addition of the PID there is serving no purpose, and
makes analysis of the count files harder.
The presence of the PID dates back to 2d84e9f (Modify
test-lib.sh to output stats to t/test-results/*,
2008-06-08), but no reasoning is given there. Looking at the
current code, we can see that other files we write to
test-results (like *.exit and *.out) do _not_ have the PID
included. So the presence of the PID does not meaningfully
allow one to store the results from multiple runs anyway.
Moreover, anybody wishing to read the *.count files to
aggregate results has to deal with the presence of multiple
files for a given test (and figure out which one is the most
recent based on their timestamps!). The only consumer of
these files is the aggregate.sh script, which arguably gets
this wrong. If a test is run multiple times, its counts will
appear multiple times in the total (I say arguably only
because the desired semantics aren't documented anywhere,
but I have trouble seeing how this behavior could be
useful).
So let's just drop the PID, which fixes aggregate.sh, and
will make new features based around the count files easier
to write.
Note that since the count-file may already exist (when
re-running a test), we also switch the "cat" from appending
to truncating. The use of append here was pointless in the
first place, as we expected to always write to a unique file.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Build clean-up.
* nd/test-helpers:
t/test-lib.sh: fix running tests with --valgrind
Makefile: use VCSSVN_LIB to refer to svn library
Makefile: drop extra dependencies for test helpers
"git add -N dir/file && git write-tree" produced an incorrect tree
when there are other paths in the same directory that sorts after
"file".
* nd/cache-tree-ita:
cache-tree: do not generate empty trees as a result of all i-t-a subentries
cache-tree.c: fix i-t-a entry skipping directory updates sometimes
test-lib.sh: introduce and use $EMPTY_BLOB
test-lib.sh: introduce and use $EMPTY_TREE
"git add -N dir/file && git write-tree" produced an incorrect tree
when there are other paths in the same directory that sorts after
"file".
* nd/cache-tree-ita:
cache-tree: do not generate empty trees as a result of all i-t-a subentries
cache-tree.c: fix i-t-a entry skipping directory updates sometimes
test-lib.sh: introduce and use $EMPTY_BLOB
test-lib.sh: introduce and use $EMPTY_TREE
Build clean-up.
* nd/test-helpers:
t/test-lib.sh: fix running tests with --valgrind
Makefile: use VCSSVN_LIB to refer to svn library
Makefile: drop extra dependencies for test helpers
Similar to $EMPTY_TREE this makes it easier to recognize this special
SHA-1 and change hash later.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This is a special SHA1. Let's keep it at one place, easier to replace
later when the hash change comes, easier to recognize.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Skip tests that are unrunnable on platforms without 64-bit long
to avoid unnecessary test failures.
* jk/tzoffset-fix:
t0006: skip "far in the future" test when unsigned long is not long enough
Git's source code refers to timestamps as unsigned longs. On 32-bit
platforms, as well as on Windows, unsigned long is not large enough
to capture dates that are "absurdly far in the future".
While we can fix this issue properly by replacing unsigned long with
a larger type, we want to be a bit more conservative and just skip
those tests on the maint track.
Signed-off-by: Jeff King <peff@peff.net>
Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We forgot to adjust this code path after moving the test helpers to
t/helper/.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Acked-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Running tests with '-x' option to trace the individual command
executions is a useful way to debug test scripts, but some tests
that capture the standard error stream and check what the command
said can be broken with the trace output mixed in. When running
our tests under "bash", however, we can redirect the trace output
to another file descriptor to keep the standard error of programs
being tested intact.
* jk/test-send-sh-x-trace-elsewhere:
test-lib: set BASH_XTRACEFD automatically
Running tests with '-x' option to trace the individual command
executions is a useful way to debug test scripts, but some tests
that capture the standard error stream and check what the command
said can be broken with the trace output mixed in. When running
our tests under "bash", however, we can redirect the trace output
to another file descriptor to keep the standard error of programs
being tested intact.
* jk/test-send-sh-x-trace-elsewhere:
test-lib: set BASH_XTRACEFD automatically
Passing "-x" to a test script enables the shell's "set -x"
tracing, which can help with tracking down the command that
is causing a failure. Unfortunately, it can also _cause_
failures in some tests that redirect the stderr of a shell
function. Inside the function the shell continues to
respect "set -x", and the trace output is collected along
with whatever stderr is generated normally by the function.
You can see an example of this by running:
./t0040-parse-options.sh -x -i
which will fail immediately in the first test, as it
expects:
test_must_fail some-cmd 2>output.err
to leave output.err empty (but with "-x" it has our trace
output).
Unfortunately there isn't a portable or scalable solution to
this. We could teach test_must_fail to disable "set -x", but
that doesn't help any of the other functions or subshells.
However, we can work around it by pointing the "set -x"
output to our descriptor 4, which always points to the
original stderr of the test script. Unfortunately this only
works for bash, but it's better than nothing (and other
shells will just ignore the BASH_XTRACEFD variable).
The patch itself is a simple one-liner, but note the caveats
in the accompanying comments.
Automatic tests for our "-x" option may be a bit too meta
(and a pain, because they are bash-specific), but I did
confirm that it works correctly both with regular "-x" and
with "--verbose-only=1". This works because the latter flips
"set -x" off and on for particular tests (if it didn't, we
would get tracing for all tests, as going to descriptor 4
effectively circumvents the verbose flag).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
To get the 'value' from '--option=value', test-lib.sh parses said
option running 'expr' with a regexp. This involves a subshell, an
external process, and a lot of non-alphanumeric characters in the
regexp.
Use a much simpler POSIX-defined shell parameter expansion instead to
do the same.
Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This keeps top dir a bit less crowded. And because these programs are
for testing purposes, it makes sense that they stay somewhere in t/
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Test scripts have been updated to remove assumptions that are not
portable between Git for POSIX and Git for Windows, or to skip ones
with expectations that are not satisfiable on Git for Windows.
* js/mingw-tests: (21 commits)
gitignore: ignore generated test-fake-ssh executable
mingw: do not bother to test funny file names
mingw: skip a test in t9130 that cannot pass on Windows
mingw: handle the missing POSIXPERM prereq in t9124
mingw: avoid illegal filename in t9118
mingw: mark t9100's test cases with appropriate prereqs
t0008: avoid absolute path
mingw: work around pwd issues in the tests
mingw: fix t9700's assumption about directory separators
mingw: skip test in t1508 that fails due to path conversion
tests: turn off git-daemon tests if FIFOs are not available
mingw: disable mkfifo-based tests
mingw: accomodate t0060-path-utils for MSYS2
mingw: fix t5601-clone.sh
mingw: let lstat() fail with errno == ENOTDIR when appropriate
mingw: try to delete target directory before renaming
mingw: prepare the TMPDIR environment variable for shell scripts
mingw: factor out Windows specific environment setup
Git.pm: stop assuming that absolute paths start with a slash
mingw: do not trust MSYS2's MinGW gettext.sh
...
The emulated "yes" command used in our test scripts has been
tweaked not to spend too much time generating unnecessary output
that is not used, to help those who test on Windows where it would
not stop until it fills the pipe buffer due to lack of SIGPIPE.
* js/test-lib-windows-emulated-yes:
test-lib: limit the output of the yes utility
The emulated "yes" command used in our test scripts has been
tweaked not to spend too much time generating unnecessary output
that is not used, to help those who test on Windows where it would
not stop until it fills the pipe buffer due to lack of SIGPIPE.
* js/test-lib-windows-emulated-yes:
test-lib: limit the output of the yes utility
The description for SANITY prerequisite the test suite uses has
been clarified both in the comment and in the implementation.
* jk/sanity:
test-lib: clarify and tighten SANITY
On Windows, there is no SIGPIPE. A consequence of this is that the
upstream process of a pipe does not notice the death of the downstream
process until the pipe buffer is full and writing more data returns an
error. This behavior is the reason for an annoying delay during the
execution of t7610-mergetool.sh: There are a number of test cases where
'yes' is invoked upstream. Since the utility is basically an endless
loop it runs, on Windows, until the pipe buffer is full. This does take
a few seconds.
The test suite has its own implementation of 'yes'. Modify it to produce
only a limited amount of output that is sufficient for the test suite.
The amount chosen should be sufficiently high for any test case, assuming
that future test cases will not exaggerate their demands of input from
an upstream 'yes' invocation.
[j6t: commit message]
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The description for SANITY prerequisite the test suite uses has
been clarified both in the comment and in the implementation.
* jk/sanity:
test-lib: clarify and tighten SANITY
MSYS2 (the POSIX emulation layer used by Git for Windows' Bash) actually
has a working mkfifo. The only problem is that it is only emulating
named pipes through the MSYS2 runtime; The Win32 API has no idea about
named pipes, hence the Git executable cannot access those pipes either.
The symptom is that Git fails with a '<name>: No such file or directory'
because MSYS2 emulates named pipes through special-crafted '.lnk' files.
The solution is to tell the test suite explicitly that we cannot use
named pipes when we want to test on Windows.
This lets t4056-diff-order.sh, t9010-svn-fe.sh and t9300-fast-import.sh
pass.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
f400e51c (test-lib.sh: set prerequisite SANITY by testing what we
really need, 2015-01-27) improved the way SANITY prerequisite was
determined, but made the resulting code (incorrectly) imply that
SANITY is all about effects of permission bits of the containing
directory has on the files contained in it by the comment it added,
its log message and the actual tests.
State what SANITY is about more clearly in the comment, and test
that a file whose permission bits says should be unreadble truly
cannot be read.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Running tests with the "-x" option to make them verbose had some
unpleasant interactions with other features of the test suite.
* jk/test-with-x:
test-lib: disable trace when test is not verbose
test-lib: turn off "-x" tracing during chain-lint check
The "-x" test-script option turns on the shell's "-x"
tracing, which can help show why a particular test is
failing. Unfortunately, this can create false negatives in
some tests if they invoke a shell function with its stderr
redirected. t5512.10 is such a test, as it does:
test_must_fail git ls-remote refs*master >actual 2>&1 &&
test_cmp exp actual
The "actual" file gets the "-x" trace for the test_must_fail
function, which prevents it from matching the expected
output.
There's no way to avoid this without managing the
trace flag inside each sub-function, which isn't really a
workable solution. But unless you specifically care about
t5512.10, we can work around it by enabling tracing only for
the specific tests we want.
You can already do:
./t5512-ls-remote.sh -x --verbose-only=16
to see the trace only for a specific test. But that doesn't
_disable_ the tracing in the other tests; it just sends it
to /dev/null. However, there's no point in generating a
trace that the user won't see, so we can simply disable
tracing whenever it doesn't have a matching verbose flag.
The normal case of just "./t5512-ls-remote.sh -x" stays the
same, as "-x" already implies "--verbose" (and
"--verbose-only" overrides "--verbose", which is why this
works at all). And for our test, we need only check
$verbose, as maybe_setup_verbose will have already
set that flag based on the $verbose_only list).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Now that GIT_TEST_CHAIN_LINT is on by default, running:
./t0000-basic.sh -x --verbose-only=1
starts with:
expecting success:
find .git/objects -type f -print >should-be-empty &&
test_line_count = 0 should-be-empty
+ exit 117
error: last command exited with $?=117
+ find .git/objects -type f -print
+ test_line_count = 0 should-be-empty
+ test 3 != 3
+ wc -l
+ test 0 = 0
ok 1 - .git/objects should be empty after git init in an empty repo
This is confusing, as the "exit 117" line and the error line
(which is printed in red, no less!) are not part of the test
at all, but are rather in the separate chain-lint test_eval.
Let's unset the "trace" variable when eval-ing the chain
lint check, which avoids this.
Note that we cannot just do a one-shot variable like:
trace= test_eval ...
as the behavior of one-shot variables for function calls
is not portable.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
An ancient test framework enhancement to allow color was not
entirely correct; this makes it work even when tput needs to read
from the ~/.terminfo under the user's real HOME directory.
* rh/test-color-avoid-terminfo-in-original-home:
test-lib.sh: fix color support when tput needs ~/.terminfo
Revert "test-lib.sh: do tests for color support after changing HOME"
An ancient test framework enhancement to allow color was not
entirely correct; this makes it work even when tput needs to read
from the ~/.terminfo under the user's real HOME directory.
* rh/test-color-avoid-terminfo-in-original-home:
test-lib.sh: fix color support when tput needs ~/.terminfo
Revert "test-lib.sh: do tests for color support after changing HOME"
If tput needs ~/.terminfo for the current $TERM, then tput will
succeed before HOME is changed to $TRASH_DIRECTORY (causing color to
be set to 't') but fail afterward.
One possible way to fix this is to treat HOME like TERM: back up the
original value and temporarily restore it before say_color() runs
tput.
Instead, pre-compute and save the color control sequences before
changing either TERM or HOME. Use the saved control sequences in
say_color() rather than call tput each time. This avoids the need to
back up and restore the TERM and HOME variables, and it avoids the
overhead of a subshell and two invocations of tput per call to
say_color().
Signed-off-by: Richard Hansen <rhansen@bbn.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This reverts commit 102fc80d32.
There are two issues with that commit:
* It is buggy. In pseudocode, it is doing:
color is set || TERM != dumb && color works && color=t
when it should be doing:
color is set || { TERM != dumb && color works && color=t }
* It unnecessarily disables color when tput needs to read
~/.terminfo to get the control sequences.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Developer support to automatically detect broken &&-chain in the
test scripts is now turned on by default.
* jk/test-chain-lint:
test-lib: turn on GIT_TEST_CHAIN_LINT by default
t7502-commit.sh: fix a broken and-chain
Now that the feature has had time to prove itself, and any
topics in flight have had a chance to clean up any broken
&&-chains, we can flip this feature on by default. This
makes one less thing submitters need to configure or check
before sending their patches.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
People often forget to chain the commands in their test together
with &&, leaving a failure from an earlier command in the test go
unnoticed. The new GIT_TEST_CHAIN_LINT mechanism allows you to
catch such a mistake more easily.
* jk/test-chain-lint: (36 commits)
t9001: drop save_confirm helper
t0020: use test_* helpers instead of hand-rolled messages
t: simplify loop exit-code status variables
t: fix some trivial cases of ignored exit codes in loops
t7701: fix ignored exit code inside loop
t3305: fix ignored exit code inside loop
t0020: fix ignored exit code inside loops
perf-lib: fix ignored exit code inside loop
t6039: fix broken && chain
t9158, t9161: fix broken &&-chain in git-svn tests
t9104: fix test for following larger parents
t4104: drop hand-rolled error reporting
t0005: fix broken &&-chains
t7004: fix embedded single-quotes
t0050: appease --chain-lint
t9001: use test_when_finished
t4117: use modern test_* helpers
t6034: use modern test_* helpers
t1301: use modern test_* helpers
t0020: use modern test_* helpers
...
It's easy to miss an "&&"-chain in a test script, like:
test_expect_success 'check something important' '
cmd1 &&
cmd2
cmd3
'
The test harness will notice if cmd3 fails, but a failure of
cmd1 or cmd2 will go unnoticed, as their exit status is lost
after cmd3 runs.
The toy example above is easy to spot because the "cmds" are
all the same length, but real code is much more complicated.
It's also difficult to detect these situations by statically
analyzing the shell code with regexps (like the
check-non-portable-shell script does); there's too much
context required to know whether a &&-chain is appropriate
on a given line or not.
This patch instead lets the shell check each test by
sticking a command with a specific and unusual return code
at the top of each test, like:
(exit 117) &&
cmd1 &&
cmd2
cmd3
In a well-formed test, the non-zero exit from the first
command prevents any of the rest from being run, and the
test's exit code is 117. In a bad test (like the one above),
the 117 is lost, and cmd3 is run.
When we encounter a failure of this check, we abort the test
script entirely. For one thing, we have no clue which subset
of the commands in the test snippet were actually run.
Running further tests would be pointless, because we're now
in an unknown state. And two, this is not a "test failure"
in the traditional sense. The test script is buggy, not the
code it is testing. We should be able to fix these problems
in the script once, and not have them come back later as a
regression in git's code.
After checking a test snippet for --chain-lint, we do still
run the test itself. We could actually have a pure-lint
mode which just checks each test, but there are a few
reasons not to. One, because the tests are executing
arbitrary code, which could impact the later environment
(e.g., that could impact which set of tests we run at all).
And two, because a pure-lint mode would still be expensive
to run, because a significant amount of code runs outside of
the test_expect_* blocks. Instead, this option is designed
to be used as part of a normal test suite run, where it adds
very little overhead.
Turning on this option detects quite a few problems in
existing tests, which will be fixed in subsequent patches.
However, there are a number of places it cannot reach:
- it cannot find a failure to break out of loops on error,
like:
cmd1 &&
for i in a b c; do
cmd2 $i
done &&
cmd3
which will not notice failures of "cmd2 a" or "cmd b"
- it cannot find a missing &&-chain inside a block or
subfunction, like:
foo () {
cmd1
cmd2
}
foo &&
bar
which will not notice a failure of cmd1.
- it only checks tests that you run; every platform will
have some tests skipped due to missing prequisites,
so it's impossible to say from one run that the test
suite is free of broken &&-chains. However, all tests get
run by _somebody_, so eventually we will notice problems.
- it does not operate on test_when_finished or prerequisite
blocks. It could, but these tends to be much shorter and
less of a problem, so I punted on them in this patch.
This patch was inspired by an earlier patch by Jonathan
Nieder:
http://article.gmane.org/gmane.comp.version-control.git/235913
This implementation and all bugs are mine.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We use this to test http pushing with a restricted
commandline. Other scripts (like t5551, which does http
fetching) will want to use it, too.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If you run a test script like:
GIT_TRACE=1 ./t0061-run-command.sh
you may get test failures, because some tests capture and
check the stderr output from git commands (and with
GIT_TRACE set to 1, the trace output will be included
there).
When we see GIT_TRACE set like this, we print a warning to
the user. However, we can do even better than that by just
pointing it to descriptor 4, which all tests leave connected
to the test script's stderr. That's likely what the user
intended (and any scripts that do want to see GIT_TRACE
output will set GIT_TRACE themselves).
Not only does this avoid false negatives in the tests, but
it means the user will actually see trace output for git
calls that redirect their stderr (whereas before, it was
sometimes confusingly buried in a file).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Right now if a test script receives SIGINT (e.g., because a
test was hanging and the user hit ^C), the shell exits
immediately. This can be annoying if the test script did any
global setup, like starting apache or git-daemon, as it will
not have an opportunity to clean up after itself. A
subsequent run of the test won't be able to start its own
daemon, and will either fail or skip the tests.
Instead, let's trap SIGINT to make sure we do a clean
shutdown, and just chain it to a normal exit (which will
trigger any cleanup).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The tests that wanted to see that file becomes unreadable after
running "chmod a-r file", and the tests that wanted to make sure it
is not run as root, we used "can we write into the / directory?" as
a cheap substitute, but on some platforms that is not a good
heuristics. The tests and their prerequisites have been updated to
check what they really require.
* jk/sanity:
test-lib.sh: set prerequisite SANITY by testing what we really need
tests: correct misuses of POSIXPERM
t/lib-httpd: switch SANITY check for NOT_ROOT
The tests that wanted to see that file becomes unreadable after
running "chmod a-r file", and the tests that wanted to make sure it
is not run as root, we used "can we write into the / directory?" as
a cheap substitute, but on some platforms that is not a good
heuristics. The tests and their prerequisites have been updated to
check what they really require.
* jk/sanity:
test-lib.sh: set prerequisite SANITY by testing what we really need
tests: correct misuses of POSIXPERM
t/lib-httpd: switch SANITY check for NOT_ROOT
What we wanted out of the SANITY precondition is that the filesystem
behaves sensibly with permission bits settings.
- You should not be able to remove a file in a read-only directory,
- You should not be able to tell if a file in a directory exists if
the directory lacks read or execute permission bits.
We used to cheat by approximating that condition with "is the /
writable?" test and/or "are we running as root?" test. Neither test
is sufficient or appropriate in environments like Cygwin.
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The SANITY prerequisite is really about whether the
filesystem will respect the permissions we set, and being
root is only one part of that. But the httpd tests really
just care about not being root, as they are trying to avoid
weirdness in apache (see a1a3011 for details).
Let's switch out SANITY for a new NOT_ROOT prerequisite,
which will let us tweak SANITY more freely.
We implement NOT_ROOT by checking `id -u`, which is in POSIX
and seems to be available even on MSYS. Note that we cannot
just call this "ROOT" and ask for "!ROOT". The possible
outcomes are:
1. we know we are root
2. we know we are not root
3. we could not tell, because `id` was not available
We should conservatively treat (3) as "does not have the
prerequisite", which means that a naive negation would not
work.
Helped-by: Kyle J. McKay <mackyle@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If ncurses needs ~/.terminfo for the current $TERM, then tput will
succeed before changing HOME to $TRASH_DIRECTORY but fail afterward.
Move the tests that determine whether there is color support after
changing HOME so that color=t is set if and only if tput would succeed
when say_color() is run.
Note that color=t is now set after --no-color is processed, so the
condition to set color=t has changed: it is now set only if
color has not already been set to the empty string by --no-color.
This disables color support for those that need ~/.terminfo for
their TERM, but it's better than filling the screen with:
tput: unknown terminal "custom-terminal-name-here"
An alternative would be to symlink or copy the user's terminfo
database into $TRASH_DIRECTORY, but this is tricky due to the lack of
a standard name for the terminfo database (for example, instead of a
~/.terminfo directory, NetBSD uses a ~/.terminfo.cdb database file).
Signed-off-by: Richard Hansen <rhansen@bbn.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The point of disallowing ".git" in the index is that we
would never want to accidentally overwrite files in the
repository directory. But this means we need to respect the
filesystem's idea of when two paths are equal. The prior
commit added a helper to make such a comparison for HFS+;
let's use it in verify_path.
We make this check optional for two reasons:
1. It restricts the set of allowable filenames, which is
unnecessary for people who are not on HFS+. In practice
this probably doesn't matter, though, as the restricted
names are rather obscure and almost certainly would
never come up in practice.
2. It has a minor performance penalty for every path we
insert into the index.
This patch ties the check to the core.protectHFS config
option. Though this is expected to be most useful on OS X,
we allow it to be set everywhere, as HFS+ may be mounted on
other platforms. The variable does default to on for OS X,
though.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When git is compiled with "-fsanitize=address" (using clang
or gcc >= 4.8), all invocations of git will check for buffer
overflows. This is similar to running with valgrind, except
that it is more thorough (because of the compiler support,
function-local buffers can be checked, too) and runs much
faster (making it much less painful to run the whole test
suite with the checks turned on).
Unlike valgrind, the magic happens at compile-time, so we
don't need the same infrastructure in the test suite that we
did to support --valgrind. But there are two things we can
help with:
1. On some platforms, the leak-detector is on by default,
and causes every invocation of "git init" (and thus
every test script) to fail. Since running git with
the leak detector is pointless, let's shut it off
automatically in the tests, unless the user has already
configured it.
2. When apache runs a CGI, it clears the environment of
unknown variables. This means that the $ASAN_OPTIONS
config doesn't make it to git-http-backend, and it
dies due to the leak detector. Let's mark the variable
as OK for apache to pass.
With these two changes, running
make CC=clang CFLAGS=-fsanitize=address test
works out of the box.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Usually running a test under "-v" makes it clear which
command is failing. However, sometimes it can be useful to
also see a complete trace of the shell commands being run in
the test. You can do so without any support from the test
suite by running "sh -x tXXXX-foo.sh". However, this
produces quite a large bit of output, as we see a trace of
the entire test suite.
This patch instead introduces a "-x" option to the test
scripts (i.e., "./tXXXX-foo.sh -x"). When enabled, this
turns on "set -x" only for the tests themselves. This can
still be a bit verbose, but should keep things to a more
manageable level. You can even use "--verbose-only" to see
the trace only for a specific test.
The implementation is a little invasive. We turn on the "set
-x" inside the "eval" of the test code. This lets the eval
itself avoid being reported in the trace (which would be
long, and redundant with the verbose listing we already
showed). And then after the eval runs, we do some trickery
with stderr to avoid showing the "set +x" to the user.
We also show traces for test_cleanup functions (since they
can impact the test outcome, too). However, we do avoid
running the noop ":" cleanup (the default if the test does
not use test_cleanup at all), as it creates unnecessary
noise in the "set -x" output.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Allow "git push" request to be signed, so that it can be verified and
audited, using the GPG signature of the person who pushed, that the
tips of branches at a public repository really point the commits
the pusher wanted to, without having to "trust" the server.
* jc/push-cert: (24 commits)
receive-pack::hmac_sha1(): copy the entire SHA-1 hash out
signed push: allow stale nonce in stateless mode
signed push: teach smart-HTTP to pass "git push --signed" around
signed push: fortify against replay attacks
signed push: add "pushee" header to push certificate
signed push: remove duplicated protocol info
send-pack: send feature request on push-cert packet
receive-pack: GPG-validate push certificates
push: the beginning of "git push --signed"
pack-protocol doc: typofix for PKT-LINE
gpg-interface: move parse_signature() to where it should be
gpg-interface: move parse_gpg_output() to where it should be
send-pack: clarify that cmds_sent is a boolean
send-pack: refactor inspecting and resetting status and sending commands
send-pack: rename "new_refs" to "need_pack_data"
receive-pack: factor out capability string generation
send-pack: factor out capability string generation
send-pack: always send capabilities
send-pack: refactor decision to send update per ref
send-pack: move REF_STATUS_REJECT_NODELETE logic a bit higher
...
* tb/crlf-tests:
MinGW: update tests to handle a native eol of crlf
Makefile: propagate NATIVE_CRLF to C
t0027: Tests for core.eol=native, eol=lf, eol=crlf
The "--signed" option received by "git push" is first passed to the
transport layer, which the native transport directly uses to notice
that a push certificate needs to be sent. When the transport-helper
is involved, however, the option needs to be told to the helper with
set_helper_option(), and the helper needs to take necessary action.
For the smart-HTTP helper, the "necessary action" involves spawning
the "git send-pack" subprocess with the "--signed" option.
Once the above all gets wired in, the smart-HTTP transport now can
use the push certificate mechanism to authenticate its pushes.
Add a test that is modeled after tests for the native transport in
t5534-push-signed.sh to t5541-http-push-smart.sh. Update the test
Apache configuration to pass GNUPGHOME environment variable through.
As PassEnv would trigger warnings for an environment variable that
is not set, export it from test-lib.sh set to a harmless value when
GnuPG is not being used in the tests.
Note that the added test is deliberately loose and does not check
the nonce in this step. This is because the stateless RPC mode is
inevitably flaky and a nonce that comes back in the actual push
processing is one issued by a different process; if the two
interactions with the server crossed a second boundary, the nonces
will not match and such a check will fail. A later patch in the
series will work around this shortcoming.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We have been using NOT_{MINGW,CYGWIN} test prerequisites long
before Peff invented support for negated prerequisites e.g. !MINGW
and we still add more uses of the former. Convert them to the
latter to avoid confusion.
* jc/not-mingw-cygwin:
test prerequisites: enumerate with commas
test prerequisites: eradicate NOT_FOO
Some of the tests were written with the assumption that the native
eol would always be lf. After defining NATIVE_CRLF on MinGW, these
tests began failing. This change will update the tests to also
handle a native eol of crlf.
Signed-off-by: Brice Lambson <bricelam@live.com>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
