Re: [PATCH v2 1/5] cmake: make it easier to diagnose regressions in CTest runs
From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2022-09-08 07:29:01
On Tue, Aug 23 2022, Johannes Schindelin via GitGitGadget wrote:
quoted hunk ↗ jump to hunk
From: Johannes Schindelin <redacted> When a test script fails in Git's test suite, the usual course of action is to re-run it using options to increase the verbosity of the output, e.g. `-v` and `-x`. 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. 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 information necessary to figure out test failures. 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. Signed-off-by: Johannes Schindelin <redacted> --- contrib/buildsystems/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt index 1b23f2440d8..4aee1e24342 100644 --- a/contrib/buildsystems/CMakeLists.txt +++ b/contrib/buildsystems/CMakeLists.txt@@ -1088,7 +1088,7 @@ file(GLOB test_scipts "${CMAKE_SOURCE_DIR}/t/t[0-9]*.sh") #test foreach(tsh ${test_scipts}) add_test(NAME ${tsh} - COMMAND ${SH_EXE} ${tsh} + COMMAND ${SH_EXE} ${tsh} --no-bin-wrappers --no-chain-lint -vx WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/t) endforeach()
The -vx part of this looks sensible, and matches the commit message
$subject.
I think the "--no-bin-wrappers --no-chain-lint" and the "while at it"
here should be stripped out, and put into its own commit.
Which, as I commented on in the v1[1] is making the implicit assumption
that this cmake file is only used on Windows, but since 561962479c
(cmake: fix CMakeLists.txt on Linux, 2022-05-24) that isn't the case.
So, perhaps we should have a performance hack due to Windows's slowness,
but:
A. It should be guarded by some "is windows?" check. Your commit
message justifies why this is a good idea on Windows, but completely
elides over the fact that this isn't Windows-specific code anymore.
B. We can still build wind "make" or "cmake", I don't see a reason for
why such a perf hack (if we decide to keep it) should depend on the
build system.
C. Since I sent [1] we've had submitted chainlint.pl in-flight
series. It's partly trying to take special considerations to be fast
on Windows. I don't think it's the case with that series that this
needs to be skipped on Windows anymore (and if it is, Eric would
like to know).
1. https://lore.kernel.org/git/220811.861qtnqb5p.gmgdl@evledraar.gmail.com/ (local)