From 04151d758b7bc4bab1ba70b0a2d2779fb510ba33 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 (cherry picked from commit 8d6fdfd9deb78545ecbf2a2eabd90aeb7cbaf78e) --- .github/workflows/main.yml | 14 +++++++------- CHANGELOG.rst | 2 ++ CONTRIBUTING.rst | 8 ++++---- cmake/Tests.cmake | 15 ++------------- tests/checks/features-qmark1.fish | 1 - 5 files changed, 15 insertions(+), 25 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 374ea89aa..099107700 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 66271197b..40034f71b 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,6 +14,8 @@ Changes since 4.0b1 - Add debug information back to cmake builds with the "RelWithDebInfo" profile (:issue:`10959`). - the :kbd:`ctrl-c` binding now calls a new bind function called "clear-commandline", the old behavior that leaves a "^C" marker is available as "cancel-commandline" (:issue:`10935`) +- The ``make test`` target was removed as it can no longer be defined in new CMake versions. Use ``make fish_run_tests``. + The built-in test target will run if you built fish before, but will not print output if it fails (:issue:`11116`). fish 4.0.0 (released ???) ========================= diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst index 767cd7d69..5b14501fc 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 ======================== @@ -207,7 +207,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 Git hooks --------- @@ -235,7 +235,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 @@ -245,7 +245,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 374f96726..5ade407ff 100644 --- a/cmake/Tests.cmake +++ b/cmake/Tests.cmake @@ -8,13 +8,11 @@ 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. # * The only way to have a test depend on a binary is to add a fake test with a name like # "build_fish" that executes CMake recursively to build the `fish` target. # * Circling back to the point about individual tests not being actual Makefile targets, CMake does @@ -32,15 +30,6 @@ 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 definining 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) - # The "test" directory. set(TEST_DIR ${CMAKE_CURRENT_BINARY_DIR}/test) @@ -81,7 +70,7 @@ configure_file(build_tools/pexpect_helper.py pexpect_helper.py COPYONLY) set(CMAKE_XCODE_GENERATE_SCHEME 0) # 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