summaryrefslogtreecommitdiff
path: root/t/test-lib.sh
AgeCommit message (Collapse)AuthorFilesLines
2022-04-04Merge branch 'jh/builtin-fsmonitor-part2'Libravatar Junio C Hamano1-0/+7
Built-in fsmonitor (part 2). * jh/builtin-fsmonitor-part2: (30 commits) t7527: test status with untracked-cache and fsmonitor--daemon fsmonitor: force update index after large responses fsmonitor--daemon: use a cookie file to sync with file system fsmonitor--daemon: periodically truncate list of modified files t/perf/p7519: add fsmonitor--daemon test cases t/perf/p7519: speed up test on Windows t/perf/p7519: fix coding style t/helper/test-chmtime: skip directories on Windows t/perf: avoid copying builtin fsmonitor files into test repo t7527: create test for fsmonitor--daemon t/helper/fsmonitor-client: create IPC client to talk to FSMonitor Daemon help: include fsmonitor--daemon feature flag in version info fsmonitor--daemon: implement handle_client callback compat/fsmonitor/fsm-listen-darwin: implement FSEvent listener on MacOS compat/fsmonitor/fsm-listen-darwin: add MacOS header files for FSEvent compat/fsmonitor/fsm-listen-win32: implement FSMonitor backend on Windows fsmonitor--daemon: create token-based changed path cache fsmonitor--daemon: define token-ids fsmonitor--daemon: add pathname classification fsmonitor--daemon: implement 'start' command ...
2022-03-25help: include fsmonitor--daemon feature flag in version infoLibravatar Jeff Hostetler1-0/+7
Add the "feature: fsmonitor--daemon" message to the output of `git version --build-options`. The builtin FSMonitor is only available on certain platforms and even then only when certain Makefile flags are enabled, so print a message in the verbose version output when it is available. This can be used by test scripts for prereq testing. Granted, tests could just try `git fsmonitor--daemon status` and look for a 128 exit code or grep for a "not supported" message on stderr, but these methods are rather obscure. The main advantage is that the feature message will automatically appear in bug reports and other support requests. This concept was also used during the development of Scalar for similar reasons. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-24test-lib: have --immediate emit valid TAP on failureLibravatar Ævar Arnfjörð Bjarmason1-1/+5
Change the "--immediate" option so that it emits valid TAP on failure. Before this it would omit the required plan at the end, e.g. under SANITIZE=leak we'd show a "No plan found in TAP output" error from "prove": $ prove t0006-date.sh :: --immediate t0006-date.sh .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/22 subtests Test Summary Report ------------------- t0006-date.sh (Wstat: 256 Tests: 22 Failed: 1) Failed test: 22 Non-zero exit status: 1 Parse errors: No plan found in TAP output Files=1, Tests=22, 0 wallclock secs ( 0.02 usr 0.01 sys + 0.18 cusr 0.06 csys = 0.27 CPU) Result: FAIL Now we'll emit output that doesn't result in TAP parsing failures: $ prove t0006-date.sh :: --immediate t0006-date.sh .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/22 subtests Test Summary Report ------------------- t0006-date.sh (Wstat: 256 Tests: 22 Failed: 1) Failed test: 22 Non-zero exit status: 1 Files=1, Tests=22, 0 wallclock secs ( 0.02 usr 0.00 sys + 0.19 cusr 0.05 csys = 0.26 CPU) Result: FAIL Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-21Merge branch 'ep/test-malloc-check-with-glibc-2.34'Libravatar Junio C Hamano1-0/+18
The method to trigger malloc check used in our tests no longer work with newer versions of glibc. * ep/test-malloc-check-with-glibc-2.34: test-lib: declare local variables as local test-lib.sh: Use GLIBC_TUNABLES instead of MALLOC_CHECK_ on glibc >= 2.34
2022-03-09test-lib: declare local variables as localLibravatar Michael J Gruber1-0/+2
131b94a10a ("test-lib.sh: Use GLIBC_TUNABLES instead of MALLOC_CHECK_ on glibc >= 2.34", 2022-03-04) introduced "local" variables without declaring them as such. This conflicts with their use in some tests (at least when running them with dash), leading to test failures in: t0006-date.sh t2002-checkout-cache-u.sh t3430-rebase-merges.sh t4138-apply-ws-expansion.sh t4124-apply-ws-rule.sh Declare those variables as local to let the tests pass again. Signed-off-by: Michael J Gruber <git@grubix.eu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04test-lib.sh: Use GLIBC_TUNABLES instead of MALLOC_CHECK_ on glibc >= 2.34Libravatar Elia Pinto1-0/+16
In glibc >= 2.34 MALLOC_CHECK_ and MALLOC_PERTURB_ environment variables have been replaced by GLIBC_TUNABLES. Also the new glibc requires that you preload a library called libc_malloc_debug.so to get these features. Using the ordinary glibc system variable detect if this is glibc >= 2.34 and use GLIBC_TUNABLES and the new library. This patch was inspired by a Richard W.M. Jones ndbkit patch Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Elia Pinto <gitter.spiros@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-02-28test-lib: add "fast_unwind_on_malloc=0" to LSAN_OPTIONSLibravatar Ævar Arnfjörð Bjarmason1-0/+1
Add "fast_unwind_on_malloc=0" to LSAN_OPTIONS to get more meaningful stack traces from LSAN. This isn't required under ASAN which will emit traces such as this one for a leak in "t/t0006-date.sh": $ ASAN_OPTIONS=detect_leaks=1 ./t0006-date.sh -vixd [...] Direct leak of 3 byte(s) in 1 object(s) allocated from: #0 0x488b94 in strdup (t/helper/test-tool+0x488b94) #1 0x9444a4 in xstrdup wrapper.c:29:14 #2 0x5995fa in parse_date_format date.c:991:24 #3 0x4d2056 in show_dates t/helper/test-date.c:39:2 #4 0x4d174a in cmd__date t/helper/test-date.c:116:3 #5 0x4cce89 in cmd_main t/helper/test-tool.c:127:11 #6 0x4cd1e3 in main common-main.c:52:11 #7 0x7fef3c695e49 in __libc_start_main csu/../csu/libc-start.c:314:16 #8 0x422b09 in _start (t/helper/test-tool+0x422b09) SUMMARY: AddressSanitizer: 3 byte(s) leaked in 1 allocation(s). Aborted Whereas LSAN would emit this instead: $ ./t0006-date.sh -vixd [...] Direct leak of 3 byte(s) in 1 object(s) allocated from: #0 0x4323b8 in malloc (t/helper/test-tool+0x4323b8) #1 0x7f2be1d614aa in strdup string/strdup.c:42:15 SUMMARY: LeakSanitizer: 3 byte(s) leaked in 1 allocation(s). Aborted Now we'll instead git this sensible stack trace under LSAN. I.e. almost the same one (but starting with "malloc", as is usual for LSAN) as under ASAN: Direct leak of 3 byte(s) in 1 object(s) allocated from: #0 0x4323b8 in malloc (t/helper/test-tool+0x4323b8) #1 0x7f012af5c4aa in strdup string/strdup.c:42:15 #2 0x5cb164 in xstrdup wrapper.c:29:14 #3 0x495ee9 in parse_date_format date.c:991:24 #4 0x453aac in show_dates t/helper/test-date.c:39:2 #5 0x453782 in cmd__date t/helper/test-date.c:116:3 #6 0x451d95 in cmd_main t/helper/test-tool.c:127:11 #7 0x451f1e in main common-main.c:52:11 #8 0x7f012aef5e49 in __libc_start_main csu/../csu/libc-start.c:314:16 #9 0x42e0a9 in _start (t/helper/test-tool+0x42e0a9) SUMMARY: LeakSanitizer: 3 byte(s) leaked in 1 allocation(s). Aborted As the option name suggests this does make things slower, e.g. for t0001-init.sh we're around 10% slower: $ hyperfine -L v 0,1 'LSAN_OPTIONS=fast_unwind_on_malloc={v} make T=t0001-init.sh' -r 3 Benchmark 1: LSAN_OPTIONS=fast_unwind_on_malloc=0 make T=t0001-init.sh Time (mean ± σ): 2.135 s ± 0.015 s [User: 1.951 s, System: 0.554 s] Range (min … max): 2.122 s … 2.152 s 3 runs Benchmark 2: LSAN_OPTIONS=fast_unwind_on_malloc=1 make T=t0001-init.sh Time (mean ± σ): 1.981 s ± 0.055 s [User: 1.769 s, System: 0.488 s] Range (min … max): 1.941 s … 2.044 s 3 runs Summary 'LSAN_OPTIONS=fast_unwind_on_malloc=1 make T=t0001-init.sh' ran 1.08 ± 0.03 times faster than 'LSAN_OPTIONS=fast_unwind_on_malloc=0 make T=t0001-init.sh' I think that's more than worth it to get the more meaningful stack traces, we can always provide LSAN_OPTIONS=fast_unwind_on_malloc=0 for one-off "fast" runs. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-02-28test-lib: make $GIT_BUILD_DIR an absolute pathLibravatar Ævar Arnfjörð Bjarmason1-2/+3
Change the GIT_BUILD_DIR from a path like "/path/to/build/t/.." to "/path/to/build". The "TEST_DIRECTORY" here is already made an absolute path a few lines above this. We could simply do $(cd "$TEST_DIRECTORY"/.." && pwd) here, but as noted in the preceding commit the "$TEST_DIRECTORY" can't be anything except the path containing this test-lib.sh file at this point, so we can more cheaply and equally strip the "/t" off the end. This change will be helpful to LSAN_OPTIONS which will want to strip the build directory path from filenames, which we couldn't do if we had a "/.." in there. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-02-28test-lib: correct and assert TEST_DIRECTORY overridingLibravatar Ævar Arnfjörð Bjarmason1-5/+17
Correct a misleading comment added by me in 62f539043c7 (test-lib: Allow overriding of TEST_DIRECTORY, 2010-08-19), and add an assertion that TEST_DIRECTORY cannot point to any directory except the "t" directory in the top-level of git.git. This assertion is in effect not new, since we'd already die if that wasn't the case[1], but it and the updated commentary help to make that clearer. The existing comments were also on the wrong arms of the "if". I.e. the "allow tests to override this" was on the "test -z" arm. That came about due to a combination of 62f539043c7 and 85176d72513 (test-lib.sh: convert $TEST_DIRECTORY to an absolute path, 2013-11-17). Those earlier comments could be read as allowing the "$TEST_DIRECTORY" to be some path outside of t/. As explained in the updated comment that's impossible, rather it was meant for *tests* that ran outside of t/, i.e. the "t0000-basic.sh" tests that use "lib-subtest.sh". Those tests have a different working directory, but they set the "TEST_DIRECTORY" to the same path for bootstrapping. The comments now reflect that, and further comment on why we have a hard dependency on this. 1. https://lore.kernel.org/git/220222.86o82z8als.gmgdl@evledraar.gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-02-28test-lib: add GIT_SAN_OPTIONS, inherit [AL]SAN_OPTIONSLibravatar Ævar Arnfjörð Bjarmason1-4/+20
Change our ASAN_OPTIONS and LSAN_OPTIONS to set defaults for those variables, rather than punting out entirely if we already have them in the environment. We want to take any user-provided settings over our own, but we can do that by prepending our defaults to the variable. The libsanitizer options parsing has "last option wins" semantics. It's now possible to do e.g.: LSAN_OPTIONS=report_objects=1 ./t0006-date.sh And not have the "report_objects=1" setting overwrite our sensible default of "abort_on_error=1", but by prepending to the list we ensure that: LSAN_OPTIONS=report_objects=1:abort_on_error=0 ./t0006-date.sh Will take the desired "abort_on_error=0" over our default. See b0f4c9087e1 (t: support clang/gcc AddressSanitizer, 2014-12-08) for the original pattern being altered here, and 85b81b35ff9 (test-lib: set LSAN_OPTIONS to abort by default, 2017-09-05) for when LSAN_OPTIONS was added in addition to the then-existing ASAN_OPTIONS. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-02-09Merge branch 'js/test-unset-trace2-parents'Libravatar Junio C Hamano1-0/+2
Avoid tests that are run under GIT_TRACE2 set from failing unnecessarily. * js/test-unset-trace2-parents: test-lib: unset trace2 parent envvars
2022-01-20test-lib: unset trace2 parent envvarsLibravatar Josh Steadmon1-0/+2
The trace2 subsystem can inherit certain information from parent processes via environment variables; e.g., the parent command name and session ID. This allows trace2 to note when a command is the child process of another Git process, and to adjust various pieces of output accordingly. This behavior breaks certain tests that examine trace2 output when the tests run as a child of another git process, such as in `git rebase -x "make test"`. While we could fix this by unsetting the relevant variables in the affected tests (currently t0210, t0211, t0212, and t6421), this would leave other tests vulnerable to similar breakage if new test cases are added which inspect trace2 output. So fix this in general by unsetting GIT_TRACE2_PARENT_NAME and GIT_TRACE2_PARENT_SID in test-lib.sh. Reported-by: Emily Shaffer <emilyshaffer@google.com> Helped-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Josh Steadmon <steadmon@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-15Merge branch 'ew/test-wo-fsync'Libravatar Junio C Hamano1-0/+7
Allow running our tests while disabling fsync. * ew/test-wo-fsync: tests: disable fsync everywhere
2021-12-15Merge branch 'ds/trace2-regions-in-tests'Libravatar Junio C Hamano1-0/+7
The default setting for trace2 event nesting was too low to cause test failures, which is worked around by bumping it up in the test framework. * ds/trace2-regions-in-tests: t/t*: remove custom GIT_TRACE2_EVENT_NESTING test-lib.sh: set GIT_TRACE2_EVENT_NESTING
2021-12-15Merge branch 'fs/test-prereq'Libravatar Junio C Hamano1-4/+21
The test framework learns to list unsatisfied test prerequisites, and optionally error out when prerequisites that are expected to be satisfied are not. * fs/test-prereq: test-lib: make BAIL_OUT() work in tests and prereq test-lib: introduce required prereq for test runs test-lib: show missing prereq summary
2021-12-01test-lib: make BAIL_OUT() work in tests and prereqLibravatar Fabian Stelzer1-4/+10
BAIL_OUT() is meant to abort the whole test run and print a message with a standard prefix that can be parsed to stdout. Since for every test the normal fd`s are redirected in test_eval_ this output would not be seen when used within the context of a test or prereq like we do in test_have_prereq(). To make this function work in these contexts we move the setup of the fd aliases a few lines up before the first use of BAIL_OUT() and then have this function always print to the alias. Signed-off-by: Fabian Stelzer <fs@gigacodes.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-11-29Merge branch 'mc/clean-smudge-with-llp64'Libravatar Junio C Hamano1-0/+4
The clean/smudge conversion code path has been prepared to better work on platforms where ulong is narrower than size_t. * mc/clean-smudge-with-llp64: clean/smudge: allow clean filters to process extremely large files odb: guard against data loss checking out a huge file git-compat-util: introduce more size_t helpers odb: teach read_blob_entry to use size_t t1051: introduce a smudge filter test for extremely large files test-lib: add prerequisite for 64-bit platforms test-tool genzeros: generate large amounts of data more efficiently test-genzeros: allow more than 2G zeros in Windows
2021-11-29test-lib.sh: set GIT_TRACE2_EVENT_NESTINGLibravatar Derrick Stolee1-0/+7
The GIT_TRACE2_EVENT feed has a limited nesting depth to avoid overloading the feed when recursing into deep paths while adding more nested regions. Some tests use the GIT_TRACE2_EVENT feed to look for internal events, ensuring that intended behavior is happening. One such example is in t4216-log-bloom.sh which looks for a statistic given as a trace2_data_intmax() call. This test started failing under '-x' with 2ca245f8be5 (csum-file.h: increase hashfile buffer size, 2021-05-18) because the change in stderr triggered the progress API to create an extra trace2 region, ejecting the statistic. This change increases the value of GIT_TRACE2_EVENT_NESTING across the entire test suite to avoid errors like this. Future changes will remove custom assignments of GIT_TRACE2_EVENT_NESTING from some test scripts that were aware of this limitation. Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Helped-by: Jeff King <peff@peff.net> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-11-20test-lib: show missing prereq summaryLibravatar Fabian Stelzer1-0/+11
When running the full test suite many tests can be skipped because of missing prerequisites. It not easy right now to get an overview of which ones are missing. When switching to a new machine or environment some libraries and tools might be missing or maybe a dependency broke completely. In this case the tests would indicate nothing since all dependant tests are simply skipped. This could hide broken behaviour or missing features in the build. Therefore this patch summarizes the missing prereqs at the end of the test run making it easier to spot such cases. - Add failed prereqs to the test results. - Aggregate and then show them with the totals. Signed-off-by: Fabian Stelzer <fs@gigacodes.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-11-03test-lib: add prerequisite for 64-bit platformsLibravatar Carlo Marcelo Arenas Belón1-0/+4
Allow tests that assume a 64-bit `size_t` to be skipped in 32-bit platforms and regardless of the size of `long`. This imitates the `LONG_IS_64BIT` prerequisite. Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-10-29Merge branch 'ab/test-bail'Libravatar Junio C Hamano1-7/+21
A new feature has been added to abort early in the test framework. * ab/test-bail: test-lib.sh: use "Bail out!" syntax on bad SANITIZE=leak use test-lib.sh: de-duplicate error() teardown code
2021-10-29tests: disable fsync everywhereLibravatar Eric Wong1-0/+7
The "GIT_TEST_FSYNC" environment variable now exists for disabling fsync() even on packfiles and other "critical" data. Running "make test -j8 NO_SVN_TESTS=1" on a noisy 8-core system on an HDD, test runtime drops from ~4 minutes down to ~3 minutes. Using "GIT_TEST_FSYNC=1" re-enables fsync() for comparison purposes. SVN interopability tests are minimally affected since SVN will still use fsync in various places. This will also be useful for 3rd-party tools which create throwaway git repositories of temporary data, but remains undocumented for end users. Signed-off-by: Eric Wong <e@80x24.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-10-25Merge branch 'ab/test-cleanly-recreate-trash-directory'Libravatar Junio C Hamano1-1/+13
Improve test framework around unwritable directories. * ab/test-cleanly-recreate-trash-directory: test-lib.sh: try to re-chmod & retry on failed trash removal
2021-10-18Merge branch 'js/retire-preserve-merges'Libravatar Junio C Hamano1-4/+0
The "--preserve-merges" option of "git rebase" has been removed. * js/retire-preserve-merges: sequencer: restrict scope of a formerly public function rebase: remove a no-longer-used function rebase: stop mentioning the -p option in comments rebase: remove obsolete code comment rebase: drop the internal `rebase--interactive` command git-svn: drop support for `--preserve-merges` rebase: drop support for `--preserve-merges` pull: remove support for `--rebase=preserve` tests: stop testing `git rebase --preserve-merges` remote: warn about unhandled branch.<name>.rebase values t5520: do not use `pull.rebase=preserve`
2021-10-15test-lib.sh: try to re-chmod & retry on failed trash removalLibravatar Ævar Arnfjörð Bjarmason1-1/+13
Try to re-chmod the trash directory on startup if we fail to "rm -rf" it. This fixes problems where the test leaves the trash directory behind in a bad permission state for whatever reason. This fixes an interaction between [1] where t0004-unwritable.sh was made to use "test_when_finished" for cleanup, and [2] which added the "--immediate" mode. If a test in this file failed when running with "--immediate" we wouldn't run the "test_when_finished" block, which re-chmods the ".git/objects" directory (see [1]). This can be demonstrated as e.g. (output snipped for less verbosity): $ ./t0004-unwritable.sh --run=3 --immediate ok 1 # skip setup (--run) ok 2 # skip write-tree should notice unwritable repository (--run) not ok 3 - commit should notice unwritable repository [...] $ ./t0004-unwritable.sh --run=3 --immediate rm: cannot remove '[...]/trash directory.t0004-unwritable/.git/objects/info': Permission denied FATAL: Cannot prepare test area [...] Instead of some version of reverting [1] let's make the test-lib.sh resilient to this edge-case, it will happen due to [1], but also e.g. if the relevant "test-lib.sh" process is kill -9'd during the test run. We should try harder to recover in this case. If we fail to remove the test directory let's retry after (re-)chmod-ing it. This doesn't need to be guarded by something that's equivalent to "POSIXPERM" since if we don't support "chmod" we were about to fail anyway. Let's also discard any error output from (a possibly nonexisting) "chmod", we'll fail on the subsequent "rm -rf" anyway, likewise for the first "rm -rf" invocation, we don't want to get the "cannot remove" output if we can get around it with the "chmod", but we do want any error output from the second "rm -rf", in case that doesn't fix the issue. The lack of &&-chaining between the "chmod" and "rm -rf" is intentional, if we fail the first "rm -rf", can't chmod, but then succeed the second time around that's what we were hoping for. We just want to nuke the directory, not carry forward every possible error code or error message. 1. dbda967684d (t0004 (unwritable files): simplify error handling, 2010-09-06) 2. b586744a864 (test: skip clean-up when running under --immediate mode, 2011-06-27) Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-10-14test-lib.sh: use "Bail out!" syntax on bad SANITIZE=leak useLibravatar Ævar Arnfjörð Bjarmason1-4/+14
Improve the "GIT_TEST_PASSING_SANITIZE_LEAK=true" test mode added in 956d2e4639b (tests: add a test mode for SANITIZE=leak, run it in CI, 2021-09-23) to use a TAP "Bail out!" message when exiting. This will cause the test run to exit immediately under a TAP consumer like "prove(1)". See 614fe015212 (test-lib: bail out when "-v" used under "prove", 2016-10-22) for the initial introduction of "Bail out!" to the --verbose being amended here. Before this compiling with "SANITIZE=" and running the tests with "prove(1)" would cause all the tests to be run to the end (output trimmed for fewer columns): $ GIT_TEST_PASSING_SANITIZE_LEAK=true make rm -f -r 'test-results' *** prove *** t0000-basic.sh ......... Dubious, test returned 1 (wstat 256, 0x100) No subtests run t0001-init.sh .......... Dubious, test returned 1 (wstat 256, 0x100) No subtests run [...output where we list every single t[0-9]*.sh file as failing snipped] Whereas now we'll fail early, like this ("->" line wrapping added): $ GIT_TEST_PASSING_SANITIZE_LEAK=true make [...] t0000-basic.sh ..................................... Bailout called. Further testing stopped: -> GIT_TEST_PASSING_SANITIZE_LEAK=true has no effect except when compiled with SANITIZE=leak FAILED--Further testing stopped: GIT_TEST_PASSING_SANITIZE_LEAK=true has no effect except -> when compiled with SANITIZE=leak make: *** [Makefile:53: prove] Error 1 This change also adds a red color to the "Bailout called" line, as we're now using "say_color error". That improves existing output in the case of e.g.: $ prove -j8 t[0-9]*.sh :: -v 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 We don't need to have a "Bail out! " prefix when we're not running under a TAP consumer (i.e. if test -n "$HARNESS_ACTIVE"), but let's not make the output conditional on that. Showing it under e.g.: $ GIT_TEST_PASSING_SANITIZE_LEAK=true ./t0095-bloom.sh Bail out! GIT_TEST_PASSING_SANITIZE_LEAK=true has no effect except when compiled with SANITIZE=leak Doesn't harm anything, and I don't think the (small) complexity of only adding this if we're under "$HARNESS_ACTIVE" is worth it. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-10-14test-lib.sh: de-duplicate error() teardown codeLibravatar Ævar Arnfjörð Bjarmason1-3/+7
De-duplicate the "finalize_junit_xml; GIT_EXIT_OK=t; exit 1" code shared between the "error()" and "--immediate on failure" code paths, in preparation for adding a third user in a subsequent commit. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-10-11Merge branch 'ab/sanitize-leak-ci'Libravatar Junio C Hamano1-0/+21
CI learns to run the leak sanitizer builds. * ab/sanitize-leak-ci: tests: add a test mode for SANITIZE=leak, run it in CI Makefile: add SANITIZE=leak flag to GIT-BUILD-OPTIONS
2021-09-23Merge branch 'ab/unused-script-helpers'Libravatar Junio C Hamano1-4/+2
Code clean-up. * ab/unused-script-helpers: test-lib: remove unused $_x40 and $_z40 variables git-bisect: remove unused SHA-1 $x40 shell variable git-sh-setup: remove unused "pull with rebase" message git-submodule: remove unused is_zero_oid() function
2021-09-23tests: add a test mode for SANITIZE=leak, run it in CILibravatar Ævar Arnfjörð Bjarmason1-0/+20
While git can be compiled with SANITIZE=leak, we have not run regression tests under that mode. Memory leaks have only been fixed as one-offs without structured regression testing. This change adds CI testing for it. We'll now build and small set of whitelisted t00*.sh tests under Linux with a new job called "linux-leaks". The CI target uses a new GIT_TEST_PASSING_SANITIZE_LEAK=true test mode. When running in that mode, we'll assert that we were compiled with SANITIZE=leak. We'll then skip all tests, except those that we've opted-in by setting "TEST_PASSES_SANITIZE_LEAK=true". A test setting "TEST_PASSES_SANITIZE_LEAK=true" setting can in turn make use of the "SANITIZE_LEAK" prerequisite, should they wish to selectively skip tests even under "GIT_TEST_PASSING_SANITIZE_LEAK=true". In the preceding commit we started doing this in "t0004-unwritable.sh" under SANITIZE=leak, now it'll combine nicely with "GIT_TEST_PASSING_SANITIZE_LEAK=true". This is how tests that don't set "TEST_PASSES_SANITIZE_LEAK=true" will be skipped under GIT_TEST_PASSING_SANITIZE_LEAK=true: $ GIT_TEST_PASSING_SANITIZE_LEAK=true ./t0001-init.sh 1..0 # SKIP skip all tests in t0001 under SANITIZE=leak, TEST_PASSES_SANITIZE_LEAK not set The intent is to add more TEST_PASSES_SANITIZE_LEAK=true annotations as follow-up change, but let's start small to begin with. In ci/run-build-and-tests.sh we make use of the default "*" case to run "make test" without any GIT_TEST_* modes. SANITIZE=leak is known to fail in combination with GIT_TEST_SPLIT_INDEX=true in t0016-oidmap.sh, and we're likely to have other such failures in various GIT_TEST_* modes. Let's focus on getting the base tests passing, we can expand coverage to GIT_TEST_* modes later. It would also be possible to implement a more lightweight version of this by only relying on setting "LSAN_OPTIONS". See <YS9OT/pn5rRK9cGB@coredump.intra.peff.net>[1] and <YS9ZIDpANfsh7N+S@coredump.intra.peff.net>[2] for a discussion of that. I've opted for this approach of adding a GIT_TEST_* mode instead because it's consistent with how we handle other special test modes. Being able to add a "!SANITIZE_LEAK" prerequisite and calling "test_done" early if it isn't satisfied also means that we can more incrementally add regression tests without being forced to fix widespread and hard-to-fix leaks at the same time. We have tests that do simple checking of some tool we're interested in, but later on in the script might be stressing trace2, or common sources of leaks like "git log" in combination with the tool (e.g. the commit-graph tests). To be clear having a prerequisite could also be accomplished by using "LSAN_OPTIONS" directly. On the topic of "LSAN_OPTIONS": It would be nice to have a mode to aggregate all failures in our various scripts, see [2] for a start at doing that which sets "log_path" in "LSAN_OPTIONS". I've punted on that for now, it can be added later. As of writing this we've got major regressions between master..seen, i.e. the t000*.sh tests and more fixed since 31f9acf9ce2 (Merge branch 'ah/plugleaks', 2021-08-04) have regressed recently. See the discussion at <87czsv2idy.fsf@evledraar.gmail.com>[3] about the lack of this sort of test mode, and 0e5bba53af (add UNLEAK annotation for reducing leak false positives, 2017-09-08) for the initial addition of SANITIZE=leak. See also 09595ab381 (Merge branch 'jk/leak-checkers', 2017-09-19), 7782066f67 (Merge branch 'jk/apache-lsan', 2019-05-19) and the recent 936e58851a (Merge branch 'ah/plugleaks', 2021-05-07) for some of the past history of "one-off" SANITIZE=leak (and more) fixes. As noted in [5] we can't support this on OSX yet until Clang 14 is released, at that point we'll probably want to resurrect that "osx-leaks" job. 1. https://github.com/google/sanitizers/wiki/AddressSanitizerLeakSanitizer 2. https://lore.kernel.org/git/YS9OT%2Fpn5rRK9cGB@coredump.intra.peff.net/ 3. https://lore.kernel.org/git/87czsv2idy.fsf@evledraar.gmail.com/ 4. https://lore.kernel.org/git/YS9ZIDpANfsh7N+S@coredump.intra.peff.net/ 5. https://lore.kernel.org/git/20210916035603.76369-1-carenas@gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-23Makefile: add SANITIZE=leak flag to GIT-BUILD-OPTIONSLibravatar Ævar Arnfjörð Bjarmason1-0/+1
When SANITIZE=leak is specified we'll now add a SANITIZE_LEAK flag to GIT-BUILD-OPTIONS, this can then be picked up by the test-lib.sh, which sets a SANITIZE_LEAK prerequisite. We can then skip specific tests that are known to fail under SANITIZE=leak, add one such annotation to t0004-unwritable.sh, which now passes under SANITIZE=leak. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-15Merge branch 'pb/test-use-user-env'Libravatar Junio C Hamano1-2/+4
Teach "test_pause" and "debug" helpers to allow using the HOME and TERM environment variables the user usually uses. * pb/test-use-user-env: test-lib-functions: keep user's debugger config files and TERM in 'debug' test-lib-functions: optionally keep HOME, TERM and SHELL in 'test_pause' test-lib-functions: use 'TEST_SHELL_PATH' in 'test_pause'
2021-09-12test-lib: remove unused $_x40 and $_z40 variablesLibravatar Ævar Arnfjörð Bjarmason1-4/+2
These two have fallen out of use with the SHA-256 migration. The last use of $_x40 was removed in fc7e73d7ef (t4013: improve diff-post-processor logic, 2020-08-21) and The last use of $_z40 was removed in 7a868c51c2 (t5562: use $ZERO_OID, 2019-12-21), but it was then needlessly refactored to be hash-agnostic in 192b517589 (t: use hash-specific lookup tables to define test constants, 2020-02-22). We can just remove it. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-07tests: stop testing `git rebase --preserve-merges`Libravatar Johannes Schindelin1-4/+0
This backend has been deprecated in favor of `git rebase --rebase-merges`. In preparation for dropping it, let's remove all the regression tests that would need it. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-07test-lib-functions: optionally keep HOME, TERM and SHELL in 'test_pause'Libravatar Philippe Blain1-2/+4
The 'test_pause' function, which is designed to help interactive debugging and exploration of tests, currently inherits the value of HOME and TERM set by 'test-lib.sh': HOME="$TRASH_DIRECTORY" and TERM=dumb. It also invokes the shell defined by TEST_SHELL_PATH, which defaults to /bin/sh (through SHELL_PATH). Changing the value of HOME means that any customization configured in a developers' shell startup files and any Git aliases defined in their global Git configuration file are not available in the shell invoked by 'test_pause'. Changing the value of TERM to 'dumb' means that colored output is disabled for all commands in that shell. Using /bin/sh as the shell invoked by 'test_pause' is not ideal since some platforms (i.e. Debian and derivatives) use Dash as /bin/sh, and this shell is usually compiled without readline support, which makes for a poor interactive command line experience. To make the interactive command line experience in the shell invoked by 'test_pause' more pleasant, save the values of HOME and TERM in USER_HOME and USER_TERM before changing them in test-lib.sh, and add options to 'test_pause' to optionally use these variables to invoke the shell. Also add an option to invoke SHELL instead of TEST_SHELL_PATH, so that developer's interactive shell is used. We use options instead of changing the behaviour unconditionally since these three variables can slightly change command behaviour. Moreover, using the original HOME means commands could overwrite files in a user's home directory. Be explicit about these caveats in the new 'Usage' section in test-lib-functions.sh. Finally, add '[options]' to the test_pause synopsys in t/README, and mention that the full list of helper functions and their options can be found in test-lib-functions.sh. Helped-by: Elijah Newren <newren@gmail.com> Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> Acked-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-30test-lib: set GIT_CEILING_DIRECTORIES to protect the surrounding repositoryLibravatar SZEDER Gábor1-1/+2
Every once in a while a test somehow manages to escape from its trash directory and modifies the surrounding repository, whether because of a bug in git itself, a bug in a test [1], or e.g. when trying to run tests with a shell that is, in general, unable to run our tests [2]. Set GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/.." as an additional safety measure to protect the surrounding repository at least from modifications by git commands executed in the tests (assuming that handling of ceiling directories during repository discovery is not broken, and, of course, it won't save us from regular shell commands, e.g. 'cd .. && rm -f ...'). [1] e.g. https://public-inbox.org/git/20210423051255.GD2947267@szeder.dev [2] $ git symbolic-ref HEAD refs/heads/master $ ksh ./t2011-checkout-invalid-head.sh [... a lot of "not ok" ...] $ git symbolic-ref HEAD refs/heads/other (In short: 'ksh' doesn't support the 'local' builtin command, which is used by 'test_oid', causing it to return with error whenever it's called, leaving ZERO_OID set to empty, so when the test 'checkout main from invalid HEAD' runs 'echo $ZERO_OID >.git/HEAD' it writes a corrupt (not invalid) HEAD, and subsequent git commands don't recognize the repository in the trash directory anymore, but operate on the surrounding repo.) Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-06Merge branch 'fc/disable-checkwinsize'Libravatar Junio C Hamano1-0/+6
* fc/disable-checkwinsize: test: fix for COLUMNS and bash 5
2021-08-06test: fix for COLUMNS and bash 5Libravatar Felipe Contreras1-0/+6
Since c49a177bec (test-lib.sh: set COLUMNS=80 for --verbose repeatability, 2021-06-29) multiple tests have been failing when using bash 5 because checkwinsize is enabled by default, therefore COLUMNS is reset using TIOCGWINSZ even for non-interactive shells. It's debatable whether or not bash should even be doing that, but for now we can avoid this undesirable behavior by disabling this option. Reported-by: Fabian Stelzer <fabian.stelzer@campoint.net> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> [jc: with SZEDER Gábor's suggestion to do this before setting COLUMNS] Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-04Merge branch 'tb/mingw-rmdir-symlink-to-directory'Libravatar Junio C Hamano1-0/+6
Windows rmdir() equivalent behaves differently from POSIX ones in that when used on a symbolic link that points at a directory, the target directory gets removed, which has been corrected. * tb/mingw-rmdir-symlink-to-directory: mingw: align symlinks-related rmdir() behavior with Linux
2021-08-02mingw: align symlinks-related rmdir() behavior with LinuxLibravatar Thomas Bétous1-0/+6
When performing a rebase, rmdir() is called on the folder .git/logs. On Unix rmdir() exits without deleting anything in case .git/logs is a symbolic link but the equivalent functions on Windows (_rmdir, _wrmdir and RemoveDirectoryW) do not behave the same and remove the folder if it is symlinked even if it is not empty. This creates issues when folders in .git/ are symlinks which is especially the case when git-repo[1] is used: It replaces `.git/logs/` with a symlink. One such issue is that the _target_ of that symlink is removed e.g. during a `git rebase`, where `delete_reflog("REBASE_HEAD")` will not only try to remove `.git/logs/REBASE_HEAD` but then recursively try to remove the parent directories until an error occurs, a technique that obviously relies on `rmdir()` refusing to remove a symlink. This was reported in https://github.com/git-for-windows/git/issues/2967. This commit updates mingw_rmdir() so that its behavior is the same as Linux rmdir() in case of symbolic links. To verify that Git does not regress on the reported issue, this patch adds a regression test for the `git rebase` symptom, even if the same `rmdir()` behavior is quite likely to cause potential problems in other Git commands as well. [1]: git-repo is a python tool built on top of Git which helps manage many Git repositories. It stores all the .git/ folders in a central place by taking advantage of symbolic links. More information: https://gerrit.googlesource.com/git-repo/ Signed-off-by: Thomas Bétous <tomspycell@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-20t0000: fix test if run with TEST_OUTPUT_DIRECTORYLibravatar Patrick Steinhardt1-0/+9
Testcases in t0000 are quite special given that they many of them run nested testcases to verify that testing functionality itself works as expected. These nested testcases are realized by writing a new ad-hoc test script which again sources test-lib.sh, where the new script is created in a nested subdirectory located beneath the current trash directory. We then execute the new test script with the nested subdirectory as current working directory and explicitly re-export TEST_OUTPUT_DIRECTORY to point to that directory. While this works as expected in the general case, it falls apart when the developer has TEST_OUTPUT_DIRECTORY explicitly defined either via the environment or via config.mak and runs "make test". In that case, test-lib.sh will clobber the value that we've just carefully set up to instead contain what the developer has defined. As a result, the TEST_OUTPUT_DIRECTORY continues to point at the root output directory, not at the nested one. This issue causes breakage in the 'test_atexit is run' test case: the nested test case writes files into "../../", which is assumed to be the parent's trash directory. But because TEST_OUTPUT_DIRECTORY already points to to the root output directory, we instead end up writing those files outside of the output directory. The parent test case will then try to check whether those files still exist in its own trash directory, which thus must fail now. Fix the issue by adding a new TEST_OUTPUT_DIRECTORY_OVERRIDE variable. If set, then we'll always override the TEST_OUTPUT_DIRECTORY with its value after sourcing GIT-BUILD-OPTIONS. Signed-off-by: Patrick Steinhardt <ps@pks.im> Acked-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-13Merge branch 'hn/prep-tests-for-reftable'Libravatar Junio C Hamano1-0/+2
Preliminary clean-up of tests before the main reftable changes hits the codebase. * hn/prep-tests-for-reftable: (22 commits) t1415: set REFFILES for test specific to storage format t4202: mark bogus head hash test with REFFILES t7003: check reflog existence only for REFFILES t7900: stop checking for loose refs t1404: mark tests that muck with .git directly as REFFILES. t2017: mark --orphan/logAllRefUpdates=false test as REFFILES t1414: mark corruption test with REFFILES t1407: require REFFILES for for_each_reflog test test-lib: provide test prereq REFFILES t5304: use "reflog expire --all" to clear the reflog t5304: restyle: trim empty lines, drop ':' before > t7003: use rev-parse rather than FS inspection t5000: inspect HEAD using git-rev-parse t5000: reformat indentation to the latest fashion t1301: fix typo in error message t1413: use tar to save and restore entire .git directory t1401-symbolic-ref: avoid direct filesystem access t1401: use tar to snapshot and restore repo state t5601: read HEAD using rev-parse t9300: check ref existence using test-helper rather than a file system check ...
2021-07-08Merge branch 'ab/fix-columns-to-80-during-tests'Libravatar Junio C Hamano1-2/+3
Output from some of our tests were affected by the width of the terminal that they were run in, which has been corrected by exporting a fixed value in the COLUMNS environment. * ab/fix-columns-to-80-during-tests: test-lib.sh: set COLUMNS=80 for --verbose repeatability
2021-07-01test-lib: avoid accidental globbing in match_pattern_list()Libravatar Jeff King1-12/+22
We have a custom match_pattern_list() function which we use for matching test names (like "t1234") against glob-like patterns (like "t1???") for $GIT_SKIP_TESTS, --verbose-only, etc. Those patterns may have multiple whitespace-separated elements (e.g., "t0* t1234 t5?78"). The callers of match_pattern_list thus pass the strings unquoted, so that the shell does the usual field-splitting into separate arguments. But this also means the shell will do the usual globbing for each argument, which can result in us seeing an expansion based on what's in the filesystem, rather than the real pattern. For example, if I have the path "t5000" in the filesystem, and you feed the pattern "t?000", that _should_ match the string "t0000", but it won't after the shell has expanded it to "t5000". This has been a bug ever since that function was introduced. But it didn't usually trigger since we typically use the function inside the trash directory, which has a very limited set of files that are unlikely to match. It became a lot easier to trigger after edc23840b0 (test-lib: bring $remove_trash out of retirement, 2021-05-10), because now we match $GIT_SKIP_TESTS before even entering the trash directory. So the t5000 example above can be seen with: GIT_SKIP_TESTS=t?000 ./t0000-basic.sh which should skip all tests but doesn't. We can fix this by using "set -f" to ask the shell not to glob (which is in POSIX, so should hopefully be portable enough). We only want to do this in a subshell (to avoid polluting the rest of the script), which means we need to get the whole string intact into the match_pattern_list function by quoting it. Arguably this is a good idea anyway, since it makes it much more obvious that we intend to split, and it's not simply sloppy scripting. Diagnosed-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-06-29test-lib.sh: set COLUMNS=80 for --verbose repeatabilityLibravatar Ævar Arnfjörð Bjarmason1-2/+3
Some tests will fail under --verbose because while we've unset COLUMNS since b1d645b58ac (tests: unset COLUMNS inherited from environment, 2012-03-27), we also look for the columns with an ioctl(.., TIOCGWINSZ, ...) on some platforms. By setting COLUMNS again we preempt the TIOCGWINSZ lookup in pager.c's term_columns(), it'll take COLUMNS over TIOCGWINSZ, This fixes t0500-progress-display.sh., which broke because of a combination of the this issue and the progress output reacting to the column width since 545dc345ebd (progress: break too long progress bar lines, 2019-04-12). The t5324-split-commit-graph.sh fails in a similar manner due to progress output, see [1] for details. The issue is not specific to progress.c, the diff code also checks COLUMNS and some of its tests can be made to fail in a similar manner[2], anything that invokes a pager is potentially affected. See ea77e675e56 (Make "git help" react to window size correctly, 2005-12-18) and ad6c3739a33 (pager: find out the terminal width before spawning the pager, 2012-02-12) for how the TIOCGWINSZ code ended up in pager.c 1. http://lore.kernel.org/git/20210624051253.GG6312@szeder.dev 2. https://lore.kernel.org/git/20210627074419.GH6312@szeder.dev/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-06-14Merge branch 'ab/test-lib-updates'Libravatar Junio C Hamano1-15/+25
Test clean-up. * ab/test-lib-updates: test-lib: split up and deprecate test_create_repo() test-lib: do not show advice about init.defaultBranch under --verbose test-lib: reformat argument list in test_create_repo() submodule tests: use symbolic-ref --short to discover branch name test-lib functions: add --printf option to test_commit describe tests: convert setup to use test_commit test-lib functions: add an --annotated option to "test_commit" test-lib-functions: document test_commit --no-tag test-lib-functions: reword "test_commit --append" docs test-lib tests: remove dead GIT_TEST_FRAMEWORK_SELFTEST variable test-lib: bring $remove_trash out of retirement
2021-06-02test-lib: provide test prereq REFFILESLibravatar Han-Wen Nienhuys1-0/+2
REFFILES can be used to mark tests that are specific to the packed/loose ref storage format and its limitations. Marking such tests is a preparation for introducing the reftable storage backend. Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-20Merge branch 'jk/test-chainlint-softer'Libravatar Junio C Hamano1-2/+5
The "chainlint" feature in the test framework is a handy way to catch common mistakes in writing new tests, but tends to get expensive. An knob to selectively disable it has been introduced to help running tests that the developer has not modified. * jk/test-chainlint-softer: t: avoid sed-based chain-linting in some expensive cases
2021-05-13t: avoid sed-based chain-linting in some expensive casesLibravatar Jeff King1-2/+5
Commit 878f988350 (t/test-lib: teach --chain-lint to detect broken &&-chains in subshells, 2018-07-11) introduced additional chain-lint tests which add an extra "sed" pipeline to each test we run. This has a measurable impact on runtime. Here are timings with and without a new environment variable (added by this patch) that lets you disable just the additional sed-based chain-lint tests: Benchmark #1: GIT_TEST_CHAIN_LINT_HARDER=1 make test Time (mean ± σ): 64.202 s ± 1.030 s [User: 622.469 s, System: 301.402 s] Range (min … max): 61.571 s … 65.662 s 10 runs Benchmark #2: GIT_TEST_CHAIN_LINT_HARDER=0 make test Time (mean ± σ): 57.591 s ± 0.333 s [User: 529.368 s, System: 270.618 s] Range (min … max): 57.143 s … 58.309 s 10 runs Summary 'GIT_TEST_CHAIN_LINT_HARDER=0 make test' ran 1.11 ± 0.02 times faster than 'GIT_TEST_CHAIN_LINT_HARDER=1 make test' Of course those extra lint checks are doing something useful, so paying a few extra seconds (at least on Linux) isn't so bad (though note the CPU time; we're bounded in our parallel run here by the slowest test, so it really is ~120s of CPU improvement). But we can observe that there are some test scripts where they produce a much stronger effect, and provide less value. In t0027 and t3070 we run a very large number of small tests, all driven by a series of functions/loops which are filling in the test bodies. There we get much less bang for our buck in terms of bug-finding versus CPU cost. This patch introduces a mechanism for controlling when those extra lint checks are run, at two levels: - a user can ask to disable or to force-enable the checks by setting GIT_TEST_CHAIN_LINT_HARDER - if the user hasn't specified a preference, individual scripts can disable the checks by setting GIT_TEST_CHAIN_LINT_HARDER_DEFAULT; scripts which don't set that get the current behavior of enabling them. In addition, this patch flips the default for t0027 and t3070's mass-generated sections to disable the extra checks. Here are the timing results for t0027: Benchmark #1: GIT_TEST_CHAIN_LINT_HARDER=1 ./t0027-auto-crlf.sh Time (mean ± σ): 17.078 s ± 0.848 s [User: 14.878 s, System: 7.075 s] Range (min … max): 15.952 s … 18.421 s 10 runs Benchmark #2: GIT_TEST_CHAIN_LINT_HARDER=0 ./t0027-auto-crlf.sh Time (mean ± σ): 9.063 s ± 0.759 s [User: 7.890 s, System: 3.362 s] Range (min … max): 7.747 s … 10.619 s 10 runs Benchmark #3: ./t0027-auto-crlf.sh Time (mean ± σ): 9.186 s ± 0.881 s [User: 7.957 s, System: 3.427 s] Range (min … max): 7.796 s … 10.498 s 10 runs Summary 'GIT_TEST_CHAIN_LINT_HARDER=0 ./t0027-auto-crlf.sh' ran 1.01 ± 0.13 times faster than './t0027-auto-crlf.sh' 1.88 ± 0.18 times faster than 'GIT_TEST_CHAIN_LINT_HARDER=1 ./t0027-auto-crlf.sh' We can see that disabling the checks for the whole script buys us an almost 2x speedup. But the new default behavior, disabling them only for the mass-generated part, gets us most of that speedup (but still leaves the checks on for further manual tests people might write). As a side note, I'd caution about comparing runtimes and CPU seconds between this timing and the earlier "make test" one. In "make test", we're running a lot of scripts in parallel, so the CPU is throttling down (and thus a CPU second saved here would count for more during a parallel run; the same work takes more CPU seconds there). We get similar results for t3070: Benchmark #1: GIT_TEST_CHAIN_LINT_HARDER=1 ./t3070-wildmatch.sh Time (mean ± σ): 20.054 s ± 3.967 s [User: 16.003 s, System: 8.286 s] Range (min … max): 11.891 s … 23.671 s 10 runs Benchmark #2: GIT_TEST_CHAIN_LINT_HARDER=0 ./t3070-wildmatch.sh Time (mean ± σ): 12.399 s ± 2.256 s [User: 7.542 s, System: 5.342 s] Range (min … max): 9.606 s … 15.727 s 10 runs Benchmark #3: ./t3070-wildmatch.sh Time (mean ± σ): 10.726 s ± 3.476 s [User: 6.790 s, System: 4.365 s] Range (min … max): 5.444 s … 15.376 s 10 runs Summary './t3070-wildmatch.sh' ran 1.16 ± 0.43 times faster than 'GIT_TEST_CHAIN_LINT_HARDER=0 ./t3070-wildmatch.sh' 1.87 ± 0.71 times faster than 'GIT_TEST_CHAIN_LINT_HARDER=1 ./t3070-wildmatch.sh' Again, we get almost a 2x speedup disabling these. In this case, there are no tests not covered by the script's "default to disable" behavior, so the second two benchmarks should be the same (and while they do differ, you can see the variance is quite high but they're within one standard deviation). So it seems like for these two scripts, at least, disabling the extra checks is a reasonable tradeoff. Sadly, the overall runtime of "make test" on my system doesn't get much faster. But that's because we're mostly limited by the cost of the single biggest test. Here are the top-5 tests by wall-clock time from a parallel run, before my patch: 57.9192368984222 t9001-send-email.sh 45.6329638957977 t0027-auto-crlf.sh 32.5278220176697 t3070-wildmatch.sh 22.2701289653778 t7610-mergetool.sh 20.8635759353638 t1701-racy-split-index.sh And after: 57.1476998329163 t9001-send-email.sh 33.776211977005 t0027-auto-crlf.sh 21.3116669654846 t7610-mergetool.sh 20.7748689651489 t1701-racy-split-index.sh 19.6957249641418 t7112-reset-submodule.sh We dropped 12s from t0027, and t3070 dropped off our list entirely at around 16s. In both cases we're bound by t9001, but its slowness is due to the actual tests, so we'll have to deal with it in a different way. But this reduces overall CPU, and means that dealing with t9001 (by improving the speed of send-email or splitting it apart) will let us reduce our overall runtime even on multi-core machines. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-05-11test-lib: split up and deprecate test_create_repo()Libravatar Ævar Arnfjörð Bjarmason1-1/+2
Remove various redundant or obsolete code from the test_create_repo() function, and split up its use in test-lib.sh from what tests need from it. This leave us with a pass-through wrapper for "git init" in test-lib-functions.sh, in test-lib.sh we have the same, except for needing to redirect stdout/stderr, and emitting an error ourselves if it fails. We don't need to error() ourselves when test_create_repo() is invoked, as the invocation will be a part of a test's "&&"-chain. Everything below this paragraph is a detailed summary of the history of test_create_repo() explaining why it's safe to remove the various things it was doing: 1. "mkdir -p" isn't needed because "git init" itself will create leading directories if needed. 2. Since we're now a simple wrapper for "git init" we don't need to check that we have only one argument. If someone wants to run "test_create_repo --bare x" that's OK. 3. We won't ever hit that "Cannot setup test environment" error. Checking the test environment sanity when doing "git init" dates back to eea420693be (t0000: catch trivial pilot errors., 2005-12-10) and 2ccd2027b01 (trivial: check, if t/trash directory was successfully created, 2006-01-05). We can also see it in another form a bit later in my own 0d314ce834d (test-lib: use subshell instead of cd $new && .. && cd $old, 2010-08-30). But since 2006f0adaee (t/test-lib: make sure Git has already been built, 2012-09-17) we already check if we have a built git earlier. The one thing this was testing after that 2012 change was that we'd just built "git", but not "git-init", but since 3af4c7156c4 (tests: respect GIT_TEST_INSTALLED when initializing repositories, 2018-11-12) we invoke "git", not "git-init". So all of that's been checked already, and we don't need to re-check it here. 4. We don't need to move .git/hooks out of the way. That dates back to c09a69a83e3 (Disable hooks during tests., 2005-10-16), since then hooks became disabled by default in f98f8cbac01 (Ship sample hooks with .sample suffix, 2008-06-24). So the hooks were already disabled by default, but as can be seen from "mkdir .git/hooks" changes various tests needed to re-setup that directory. Now they no longer do. This makes us implicitly depend on the default hooks being disabled, which is a good thing. If and when we'd have any on-by-default hooks (I see no reason we ever would) we'd want to see the subtle and not so subtle ways that would break the test suite. 5. We don't need to "cd" to the "$repo" directory at all anymore. In the code being removed here we both "cd"'d to the repository before calling "init", and did so in a subshell. It's not important to do either, so both of those can be removed. We cd'd because this code grew from test-lib.sh code where we'd have done so already, see eedf8f97e58 (Abstract test_create_repo out for use in tests., 2006-02-17), and later "cd"'d inside a subshell since 0d314ce834d to avoid having to keep track of an "old pwd" variable to cd back after the setup. Being in the repository directory made moving the hooks around easier (we wouldn't have to fully qualify the path). Since we're not moving the hooks per #4 above we don't need to "cd" for that reason either. 6. We can drop the --template argument and instead rely on the GIT_TEMPLATE_DIR set to the same path earlier in test-lib.sh. See 8683a45d669 (Introduce GIT_TEMPLATE_DIR, 2006-12-19) 7. We only needed that ">&3 2>&4" redirection when invoked from test-lib.sh. We could still invoke test_create_repo() there, but as the invocation is now trivial and we don't have a good reason to use test_create_repo() elsewhere let's call "git init" there ourselves. 8. We didn't need to resolve "git" as "${GIT_TEST_INSTALLED:-$GIT_EXEC_PATH}/git$X" in test_create_repo(), even for the use of test-lib.sh PATH is already set up in test-lib.sh to start with GIT_TEST_INSTALLED and/or GIT_EXEC_PATH before test_create_repo() (now "git init") is called.. So we can simply run "git" and rely on the PATH lookup choosing the right executable. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>