Thread (49 messages) 49 messages, 6 authors, 2022-10-18

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)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help