Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix CTest failures #3977

Merged
merged 8 commits into from
Nov 2, 2022
Merged

Fix CTest failures #3977

merged 8 commits into from
Nov 2, 2022

Conversation

dscho
Copy link
Member

@dscho dscho commented Aug 9, 2022

When building Git via Visual Studio and then running the tests via CTest (which is made very easy by Visual Studio), there are test failures. This PR intends to address those.

This closes #3966

@dscho dscho self-assigned this Aug 9, 2022
@dscho dscho force-pushed the fix-CTest-failures branch from d882026 to e051e0b Compare August 9, 2022 22:19
@dscho
Copy link
Member Author

dscho commented Aug 10, 2022

I think I'll want to wait for @taras-janea's confirmation that this fixes things also on their side, not only on mine, before merging it.

@PhilipOakley
Copy link

Minor commit message correction/comment.
In 951d328 "tests: explicitly skip chmod calls on Windows", the second reference to 'Linux' timings should probably be to 'Windows' as the latter is the much longer time.

@dscho
Copy link
Member Author

dscho commented Aug 10, 2022

Minor commit message correction/comment. In 951d328 "tests: explicitly skip chmod calls on Windows", the second reference to 'Linux' timings should probably be to 'Windows' as the latter is the much longer time.

Ooops, thank you so much, I appreciate the diligence!

@dscho dscho force-pushed the fix-CTest-failures branch from e051e0b to 17832ec Compare August 10, 2022 08:38
@dscho
Copy link
Member Author

dscho commented Aug 10, 2022

@PhilipOakley fixed in 79abfa8

@dscho dscho force-pushed the fix-CTest-failures branch 3 times, most recently from 295c61d to 89d704d Compare August 10, 2022 13:40
@dscho
Copy link
Member Author

dscho commented Aug 10, 2022

I submitted the already-upstreamable part in gitgitgadget#1320 and will wait for that to stabilize.

@dscho dscho force-pushed the fix-CTest-failures branch 3 times, most recently from 131a86b to 2c27ef4 Compare August 23, 2022 07:44
dscho added 8 commits October 28, 2022 15:21
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: 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
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. 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 <[email protected]>
Even when running the tests via CTest, t7609 and t7610 rely on more than
only a few mergetools to be copied to the build directory. Let's make it
so.

Signed-off-by: Johannes Schindelin <[email protected]>
In the interactive `add` operation, users can choose to jump to specific
hunks, and Git will present the hunk list in that case. To avoid showing
too many lines at once, only a maximum of 21 hunks are shown, skipping
the "mode change" pseudo hunk.

The comparison performed to skip the "mode change" pseudo hunk (if any)
compares a signed integer `i` to the unsigned value `mode_change` (which
can be 0 or 1 because it is a 1-bit type).

