All of lore.kernel.org
 help / color / mirror / Atom feed
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>,
	"Johannes Schindelin" <johannes.schindelin@gmx.de>
Subject: [PATCH v2 0/5] Some fixes and an improvement for using CTest on Windows
Date: Tue, 23 Aug 2022 08:30:58 +0000	[thread overview]
Message-ID: <pull.1320.v2.git.1661243463.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1320.git.1660143750.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 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                       | 11 ++++++++++-
 5 files changed, 21 insertions(+), 10 deletions(-)


base-commit: bbea4dcf42b28eb7ce64a6306cdde875ae5d09ca
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1320%2Fdscho%2Fctest-on-windows-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1320/dscho/ctest-on-windows-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1320

Range-diff vs v1:

 1:  9cf14984c0a ! 1:  e00cb37b98a cmake: align CTest definition with Git's CI runs
     @@ Metadata
      Author: Johannes Schindelin <Johannes.Schindelin@gmx.de>
      
       ## Commit message ##
     -    cmake: align CTest definition with Git's CI runs
     +    cmake: make it easier to diagnose regressions in CTest runs
      
     -    In Git's CI runs, the Windows tests are run with `--no-bin-wrappers` and
     -    `--no-chain-lint`, mainly to win back some time caused by the serious
     -    performance penalty paid for the tests relying so heavily on POSIX shell
     -    scripting, which only works by using a POSIX emulation layer.
     +    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`.
      
     -    Let's do the same when running the tests, say, in Visual Studio.
     +    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.
      
     -    While at it, enable the command trace via `-x` and verbose output via
     -    `-v`, otherwise it would be near impossible to diagnose any problems.
     +    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 <johannes.schindelin@gmx.de>
      
 2:  86ab58b6508 = 2:  de7b47a9aa7 cmake: copy the merge tools for testing
 3:  79abfa82c32 < -:  ----------- tests: explicitly skip `chmod` calls on Windows
 4:  4d24a4345ba ! 3:  f96d5ab484c add -p: avoid ambiguous signed/unsigned comparison
     @@ Commit message
          Let's avoid that by converting the unsigned bit explicitly to a signed
          integer.
      
     +    Note: This is a long-standing bug in the Visual C build of Git, but it
     +    has never been caught because t3701 is skipped when `NO_PERL` is set,
     +    which is the case in the `vs-test` jobs of Git's CI runs.
     +
          Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
      
       ## add-patch.c ##
 5:  c7fc5a4ee4c = 4:  22473d6b8f3 cmake: avoid editing t/test-lib.sh
 -:  ----------- > 5:  6aaa675301c cmake: increase time-out for a long-running test

-- 
gitgitgadget

  parent reply	other threads:[~2022-08-23  9:12 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 ` Johannes Schindelin via GitGitGadget [this message]
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   ` [PATCH v3 " Johannes Schindelin via GitGitGadget
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.v2.git.1661243463.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=johannes.schindelin@gmx.de \
    --cc=phillip.wood123@gmail.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.