Visual Studio users enjoy support for running the test suite via CTest, thanks to Git's CMake definition. In https://github.com/git-for-windows/git/issues/3966, it has been reported that this does not work out of the box, though, but causes a couple of test failures instead. These problems are not caught by Git's CI runs because the vs-tests jobs actually use prove to run the test suite, not CTest. In addition to fixing these problems, this patch series also addresses a long-standing gripe I have with the way Git's CMake definition supports CTest: It edits t/test-lib.sh, which leaves this file eternally modified (but these modification should never be committed, they refer to a local-only, configuration-dependent directory). Note: The signed/unsigned comparison bug in git add -p that is fixed in this here patch series is a relatively big one, and it merits further investigation whether there are similar bugs lurking in Git's code base. However, this is a much bigger project than can be addressed as part of this patch series, in particular because the analysis would require tools other than GCC's -Wsign-compare option (which totally misses the instance that is fixed in this here patch series). Changes since v2: * Enhanced a commit message to clarify how the "cumbersome approach" looks like when a developer wishes to pass options to the test scripts in Visual Studio. * Changed the base branch to ac/fuzzers, to avoid merge conflicts in .gitignore. * Moved the PANIC check in test-lib.sh so that it is not skipped when the file GIT-BUILD-DIR exists. Changes since v1: * Clarified why it is a good idea to pass --no-bin-wrappers and --no-chain-lint when running on Windows. * Clarified why the add -p bug has not been caught earlier. * Clarified the scope of this patch series to fix running Git's tests within Visual Studio. * Increased the time-out for the very slow t7112 test script. * The test_chmod was determined to be not only faulty, but unneeded, and was dropped. Johannes Schindelin (5): cmake: make it easier to diagnose regressions in CTest runs cmake: copy the merge tools for testing add -p: avoid ambiguous signed/unsigned comparison cmake: avoid editing t/test-lib.sh cmake: increase time-out for a long-running test .gitignore | 1 + Makefile | 1 + add-patch.c | 2 +- contrib/buildsystems/CMakeLists.txt | 16 ++++++++-------- t/test-lib.sh | 10 ++++++++++ 5 files changed, 21 insertions(+), 9 deletions(-) base-commit: 6713bfc70c4dc6da1fa4084f000b72f5d74fecfb Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1320%2Fdscho%2Fctest-on-windows-v3 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1320/dscho/ctest-on-windows-v3 Pull-Request: https://github.com/gitgitgadget/git/pull/1320 Range-diff vs v2: 1: e00cb37b98a ! 1: 356b2e9a100 cmake: make it easier to diagnose regressions in CTest runs @@ Commit message Like in Git's CI runs, when running the tests in Visual Studio via the CTest route, it is cumbersome or at least requires a very unintuitive - approach to pass options to the test scripts. + approach to pass options to the test scripts: the CMakeLists.txt file + would have to be modified, passing the desired options to _all_ test + scripts, and then the CMake Cache would have to be reconfigured before + running the test in question individually. Unintuitive at best, and + opposite to the niceties IDE users expect. So let's just pass those options by default: This will not clutter any output window but the log that is written to a log file will have @@ Commit message While at it, also imitate what the Windows jobs in Git's CI runs do to accelerate running the test scripts: pass the `--no-bin-wrappers` and - `--no-chain-lint` options. This makes the test runs noticeably faster - because the `bin-wrappers/` scripts as well as the `chain-lint` code - make heavy use of POSIX shell scripting, which is really, really slow on - Windows due to the need to emulate POSIX behavior via the MSYS2 runtime. + `--no-chain-lint` options. + + This makes the test runs noticeably faster because the `bin-wrappers/` + scripts as well as the `chain-lint` code make heavy use of POSIX shell + scripting, which is really, really slow on Windows due to the need to + emulate POSIX behavior via the MSYS2 runtime. In a test by Eric + Sunshine, it added two minutes (!) just to perform the chain-lint task. + + The idea of adding a CMake config option (á la `GIT_TEST_OPTS`) was + considered during the development of this patch, but then dropped: such + a setting is global, across _all_ tests, where e.g. `--run=...` would + not make sense. Users wishing to override these new defaults are better + advised running the test script manually, in a Git Bash, with full + control over the command line. Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx> 2: de7b47a9aa7 = 2: 9faca9d5cbe cmake: copy the merge tools for testing 3: f96d5ab484c = 3: 41a8021a4bd add -p: avoid ambiguous signed/unsigned comparison 4: 22473d6b8f3 ! 4: 5b0c2a150e9 cmake: avoid editing t/test-lib.sh @@ Commit message ## .gitignore ## @@ /fuzz_corpora - /fuzz-pack-headers - /fuzz-pack-idx +/GIT-BUILD-DIR /GIT-BUILD-OPTIONS /GIT-CFLAGS @@ contrib/buildsystems/CMakeLists.txt: endif() ## t/test-lib.sh ## @@ t/test-lib.sh: then - TEST_OUTPUT_DIRECTORY=$TEST_DIRECTORY + echo "PANIC: Running in a $TEST_DIRECTORY that doesn't end in '/t'?" >&2 + exit 1 fi - GIT_BUILD_DIR="${TEST_DIRECTORY%/t}" --if test "$TEST_DIRECTORY" = "$GIT_BUILD_DIR" +if test -f "$GIT_BUILD_DIR/GIT-BUILD-DIR" +then + GIT_BUILD_DIR="$(cat "$GIT_BUILD_DIR/GIT-BUILD-DIR")" || exit 1 @@ t/test-lib.sh: then + GIT_BUILD_DIR="$(cygpath -au "$GIT_BUILD_DIR")" + ;; + esac -+elif test "$TEST_DIRECTORY" = "$GIT_BUILD_DIR" - then - echo "PANIC: Running in a $TEST_DIRECTORY that doesn't end in '/t'?" >&2 - exit 1 ++fi + + # Prepend a string to a VAR using an arbitrary ":" delimiter, not + # adding the delimiter if VAR or VALUE is empty. I.e. a generalized: 5: 6aaa675301c = 5: 40cf872f483 cmake: increase time-out for a long-running test -- gitgitgadget
