cmake: Use full signature of `add_test()` command #1289

pull hebasto wants to merge 1 commits into bitcoin-core:master from hebasto:230429-tests changing 3 files +7 −7

hebasto commented at 8:56 AM on April 29, 2023: member

This PR fixes tests for Windows binaries using Wine:

$ cmake -S . -B ../mingw -DCMAKE_TOOLCHAIN_FILE=cmake/x86_64-w64-mingw32.toolchain.cmake
$ cmake --build ../mingw
$ cmake --build ../mingw -t check
Test project /home/hebasto/git/secp256k1/mingw
    Start 1: noverify_tests
Could not find executable noverify_tests
...

cmake: Use full signature of `add_test()` command

An executable target in the `COMMAND` option will automatically be
replaced by the location of the executable created at build time.

This change fixes tests for Windows binaries using Wine.

755629bc03

hebasto cross-referenced this on Apr 29, 2023 from issue cmake: Set `ENVIRONMENT` property for examples on Windows by hebasto
real-or-random commented at 1:20 PM on April 29, 2023: contributor

Concept ACK

Mostly because I'm curious: Do you know why this fixes tests? After reading the docs in https://cmake.org/cmake/help/latest/command/add_test.html, I don't see how the full syntax should make a difference here.

Nevermind, I found it:

Unlike the above NAME signature, target names are not supported in the command-line.

in .cirrus.yml:406 in 755629bc03

 402 | @@ -403,7 +403,7 @@ task:
 403 |      - cmake --build build --config RelWithDebInfo -- -property:UseMultiToolTask=true;CL_MPcount=5
 404 |    check_script:
 405 |      - '%x64_NATIVE_TOOLS%'
 406 | -    - ctest --test-dir build -j 5
 407 | +    - ctest -C RelWithDebInfo --test-dir build -j 5

real-or-random commented at 1:24 PM on April 29, 2023:

Is this change necessary? Shouldn't that be the default already?

hebasto commented at 1:56 PM on April 29, 2023:

Is this change necessary?

It is.

Shouldn't that be the default already?

Otherwise, for multi-config generators, ctest complains:

Test not available without configuration.  (Missing "-C <config>"?)

real-or-random approved
real-or-random commented at 3:10 PM on April 29, 2023: contributor

ACK 755629bc032af155ff68fa820244d04cb80547c5
real-or-random merged this on Apr 29, 2023
real-or-random closed this on Apr 29, 2023
hebasto deleted the branch on Apr 29, 2023
sipa referenced this in commit b4eb644b6c on May 12, 2023
hebasto referenced this in commit 49c52ea2b1 on May 13, 2023
vmta referenced this in commit e1120c94a1 on Jun 4, 2023
sipa referenced this in commit 901336eee7 on Jun 21, 2023
vmta referenced this in commit 8f03457eed on Jul 1, 2023
jonasnick cross-referenced this on Jul 24, 2023 from issue Upstream PRs 1268, 1276, 1267, 1265, 1230, 1279, 1273, 1263, 1231, 1285, 1283, 1205, 1286, 1275, 1234, 1239, 1240, 1284, 1277, 1289, 1270, 1296, 1301, 1299, 1066, 1300, 1292, 1305, 1303, 1133, 1306, 1207, 1304, 1307, 1311, 1309, 1312 by jonasnick

Contributors

hebasto

real-or-random

Linked (view graph)

#1290 cmake: Set `ENVIRONMENT` property for examples on Windows