From 8d6fdfd9deb78545ecbf2a2eabd90aeb7cbaf78e Mon Sep 17 00:00:00 2001 From: Fabian Boehm Date: Sun, 2 Feb 2025 13:40:08 +0100 Subject: [PATCH] Remove cmake "test" target This can no longer be overridden, which means we have a broken "test" target now. Instead, you need to call "make fish_run_tests". Blergh. Fixes #11116 --- .github/workflows/main.yml | 14 +++++++------- CHANGELOG.rst | 1 + CONTRIBUTING.rst | 8 ++++---- cmake/Tests.cmake | 17 ++++------------- tests/checks/features-qmark1.fish | 1 - tests/checks/symlinks-not-overwritten.fish | 3 --- 6 files changed, 16 insertions(+), 28 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 7e3344d5f..ffb7de1c4 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -1,4 +1,4 @@ -name: make test +name: make fish_run_tests on: [push, pull_request] @@ -29,7 +29,7 @@ jobs: - name: make run: | make VERBOSE=1 - - name: make test + - name: make fish_run_tests run: | make VERBOSE=1 test @@ -55,7 +55,7 @@ jobs: - name: make run: | make VERBOSE=1 - - name: make test + - name: make fish_run_tests run: | make VERBOSE=1 test @@ -93,7 +93,7 @@ jobs: - name: make run: | make VERBOSE=1 - - name: make test + - name: make fish_run_tests env: FISH_CI_SAN: 1 ASAN_OPTIONS: check_initialization_order=1:detect_stack_use_after_return=1:detect_leaks=1:fast_unwind_on_malloc=0 @@ -133,9 +133,9 @@ jobs: # - name: make # run: | # make - # - name: make test + # - name: make fish_run_tests # run: | - # make test + # make fish_run_tests macos: @@ -161,6 +161,6 @@ jobs: - name: make run: | make VERBOSE=1 - - name: make test + - name: make fish_run_tests run: | make VERBOSE=1 test diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 9eee69027..db1358b14 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -48,6 +48,7 @@ Other improvements For distributors ---------------- - ``fish_indent`` and ``fish_key_reader`` are still built as separate binaries for now, but can also be replaced with a symlink if you want to save disk space. +- The CMake ``test`` target no longer exists as CMake no longer allows defining a custom "test" target. Use ``make fish_run_tests`` instead (:issue:`11116`). fish 4.0b1 (released December 17, 2024) ======================================= diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst index b78730fb9..a479770f9 100644 --- a/CONTRIBUTING.rst +++ b/CONTRIBUTING.rst @@ -43,7 +43,7 @@ Guidelines In short: - Be conservative in what you need (keep to the agreed minimum supported Rust version, limit new dependencies) -- Use automated tools to help you (including ``make test`` and ``build_tools/style.fish``) +- Use automated tools to help you (including ``make fish_run_tests`` and ``build_tools/style.fish``) Contributing completions ======================== @@ -213,7 +213,7 @@ The tests can be run on your local computer on all operating systems. :: cmake path/to/fish-shell - make test + make fish_run_tests Or you can run them on a fish, without involving cmake:: @@ -252,7 +252,7 @@ One possibility is a pre-push hook script like this one: done if [ "x$isprotected" = x1 ]; then echo "Running tests before push to master" - make test + make fish_run_tests RESULT=$? if [ $RESULT -ne 0 ]; then echo "Tests failed for a push to master, we can't let you do that" >&2 @@ -262,7 +262,7 @@ One possibility is a pre-push hook script like this one: exit 0 This will check if the push is to the master branch and, if it is, only -allow the push if running ``make test`` succeeds. In some circumstances +allow the push if running ``make fish_run_tests`` succeeds. In some circumstances it may be advisable to circumvent this check with ``git push --no-verify``, but usually that isn’t necessary. diff --git a/cmake/Tests.cmake b/cmake/Tests.cmake index 8e6ef66cd..075108209 100644 --- a/cmake/Tests.cmake +++ b/cmake/Tests.cmake @@ -21,13 +21,13 @@ set(CMAKE_FOLDER tests) # pass but it should not be considered a failed test run, either. set(SKIP_RETURN_CODE 125) -# Even though we are using CMake's ctest for testing, we still define our own `make test` target +# Even though we are using CMake's ctest for testing, we still define our own `make fish_run_tests` target # rather than use its default for many reasons: # * CMake doesn't run tests in-proc or even add each tests as an individual node in the ninja # dependency tree, instead it just bundles all tests into a target called `test` that always just # shells out to `ctest`, so there are no build-related benefits to not doing that ourselves. -# * CMake devs insist that it is appropriate for `make test` to never depend on `make all`, i.e. -# running `make test` does not require any of the binaries to be built before testing. +# * CMake devs insist that it is appropriate for `make fish_run_tests` to never depend on `make all`, i.e. +# running `make fish_run_tests` does not require any of the binaries to be built before testing. # * It is not possible to set top-level CTest options/settings such as CTEST_PARALLEL_LEVEL from # within the CMake configuration file. # * The only way to have a test depend on a binary is to add a fake test with a name like @@ -47,17 +47,8 @@ add_custom_target(fish_run_tests USES_TERMINAL ) -# If CMP0037 is available, also make an alias "test" target. -# Note that this policy may not be available, in which case defining such a target silently fails. -cmake_policy(PUSH) -if(POLICY CMP0037) - cmake_policy(SET CMP0037 OLD) - add_custom_target(test DEPENDS fish_run_tests) -endif() -cmake_policy(POP) - # CMake being CMake, you can't just add a DEPENDS argument to add_test to make it depend on any of -# your binaries actually being built before `make test` is executed (requiring `make all` first), +# your binaries actually being built before `make fish_run_tests` is executed (requiring `make all` first), # and the only dependency a test can have is on another test. So we make building fish # prerequisites to our entire top-level `test` target. function(add_test_target NAME) diff --git a/tests/checks/features-qmark1.fish b/tests/checks/features-qmark1.fish index b0ebdb03d..66eed3cc4 100644 --- a/tests/checks/features-qmark1.fish +++ b/tests/checks/features-qmark1.fish @@ -1,3 +1,2 @@ -# Explicitly overriding HOME/XDG_CONFIG_HOME is only required if not invoking via `make test` # RUN: %fish --features '' -c 'string match --quiet "??" ab ; echo "qmarkon: $status"' #CHECK: qmarkon: 1 diff --git a/tests/checks/symlinks-not-overwritten.fish b/tests/checks/symlinks-not-overwritten.fish index f4b8556af..5b3446352 100644 --- a/tests/checks/symlinks-not-overwritten.fish +++ b/tests/checks/symlinks-not-overwritten.fish @@ -1,8 +1,5 @@ -# Explicitly overriding HOME/XDG_CONFIG_HOME is only required if not invoking via `make test` # RUN: fish=%fish %fish %s -mkdir -p $XDG_CONFIG_HOME/fish - # fish_variables set -l target_file $XDG_CONFIG_HOME/fish/target_fish_variables set -l fish_variables $XDG_CONFIG_HOME/fish/fish_variables