diff mbox series

[v5,11/15] cmake: support GIT_TEST_OPTS, abstract away WIN32 defaults

Message ID patch-v5-11.15-a5dd4fa1d00-20221202T110947Z-avarab@gmail.com (mailing list archive)
State Superseded
Headers show
Series cmake: document, fix on *nix, add CI | expand

Commit Message

Ævar Arnfjörð Bjarmason Dec. 2, 2022, 11:28 a.m. UTC
The rationale for adding "--no-bin-wrappers" and "--no-chain-lint" in
2ea1d8b5563 (cmake: make it easier to diagnose regressions in CTest
runs, 2022-10-18) was those options slowed down the tests considerably
on Windows.

But since f31b6244950 (Merge branch 'yw/cmake-updates', 2022-06-07)
and with the preceding commits cmake and ctest are not
Windows-specific anymore.

So let's set those same options by default on Windows, but do so with
the set(... CACHE <type> <docstring>) facility. As noted in cmake's
documentation[1] this integrates nicely with e.g. cmake-gui.

On *nix we don't set any custom options. The change in 2ea1d8b5563
didn't discuss why Windows should have divergent defaults with "cmake"
and "make", but such reasons presumably don't apply on *nix. I for one
am happy with the same defaults as the tests have when running via the
Makefile.

With the "message()" addition we'll emit this when running cmake:

	Generating hook-list.h
	-- Using user-selected test options: -vixd
	-- Configuring done
	-- Generating done
	-- Build files have been written to: /home/avar/g/git/contrib/buildsystems/out

Unfortunately cmake doesn't support a non-hacky way to pass variables
to ctest without re-running cmake itself, so when re-running tests via
cmake and wanting to change the test defaults we'll need:

	GIT_TEST_OPTS=-i cmake -S contrib/buildsystems -B contrib/buildsystems/out &&
	ctest --jobs=$(nproc) --test-dir contrib/buildsystems/out -R t0071 --verbose

The "separate_arguments()" here will do the right thing for arguments
that don't contain whitespace, so e.g. the path to --root="" can't
have a space in it. There's supposedly a way to work around that with
separate_arguments(), but it requires features newer than our required
cmake version, so let's live with that edge case for now.

1. https://cmake.org/cmake/help/latest/command/set.html#set-cache-entry

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/buildsystems/CMakeLists.txt | 50 +++++++++++++++++++++++++++--
 1 file changed, 48 insertions(+), 2 deletions(-)

Comments

Eric Sunshine Dec. 3, 2022, 5:38 a.m. UTC | #1
On Fri, Dec 2, 2022 at 6:28 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> +       message(STATUS "Using Windowns-specific default test options: ${GIT_TEST_OPTS}")

s/Windowns/Windows/
Ævar Arnfjörð Bjarmason Dec. 3, 2022, 1:10 p.m. UTC | #2
On Sat, Dec 03 2022, Eric Sunshine wrote:

> On Fri, Dec 2, 2022 at 6:28 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
>> +       message(STATUS "Using Windowns-specific default test options: ${GIT_TEST_OPTS}")
>
> s/Windowns/Windows/

Thanks, the rest of this is just typos, but maybe this one's a Freudian slip :)
Eric Sunshine Dec. 3, 2022, 4:58 p.m. UTC | #3
On Sat, Dec 3, 2022 at 8:11 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> On Sat, Dec 03 2022, Eric Sunshine wrote:
> > On Fri, Dec 2, 2022 at 6:28 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> >> +       message(STATUS "Using Windowns-specific default test options: ${GIT_TEST_OPTS}")
> >
> > s/Windowns/Windows/
>
> Thanks, the rest of this is just typos, but maybe this one's a Freudian slip :)

I had the same thought and nearly said so.
diff mbox series

Patch

diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index 866bdc1914a..cfe8126d939 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -32,7 +32,7 @@  To use this in Visual Studio:
 
 Open the worktree as a folder. Visual Studio 2019 and later will detect
 the CMake configuration automatically and set everything up for you,
-ready to build. You can then run the tests in `t/` via a regular Git Bash.
+ready to build. See "== Running the tests ==" below for running the tests.
 
 Note: Visual Studio also has the option of opening `CMakeLists.txt`
 directly; Using this option, Visual Studio will not find the source code,
@@ -76,6 +76,37 @@  empty(default) :
 
 NOTE: -DCMAKE_BUILD_TYPE is optional. For multi-config generators like Visual Studio
 this option is ignored
+
+== Running the tests ==
+
+Once we've built in "contrib/buildsystems/out" the tests can be run at
+the top-level (note: not the generated "contrib/buildsystems/out/t/"
+drectory). If no top-level build is found (as created with the
+Makefile) the t/test-lib.sh will discover the git in
+"contrib/buildsystems/out" on e.g.:
+
+	(cd t && ./t0001-init.sh)
+	setup: had no ../git, but found & used cmake built git in ../contrib/buildsystems/out/git
+	[...]
+
+The tests can also be run with ctest, e.g. after building with "cmake"
+and "make" or "msbuild" run, from the top-level e.g.:
+
+	# "--test-dir" is new in cmake v3.20, so "(cd
+	# contrib/buildsystems/out && ctest ...)" on older versions.
+	ctest --test-dir contrib/buildsystems/out --jobs="$(nproc)"--output-on-failure
+
+Options can be passed by setting GIT_TEST_OPTIONS before invoking
+cmake. E.g. on a Linux system with systemd the tests can be sped up by
+using a ramdisk for the scratch files:
+
+	GIT_TEST_OPTS="--root=/dev/shm/$(id -u)/ctest" cmake -S contrib/buildsystems -B contrib/buildsystems/out
+	[...]
+	-- Using user-selected test options: --root=/dev/shm/<uid>/ctest
+
+Then running the tests with "ctest" (here with --jobs="$(nproc)"):
+
+	ctest --jobs=$(nproc) --test-dir contrib/buildsystems/out
 ]]
 cmake_minimum_required(VERSION 3.14)
 
@@ -1146,10 +1177,25 @@  endif()
 
 file(GLOB test_scipts "${CMAKE_SOURCE_DIR}/t/t[0-9]*.sh")
 
+if(DEFINED ENV{GIT_TEST_OPTS})
+	set(GIT_TEST_OPTS "$ENV{GIT_TEST_OPTS}"
+		CACHE STRING "test options, see t/README")
+	message(STATUS "Using user-selected test options: ${GIT_TEST_OPTS}")
+elseif(WIN32)
+	set(GIT_TEST_OPTS "--no-bin-wrappers --no-chain-lint -vx"
+		CACHE STRING "test options, see t/README")
+	message(STATUS "Using Windowns-specific default test options: ${GIT_TEST_OPTS}")
+else()
+	set(GIT_TEST_OPTS ""
+		CACHE STRING "test options, see t/README")
+	message(STATUS "No custom test options selected, set e.g. GIT_TEST_OPTS=\"-vixd\"")
+endif()
+separate_arguments(GIT_TEST_OPTS)
+
 #test
 foreach(tsh ${test_scipts})
 	add_test(NAME ${tsh}
-		COMMAND ${SH_EXE} ${tsh} --no-bin-wrappers --no-chain-lint -vx
+		COMMAND ${SH_EXE} ${tsh} ${GIT_TEST_OPTS}
 		WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/t)
 if(NOT GIT_CTEST_SETS_BUILD_DIR)
 	set_property(TEST ${tsh} APPEND PROPERTY ENVIRONMENT