According to section 6.3.1.8 of the C99 standard (see e.g.
https://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf), what should
happen is an automatic conversion of the "lesser" type to the "greater"
type, but since the types differ in signedness, it is ill-defined what
is the correct "usual arithmetic conversion".

Which means that Visual C's behavior can (and does) differ from GCC's:
When compiling Git using the latter, `add -p`'s `goto` command shows no
hunks by default because it casts a negative start offset to a pretty
large unsigned value, breaking the "goto hunk" test case in
`t3701-add-interactive.sh`.

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 <[email protected]>
In 7f5397a (cmake: support for testing git when building out of the
source tree, 2020-06-26), we implemented support for running Git's test
scripts even after building Git in a different directory than the source
directory.

The way we did this was to edit the file `t/test-lib.sh` to override
`GIT_BUILD_DIR` to point somewhere else than the parent of the `t/`
directory.

This is unideal because it always leaves a tracked file marked as
modified, and it is all too easy to commit that change by mistake.

Let's change the strategy by teaching `t/test-lib.sh` to detect the
presence of a file called `GIT-BUILD-DIR` in the source directory. If it
exists, the contents are interpreted as the location to the _actual_
build directory. We then write this file as part of the CTest
definition.

To support building Git via a regular `make` invocation after building
it using CMake, we ensure that the `GIT-BUILD-DIR` file is deleted (for
convenience, this is done as part of the Makefile rule that is already
run with every `make` invocation to ensure that `GIT-BUILD-OPTIONS` is
up to date).

Signed-off-by: Johannes Schindelin <[email protected]>
As suggested in
git-for-windows#3966 (comment),
t7112 can run for well over one hour, which seems to be the default
maximum run time at least when running CTest-based tests in Visual
Studio.

Let's increase the time-out as a stop gap to unblock developers wishing
to run Git's test suite in Visual Studio.

Note: The actual run time is highly dependent on the circumstances. For
example, in Git's CI runs, the Windows-based tests typically take a bit
over 5 minutes to run. CI runs have the added benefit that Windows
Defender (the common anti-malware scanner on Windows) is turned off,
something many developers are not at liberty to do on their work
stations. When Defender is turned on, even on this developer's high-end
Ryzen system, t7112 takes over 15 minutes to run.

Signed-off-by: Johannes Schindelin <[email protected]>
In 7f5397a (cmake: support for testing git when building out of the
source tree, 2020-06-26), we implemented support for running Git's test
scripts even after building Git in a different directory than the source
directory.

The way we did this was to edit the file `t/test-lib.sh` to override
`GIT_BUILD_DIR` to point somewhere else than the parent of the `t/`
directory.

This is unideal because it always leaves a tracked file marked as
modified, and it is all too easy to commit that change by mistake.

Let's change the strategy by teaching `t/test-lib.sh` to detect the
presence of a file called `GIT-BUILD-DIR` in the source directory. If it
exists, the contents are interpreted as the location to the _actual_
build directory. We then write this file as part of the CTest
definition.

To support building Git via a regular `make` invocation after building
it using CMake, we ensure that the `GIT-BUILD-DIR` file is deleted (for
convenience, this is done as part of the Makefile rule that is already
run with every `make` invocation to ensure that `GIT-BUILD-OPTIONS` is
up to date).

Signed-off-by: Johannes Schindelin <[email protected]>
This is a backport of the patches that made it into core Git via
246eedf (Merge branch 'js/cmake-updates', 2022-10-27).

Signed-off-by: Johannes Schindelin <[email protected]>
When building Git via CMake on Windows, the dynamic libraries (`.dll`
files) on which Git's executables depend are copied into the build
directory, because the `.exe` files would not even load otherwise.

The "'MSYSTEM/PATH is adjusted if necessary'" test case of
`t0060-path-utils.sh` wants to copy said `.exe` files, though, and
expects them to run. Therefore, we need to copy the `.dll` files, too,
if there are any.

Signed-off-by: Johannes Schindelin <[email protected]>
@dscho dscho force-pushed the fix-CTest-failures branch from 2c27ef4 to 474668c Compare October 28, 2022 13:29
@dscho dscho merged commit 18da1d2 into git-for-windows:main Nov 2, 2022
@dscho dscho deleted the fix-CTest-failures branch November 2, 2022 13:53
@dscho dscho added this to the Next release milestone Nov 2, 2022
git-for-windows-ci pushed a commit that referenced this pull request Nov 2, 2022
When building Git via Visual Studio and then running the tests via CTest
(which is made very easy by Visual Studio), there are test failures.
This PR intends to address those.

This closes #3966
dscho added a commit that referenced this pull request Nov 3, 2022
When building Git via Visual Studio and then running the tests via CTest
(which is made very easy by Visual Studio), there are test failures.
This PR intends to address those.

This closes #3966
dscho added a commit that referenced this pull request Nov 3, 2022
When building Git via Visual Studio and then running the tests via CTest
(which is made very easy by Visual Studio), there are test failures.
This PR intends to address those.

This closes #3966
git-for-windows-ci pushed a commit that referenced this pull request Nov 3, 2022
When building Git via Visual Studio and then running the tests via CTest
(which is made very easy by Visual Studio), there are test failures.
This PR intends to address those.

This closes #3966
dscho added a commit that referenced this pull request Nov 3, 2022
When building Git via Visual Studio and then running the tests via CTest
(which is made very easy by Visual Studio), there are test failures.
This PR intends to address those.

This closes #3966
dscho added a commit that referenced this pull request Nov 3, 2022
When building Git via Visual Studio and then running the tests via CTest
(which is made very easy by Visual Studio), there are test failures.
This PR intends to address those.

This closes #3966
git-for-windows-ci pushed a commit that referenced this pull request Nov 3, 2022
When building Git via Visual Studio and then running the tests via CTest
(which is made very easy by Visual Studio), there are test failures.
This PR intends to address those.

This closes #3966
dscho added a commit that referenced this pull request Nov 3, 2022
When building Git via Visual Studio and then running the tests via CTest
(which is made very easy by Visual Studio), there are test failures.
This PR intends to address those.

This closes #3966
dscho added a commit that referenced this pull request Nov 3, 2022
When building Git via Visual Studio and then running the tests via CTest
(which is made very easy by Visual Studio), there are test failures.
This PR intends to address those.

This closes #3966
dscho added a commit that referenced this pull request Nov 4, 2022
When building Git via Visual Studio and then running the tests via CTest
(which is made very easy by Visual Studio), there are test failures.
This PR intends to address those.

This closes #3966
dscho added a commit that referenced this pull request Nov 4, 2022
When building Git via Visual Studio and then running the tests via CTest
(which is made very easy by Visual Studio), there are test failures.
This PR intends to address those.

This closes #3966
dscho added a commit that referenced this pull request Nov 9, 2022
When building Git via Visual Studio and then running the tests via CTest
(which is made very easy by Visual Studio), there are test failures.
This PR intends to address those.

This closes #3966
dscho added a commit that referenced this pull request Nov 10, 2022
When building Git via Visual Studio and then running the tests via CTest
(which is made very easy by Visual Studio), there are test failures.
This PR intends to address those.

This closes #3966
dscho added a commit that referenced this pull request Nov 15, 2022
When building Git via Visual Studio and then running the tests via CTest
(which is made very easy by Visual Studio), there are test failures.
This PR intends to address those.

This closes #3966
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

git tests in visual studio: 14 tests failed out of 966
2 participants