From: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
"Phillip Wood" <phillip.wood123@gmail.com>,
"Victoria Dye" <vdye@github.com>,
"Eric Sunshine" <ericsunshine@gmail.com>,
"Johannes Schindelin" <johannes.schindelin@gmx.de>
Subject: [PATCH v3 0/5] Some fixes and an improvement for using CTest on Windows
Date: Tue, 18 Oct 2022 10:59:00 +0000 [thread overview]
Message-ID: <pull.1320.v3.git.1666090745.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1320.v2.git.1661243463.gitgitgadget@gmail.com>
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@gmx.de>
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
next prev parent reply other threads:[~2022-10-18 10:59 UTC|newest]
Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-10 15:02 [PATCH 0/5] Some fixes and an improvement for using CTest on Windows Johannes Schindelin via GitGitGadget
2022-08-10 15:02 ` [PATCH 1/5] cmake: align CTest definition with Git's CI runs Johannes Schindelin via GitGitGadget
2022-08-10 17:48 ` Junio C Hamano
2022-08-16 10:11 ` Johannes Schindelin
2022-08-16 15:15 ` Junio C Hamano
2022-08-19 13:57 ` Johannes Schindelin
2022-08-11 11:18 ` Ævar Arnfjörð Bjarmason
2022-08-10 15:02 ` [PATCH 2/5] cmake: copy the merge tools for testing Johannes Schindelin via GitGitGadget
2022-08-10 15:02 ` [PATCH 3/5] tests: explicitly skip `chmod` calls on Windows Johannes Schindelin via GitGitGadget
2022-08-11 11:22 ` Ævar Arnfjörð Bjarmason
2022-08-22 10:19 ` Johannes Schindelin
2022-08-23 7:34 ` Johannes Schindelin
2022-08-10 15:02 ` [PATCH 4/5] add -p: avoid ambiguous signed/unsigned comparison Johannes Schindelin via GitGitGadget
2022-08-10 17:54 ` Junio C Hamano
2022-08-16 9:56 ` Johannes Schindelin
2022-08-16 15:10 ` Junio C Hamano
2022-08-19 14:52 ` Johannes Schindelin
2022-08-11 12:49 ` Phillip Wood
2022-08-16 10:00 ` Johannes Schindelin
2022-08-16 14:23 ` Phillip Wood
2022-08-19 14:07 ` Johannes Schindelin
2022-08-10 15:02 ` [PATCH 5/5] cmake: avoid editing t/test-lib.sh Johannes Schindelin via GitGitGadget
2022-08-11 11:35 ` Ævar Arnfjörð Bjarmason
2022-10-18 14:02 ` Johannes Schindelin
2022-08-11 12:58 ` Phillip Wood
2022-08-16 10:09 ` Johannes Schindelin
2022-08-16 14:27 ` Phillip Wood
2022-08-23 8:30 ` [PATCH v2 0/5] Some fixes and an improvement for using CTest on Windows Johannes Schindelin via GitGitGadget
2022-08-23 8:30 ` [PATCH v2 1/5] cmake: make it easier to diagnose regressions in CTest runs Johannes Schindelin via GitGitGadget
2022-09-07 22:10 ` Victoria Dye
2022-10-18 14:02 ` Johannes Schindelin
2022-09-08 7:22 ` Ævar Arnfjörð Bjarmason
2022-09-28 6:55 ` Eric Sunshine
2022-08-23 8:31 ` [PATCH v2 2/5] cmake: copy the merge tools for testing Johannes Schindelin via GitGitGadget
2022-08-23 8:31 ` [PATCH v2 3/5] add -p: avoid ambiguous signed/unsigned comparison Johannes Schindelin via GitGitGadget
2022-08-23 8:31 ` [PATCH v2 4/5] cmake: avoid editing t/test-lib.sh Johannes Schindelin via GitGitGadget
2022-09-08 7:39 ` Ævar Arnfjörð Bjarmason
2022-10-18 14:03 ` Johannes Schindelin
2022-10-18 15:09 ` Ævar Arnfjörð Bjarmason
2022-09-08 23:37 ` Victoria Dye
2022-09-08 23:42 ` Victoria Dye
2022-09-08 23:58 ` Junio C Hamano
2022-10-18 14:03 ` Johannes Schindelin
2022-08-23 8:31 ` [PATCH v2 5/5] cmake: increase time-out for a long-running test Johannes Schindelin via GitGitGadget
2022-09-08 7:34 ` Ævar Arnfjörð Bjarmason
2022-09-08 17:29 ` Victoria Dye
2022-09-08 3:51 ` [PATCH v2 0/5] Some fixes and an improvement for using CTest on Windows Victoria Dye
2022-10-18 10:59 ` Johannes Schindelin via GitGitGadget [this message]
2022-10-18 10:59 ` [PATCH v3 1/5] cmake: make it easier to diagnose regressions in CTest runs Johannes Schindelin via GitGitGadget
2022-10-18 13:41 ` Ævar Arnfjörð Bjarmason
2022-10-18 10:59 ` [PATCH v3 2/5] cmake: copy the merge tools for testing Johannes Schindelin via GitGitGadget
2022-10-18 13:49 ` Ævar Arnfjörð Bjarmason
2022-10-18 10:59 ` [PATCH v3 3/5] add -p: avoid ambiguous signed/unsigned comparison Johannes Schindelin via GitGitGadget
2022-10-18 13:53 ` Ævar Arnfjörð Bjarmason
2022-10-18 10:59 ` [PATCH v3 4/5] cmake: avoid editing t/test-lib.sh Johannes Schindelin via GitGitGadget
2022-10-18 13:54 ` Ævar Arnfjörð Bjarmason
2022-10-18 14:21 ` Johannes Schindelin
2022-10-18 10:59 ` [PATCH v3 5/5] cmake: increase time-out for a long-running test Johannes Schindelin via GitGitGadget
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=pull.1320.v3.git.1666090745.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=avarab@gmail.com \
--cc=ericsunshine@gmail.com \
--cc=git@vger.kernel.org \
--cc=johannes.schindelin@gmx.de \
--cc=phillip.wood123@gmail.com \
--cc=vdye@github.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.