From a15d4465a9918911e0953e89276d563fdc8cceef Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 25 Sep 2023 11:20:30 +0000 Subject: [PATCH 1/7] cmake: also build unit tests A new, better way to run unit tests was just added to Git. This adds support for building those unit tests via CMake. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- contrib/buildsystems/CMakeLists.txt | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt index 2f6e0197ffa489..ffeca1a53f9ac1 100644 --- a/contrib/buildsystems/CMakeLists.txt +++ b/contrib/buildsystems/CMakeLists.txt @@ -965,6 +965,24 @@ target_link_libraries(test-fake-ssh common-main) parse_makefile_for_sources(test-reftable_SOURCES "REFTABLE_TEST_OBJS") list(TRANSFORM test-reftable_SOURCES PREPEND "${CMAKE_SOURCE_DIR}/") +#unit-tests +add_library(unit-test-lib OBJECT ${CMAKE_SOURCE_DIR}/t/unit-tests/test-lib.c) + +parse_makefile_for_scripts(unit_test_PROGRAMS "UNIT_TEST_PROGRAMS" "") +foreach(unit_test ${unit_test_PROGRAMS}) + add_executable("${unit_test}" "${CMAKE_SOURCE_DIR}/t/unit-tests/${unit_test}.c") + target_link_libraries("${unit_test}" unit-test-lib common-main) + set_target_properties("${unit_test}" + PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/t/unit-tests/bin) + if(MSVC) + set_target_properties("${unit_test}" + PROPERTIES RUNTIME_OUTPUT_DIRECTORY_DEBUG ${CMAKE_BINARY_DIR}/t/unit-tests/bin) + set_target_properties("${unit_test}" + PROPERTIES RUNTIME_OUTPUT_DIRECTORY_RELEASE ${CMAKE_BINARY_DIR}/t/unit-tests/bin) + endif() + list(APPEND PROGRAMS_BUILT "${unit_test}") +endforeach() + #test-tool parse_makefile_for_sources(test-tool_SOURCES "TEST_BUILTINS_OBJS") From 0df903d402959e052b6fa927dfe447d92b81eaef Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 25 Sep 2023 11:20:31 +0000 Subject: [PATCH 2/7] unit-tests: do not mistake `.pdb` files for being executable When building the unit tests via CMake, the `.pdb` files are built. Those are, essentially, files containing the debug information separately from the executables. Let's not confuse them with the executables we actually want to run. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/Makefile b/t/Makefile index 75d93304370db3..225aaf78edc7cf 100644 --- a/t/Makefile +++ b/t/Makefile @@ -42,7 +42,7 @@ TPERF = $(sort $(wildcard perf/p[0-9][0-9][0-9][0-9]-*.sh)) TINTEROP = $(sort $(wildcard interop/i[0-9][0-9][0-9][0-9]-*.sh)) CHAINLINTTESTS = $(sort $(patsubst chainlint/%.test,%,$(wildcard chainlint/*.test))) CHAINLINT = '$(PERL_PATH_SQ)' chainlint.pl -UNIT_TESTS = $(sort $(filter-out unit-tests/bin/t-basic%,$(wildcard unit-tests/bin/t-*))) +UNIT_TESTS = $(sort $(filter-out %.pdb unit-tests/bin/t-basic%,$(wildcard unit-tests/bin/t-*))) # `test-chainlint` (which is a dependency of `test-lint`, `test` and `prove`) # checks all tests in all scripts via a single invocation, so tell individual From a2c5e294dbb095023e6c0d53b302c4363021f1b2 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 25 Sep 2023 11:20:32 +0000 Subject: [PATCH 3/7] unit-tests: do show relative file paths Visual C interpolates `__FILE__` with the absolute _Windows_ path of the source file. GCC interpolates it with the relative path, and the tests even verify that. So let's make sure that the unit tests only emit such paths. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/unit-tests/test-lib.c | 52 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 48 insertions(+), 4 deletions(-) diff --git a/t/unit-tests/test-lib.c b/t/unit-tests/test-lib.c index a2cc21c706b3bc..7bf9dfdb959551 100644 --- a/t/unit-tests/test-lib.c +++ b/t/unit-tests/test-lib.c @@ -21,6 +21,46 @@ static struct { .result = RESULT_NONE, }; +#ifndef _MSC_VER +#define make_relative(location) location +#else +/* + * Visual C interpolates the absolute Windows path for `__FILE__`, + * but we want to see relative paths, as verified by t0080. + */ +#include "dir.h" + +static const char *make_relative(const char *location) +{ + static char prefix[] = __FILE__, buf[PATH_MAX], *p; + static size_t prefix_len; + + if (!prefix_len) { + size_t len = strlen(prefix); + const char *needle = "\\t\\unit-tests\\test-lib.c"; + size_t needle_len = strlen(needle); + + if (len < needle_len || strcmp(needle, prefix + len - needle_len)) + die("unexpected suffix of '%s'", prefix); + + /* let it end in a directory separator */ + prefix_len = len - needle_len + 1; + } + + /* Does it not start with the expected prefix? */ + if (fspathncmp(location, prefix, prefix_len)) + return location; + + strlcpy(buf, location + prefix_len, sizeof(buf)); + /* convert backslashes to forward slashes */ + for (p = buf; *p; p++) + if (*p == '\\') + *p = '/'; + + return buf; +} +#endif + static void msg_with_prefix(const char *prefix, const char *format, va_list ap) { fflush(stderr); @@ -147,7 +187,8 @@ int test__run_end(int was_run UNUSED, const char *location, const char *format, break; case RESULT_NONE: - test_msg("BUG: test has no checks at %s", location); + test_msg("BUG: test has no checks at %s", + make_relative(location)); printf("not ok %d", ctx.count); print_description(format, ap); ctx.result = RESULT_FAILURE; @@ -193,14 +234,16 @@ int test_assert(const char *location, const char *check, int ok) assert(ctx.running); if (ctx.result == RESULT_SKIP) { - test_msg("skipping check '%s' at %s", check, location); + test_msg("skipping check '%s' at %s", check, + make_relative(location)); return 1; } if (!ctx.todo) { if (ok) { test_pass(); } else { - test_msg("check \"%s\" failed at %s", check, location); + test_msg("check \"%s\" failed at %s", check, + make_relative(location)); test_fail(); } } @@ -225,7 +268,8 @@ int test__todo_end(const char *location, const char *check, int res) if (ctx.result == RESULT_SKIP) return 1; if (res) { - test_msg("todo check '%s' succeeded at %s", check, location); + test_msg("todo check '%s' succeeded at %s", check, + make_relative(location)); test_fail(); } else { test_todo(); From ca76cca3a6e85311b1518d4e585b28b8177570bc Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 25 Sep 2023 11:20:33 +0000 Subject: [PATCH 4/7] artifacts-tar: when including `.dll` files, don't forget the unit-tests As of recent, Git also builds executables in `t/unit-tests/`. For technical reasons, when building with CMake and Visual C, the dependencies (".dll files") need to be copied there, too, otherwise running the executable will fail "due to missing dependencies". The CMake definition already contains the directives to copy those `.dll` files, but we also need to adjust the `artifacts-tar` rule in the `Makefile` accordingly to let the `vs-test` job in the CI runs pass successfully. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 18c13f06c03a2c..e15c34e506a2ae 100644 --- a/Makefile +++ b/Makefile @@ -3597,7 +3597,7 @@ rpm:: .PHONY: rpm ifneq ($(INCLUDE_DLLS_IN_ARTIFACTS),) -OTHER_PROGRAMS += $(shell echo *.dll t/helper/*.dll) +OTHER_PROGRAMS += $(shell echo *.dll t/helper/*.dll t/unit-tests/bin/*.dll) endif artifacts-tar:: $(ALL_COMMANDS_TO_INSTALL) $(SCRIPT_LIB) $(OTHER_PROGRAMS) \ From 5bd7fb49afbe315c08102fea3536823068fad46d Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 25 Sep 2023 11:20:34 +0000 Subject: [PATCH 5/7] cmake: fix typo in variable name Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- contrib/buildsystems/CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt index ffeca1a53f9ac1..54bdf5f3fe9f90 100644 --- a/contrib/buildsystems/CMakeLists.txt +++ b/contrib/buildsystems/CMakeLists.txt @@ -1102,10 +1102,10 @@ if(NOT ${CMAKE_BINARY_DIR}/CMakeCache.txt STREQUAL ${CACHE_PATH}) file(COPY ${CMAKE_SOURCE_DIR}/contrib/completion/git-completion.bash DESTINATION ${CMAKE_BINARY_DIR}/contrib/completion/) endif() -file(GLOB test_scipts "${CMAKE_SOURCE_DIR}/t/t[0-9]*.sh") +file(GLOB test_scripts "${CMAKE_SOURCE_DIR}/t/t[0-9]*.sh") #test -foreach(tsh ${test_scipts}) +foreach(tsh ${test_scripts}) add_test(NAME ${tsh} COMMAND ${SH_EXE} ${tsh} --no-bin-wrappers --no-chain-lint -vx WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/t) From 2f2729f3a4626838fce294a0cec4e525c5f4be39 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 25 Sep 2023 11:20:35 +0000 Subject: [PATCH 6/7] cmake: use test names instead of full paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The primary purpose of Git's CMake definition is to allow developing Git in Visual Studio. As part of that, the CTest feature allows running individual test scripts conveniently in Visual Studio's Test Explorer. However, this Test Explorer's design targets object-oriented languages and therefore expects the test names in the form `..`. And since we specify the full path of the test scripts instead, including the ugly `/.././t/` part, these dots confuse the Test Explorer and it uses a large part of the path as "namespace". Let's just use `t.suite.` instead. This presents the tests in Visual Studio's Test Explorer in the following form by default (i.e. unless the user changes the view via the "Group by" menu): ◢ ◈ git ◢ ◈ t ◢ ◈ suite ◈ t0000-basic ◈ t0001-init ◈ t0002-gitfile [...] Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- contrib/buildsystems/CMakeLists.txt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt index 54bdf5f3fe9f90..356f068cacd0f3 100644 --- a/contrib/buildsystems/CMakeLists.txt +++ b/contrib/buildsystems/CMakeLists.txt @@ -1106,13 +1106,14 @@ file(GLOB test_scripts "${CMAKE_SOURCE_DIR}/t/t[0-9]*.sh") #test foreach(tsh ${test_scripts}) - add_test(NAME ${tsh} + string(REGEX REPLACE ".*/(.*)\\.sh" "\\1" test_name ${tsh}) + add_test(NAME "t.suite.${test_name}" COMMAND ${SH_EXE} ${tsh} --no-bin-wrappers --no-chain-lint -vx WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/t) endforeach() # This test script takes an extremely long time and is known to time out even # on fast machines because it requires in excess of one hour to run -set_tests_properties("${CMAKE_SOURCE_DIR}/t/t7112-reset-submodule.sh" PROPERTIES TIMEOUT 4000) +set_tests_properties("t.suite.t7112-reset-submodule" PROPERTIES TIMEOUT 4000) endif()#BUILD_TESTING From 694e89baeb824a173b655507b7b62174d2d15688 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 25 Sep 2023 11:20:36 +0000 Subject: [PATCH 7/7] cmake: handle also unit tests The unit tests should also be available e.g. in Visual Studio's Test Explorer when configuring Git's source code via CMake. Suggested-by: Phillip Wood Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- contrib/buildsystems/CMakeLists.txt | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt index 356f068cacd0f3..671c7ead75381d 100644 --- a/contrib/buildsystems/CMakeLists.txt +++ b/contrib/buildsystems/CMakeLists.txt @@ -981,6 +981,17 @@ foreach(unit_test ${unit_test_PROGRAMS}) PROPERTIES RUNTIME_OUTPUT_DIRECTORY_RELEASE ${CMAKE_BINARY_DIR}/t/unit-tests/bin) endif() list(APPEND PROGRAMS_BUILT "${unit_test}") + + # t-basic intentionally fails tests, to validate the unit-test infrastructure. + # Therefore, it should only be run as part of t0080, which verifies that it + # fails only in the expected ways. + # + # All other unit tests should be run. + if(NOT ${unit_test} STREQUAL "t-basic") + add_test(NAME "t.unit-tests.${unit_test}" + COMMAND "./${unit_test}" + WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/t/unit-tests/bin) + endif() endforeach() #test-tool