Make test a custom target again and add top-level test targets

Even though we are using CMake's ctest for testing, we still define our
own `make test` 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.
 * It is not possible to set top-level CTest options/settings such as
   CTEST_PARALLEL_LEVEL from within the CMake configuration file.
 * Circling back to the point about individual tests not being actual
   Makefile targets, CMake does not offer any way to execute a named
   test via the `make`/`ninja`/whatever interface; the only way to
   manually invoke test `foo` is to to manually run `ctest` and specify
   a regex matching `foo` as an argument, e.g. `ctest -R ^foo$`... which
   is really crazy.

With this patch, it is now possible to execute any single test by name,
by invoking the build directly, e.g. to run the `universal.fish` check:
`cmake --build build --target universal.fish` or
`ninja -C build universal.fish`. Unfortunately, this is not integrated
into the Makefile wrapper, so `make universal.fish` won't work (although
this can potentially be hacked around).
This commit is contained in:
Mahmoud Al-Qudsi 2021-08-08 18:31:50 -05:00 committed by Johannes Altmanninger
parent 26092456d4
commit aaac759d9a
3 changed files with 81 additions and 114 deletions

View file

@ -5,11 +5,40 @@ enable_testing()
if(NOT CTEST_PARALLEL_LEVEL)
include(ProcessorCount)
ProcessorCount(CORES)
message("CTEST_PARALLEL_LEVEL ${CORES}")
set(CTEST_PARALLEL_LEVEL ${CORES})
endif()
# Define fish_tests.
# Even though we are using CMake's ctest for testing, we still define our own `make test` 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.
# * It is not possible to set top-level CTest options/settings such as CTEST_PARALLEL_LEVEL from
# within the CMake configuration file.
# * Circling back to the point about individual tests not being actual Makefile targets, CMake does
# not offer any way to execute a named test via the `make`/`ninja`/whatever interface; the only
# way to manually invoke test `foo` is to to manually run `ctest` and specify a regex matching
# `foo` as an argument, e.g. `ctest -R ^foo$`... which is really crazy.
# Set a policy so CMake stops complaining when we use the target name "test"
cmake_policy(PUSH)
if(${CMAKE_VERSION} VERSION_LESS 3.11.0 AND POLICY CMP0037)
cmake_policy(SET CMP0037 OLD)
endif()
add_custom_target(test
COMMAND env CTEST_PARALLEL_LEVEL=${CTEST_PARALLEL_LEVEL}
${CMAKE_CTEST_COMMAND} --force-new-ctest-process
--output-on-failure
DEPENDS fish_tests tests_buildroot_target
USES_TERMINAL
)
cmake_policy(POP)
# Build the low-level tests code
add_executable(fish_tests EXCLUDE_FROM_ALL
src/fish_tests.cpp)
fish_link_deps_and_sign(fish_tests)
@ -35,18 +64,23 @@ set(TEST_INSTALL_DIR ${TEST_DIR}/buildroot)
# The directory where the tests expect to find the fish root (./bin, etc)
set(TEST_ROOT_DIR ${TEST_DIR}/root)
# Copy tests files.
file(GLOB TESTS_FILES tests/*)
add_custom_target(tests_dir DEPENDS tests)
# Copy needed directories for out-of-tree builds
if(NOT FISH_IN_TREE_BUILD)
add_custom_command(TARGET tests_dir
add_custom_target(funcs_dir)
add_custom_command(TARGET funcs_dir
COMMAND mkdir -p ${CMAKE_BINARY_DIR}/share && ln -sf
${CMAKE_SOURCE_DIR}/share/functions/ ${CMAKE_BINARY_DIR}/share/functions
COMMENT "Symlinking fish functions to binary dir"
VERBATIM)
add_custom_target(tests_dir DEPENDS tests)
add_custom_command(TARGET tests_dir
COMMAND ${CMAKE_COMMAND} -E copy_directory
${CMAKE_SOURCE_DIR}/tests/ ${CMAKE_BINARY_DIR}/tests/
COMMENT "Copying test files to binary dir"
VERBATIM)
add_dependencies(fish_tests tests_dir)
add_dependencies(fish_tests tests_dir funcs_dir)
endif()
# Copy littlecheck.py
@ -55,30 +89,41 @@ configure_file(build_tools/littlecheck.py littlecheck.py COPYONLY)
# Copy pexpect_helper.py
configure_file(build_tools/pexpect_helper.py pexpect_helper.py COPYONLY)
# Make the directory in which to run tests.
# Also symlink fish to where the tests expect it to be.
# Lastly put fish_test_helper there too.
# 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),
# and the only dependency a test can have is on another test. So we make building fish and
# `fish_tests` prerequisites to our entire top-level `test` target.
add_custom_target(tests_buildroot_target
# Make the directory in which to run tests:
COMMAND ${CMAKE_COMMAND} -E make_directory ${TEST_INSTALL_DIR}
COMMAND DESTDIR=${TEST_INSTALL_DIR} ${CMAKE_COMMAND}
--build ${CMAKE_CURRENT_BINARY_DIR} --target install
# Put fish_test_helper there too:
COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_BINARY_DIR}/fish_test_helper
${TEST_INSTALL_DIR}/${CMAKE_INSTALL_PREFIX}/bin
# Also symlink fish to where the tests expect it to be:
COMMAND ${CMAKE_COMMAND} -E create_symlink
${TEST_INSTALL_DIR}/${CMAKE_INSTALL_PREFIX}
${TEST_ROOT_DIR}
DEPENDS fish fish_test_helper)
function(add_test_target NAME)
add_custom_target(${NAME}
COMMAND ${CMAKE_CTEST_COMMAND} --output-on-failure -R "^${NAME}$$"
DEPENDS fish_tests tests_buildroot_target
USES_TERMINAL
)
endfunction()
foreach(LTEST ${LOW_LEVEL_TESTS})
add_test(
NAME ${LTEST}
COMMAND ${CMAKE_BINARY_DIR}/fish_tests ${LTEST}
WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
)
add_test_target("${LTEST}")
endforeach(LTEST)
add_test(tests_buildroot_target
"${CMAKE_COMMAND}" --build ${CMAKE_BINARY_DIR} --target tests_buildroot_target)
FILE(GLOB FISH_CHECKS CONFIGURE_DEPENDS ${CMAKE_SOURCE_DIR}/tests/checks/*.fish)
foreach(CHECK ${FISH_CHECKS})
get_filename_component(CHECK_NAME ${CHECK} NAME)
@ -88,6 +133,5 @@ foreach(CHECK ${FISH_CHECKS})
${CMAKE_CURRENT_BINARY_DIR}/tests/test.fish ${CHECK}
WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/tests
)
set_tests_properties(${LTEST} PROPERTIES DEPENDS tests_buildroot_target)
add_test_target("${CHECK_NAME}")
endforeach(CHECK)

View file

@ -4341,6 +4341,11 @@ void history_tests_t::test_history_races_pound_on_history(size_t item_count, siz
}
void history_tests_t::test_history_races() {
// This always fails under WSL
if (is_windows_subsystem_for_linux()) {
return;
}
say(L"Testing history race conditions");
// It appears TSAN and ASAN's allocators do not release their locks properly in atfork, so
@ -6772,9 +6777,13 @@ void list_tests() {
/// Main test.
int main(int argc, char **argv) {
UNUSED(argc);
setlocale(LC_ALL, "");
if (argc >= 2 && std::strcmp(argv[1], "--list") == 0) {
list_tests();
return 0;
}
// Look for the file tests/test.fish. We expect to run in a directory containing that file.
// If we don't find it, walk up the directory hierarchy until we do, or error.
while (access("./tests/test.fish", F_OK) != 0) {
@ -6819,103 +6828,11 @@ int main(int argc, char **argv) {
// Set PWD from getcwd - fixes #5599
env_stack_t::principal().set_pwd_from_getcwd();
if (should_test_function("utility_functions")) test_utility_functions();
if (should_test_function("string_split")) test_split_string_tok();
if (should_test_function("wwrite_to_fd")) test_wwrite_to_fd();
if (should_test_function("env_vars")) test_env_vars();
if (should_test_function("env")) test_env_snapshot();
if (should_test_function("str_to_num")) test_str_to_num();
if (should_test_function("enum")) test_enum_set();
if (should_test_function("enum")) test_enum_array();
if (should_test_function("highlighting")) test_highlighting();
if (should_test_function("new_parser_ll2")) test_new_parser_ll2();
if (should_test_function("new_parser_fuzzing"))
test_new_parser_fuzzing(); // fuzzing is expensive
if (should_test_function("new_parser_correctness")) test_new_parser_correctness();
if (should_test_function("new_parser_ad_hoc")) test_new_parser_ad_hoc();
if (should_test_function("new_parser_errors")) test_new_parser_errors();
if (should_test_function("error_messages")) test_error_messages();
if (should_test_function("escape")) test_unescape_sane();
if (should_test_function("escape")) test_escape_crazy();
if (should_test_function("escape")) test_escape_quotes();
if (should_test_function("format")) test_format();
if (should_test_function("convert")) test_convert();
if (should_test_function("convert")) test_convert_private_use();
if (should_test_function("convert_ascii")) test_convert_ascii();
if (should_test_function("perf_convert_ascii", false)) perf_convert_ascii();
if (should_test_function("convert_nulls")) test_convert_nulls();
if (should_test_function("tokenizer")) test_tokenizer();
if (should_test_function("fd_monitor")) test_fd_monitor();
if (should_test_function("iothread")) test_iothread();
if (should_test_function("pthread")) test_pthread();
if (should_test_function("debounce")) test_debounce();
if (should_test_function("debounce")) test_debounce_timeout();
if (should_test_function("parser")) test_parser();
if (should_test_function("cancellation")) test_cancellation();
if (should_test_function("indents")) test_indents();
if (should_test_function("utf8")) test_utf8();
if (should_test_function("feature_flags")) test_feature_flags();
if (should_test_function("escape_sequences")) test_escape_sequences();
if (should_test_function("pcre2_escape")) test_pcre2_escape();
if (should_test_function("lru")) test_lru();
if (should_test_function("expand")) test_expand();
if (should_test_function("expand")) test_expand_overflow();
if (should_test_function("fuzzy_match")) test_fuzzy_match();
if (should_test_function("ifind")) test_ifind();
if (should_test_function("ifind_fuzzy")) test_ifind_fuzzy();
if (should_test_function("abbreviations")) test_abbreviations();
if (should_test_function("test")) test_test();
if (should_test_function("wcstod")) test_wcstod();
if (should_test_function("dup2s")) test_dup2s();
if (should_test_function("dup2s")) test_dup2s_fd_for_target_fd();
if (should_test_function("path")) test_path();
if (should_test_function("pager_navigation")) test_pager_navigation();
if (should_test_function("pager_layout")) test_pager_layout();
if (should_test_function("word_motion")) test_word_motion();
if (should_test_function("is_potential_path")) test_is_potential_path();
if (should_test_function("colors")) test_colors();
if (should_test_function("complete")) test_complete();
if (should_test_function("autoload")) test_autoload();
if (should_test_function("input")) test_input();
if (should_test_function("line_iterator")) test_line_iterator();
if (should_test_function("undo")) test_undo();
if (should_test_function("universal")) test_universal();
if (should_test_function("universal")) test_universal_output();
if (should_test_function("universal")) test_universal_parsing();
if (should_test_function("universal")) test_universal_parsing_legacy();
if (should_test_function("universal")) test_universal_callbacks();
if (should_test_function("universal")) test_universal_formats();
if (should_test_function("universal")) test_universal_ok_to_save();
if (should_test_function("notifiers")) test_universal_notifiers();
if (should_test_function("wait_handles")) test_wait_handles();
if (should_test_function("completion_insertions")) test_completion_insertions();
if (should_test_function("autosuggestion_ignores")) test_autosuggestion_ignores();
if (should_test_function("autosuggestion_combining")) test_autosuggestion_combining();
if (should_test_function("autosuggest_suggest_special")) test_autosuggest_suggest_special();
if (should_test_function("history")) history_tests_t::test_history();
if (should_test_function("history_merge")) history_tests_t::test_history_merge();
if (should_test_function("history_paths")) history_tests_t::test_history_path_detection();
if (!is_windows_subsystem_for_linux()) {
// this test always fails under WSL
if (should_test_function("history_races")) history_tests_t::test_history_races();
for (const auto &test : s_tests) {
if (should_test_function(test.group)) {
test.test();
}
}
if (should_test_function("history_formats")) history_tests_t::test_history_formats();
if (should_test_function("string")) test_string();
if (should_test_function("illegal_command_exit_code")) test_illegal_command_exit_code();
if (should_test_function("maybe")) test_maybe();
if (should_test_function("layout_cache")) test_layout_cache();
if (should_test_function("prompt")) test_prompt_truncation();
if (should_test_function("normalize")) test_normalize_path();
if (should_test_function("dirname")) test_dirname_basename();
if (should_test_function("topics")) test_topic_monitor();
if (should_test_function("topics")) test_topic_monitor_torture();
if (should_test_function("pipes")) test_pipes();
if (should_test_function("fd_event")) test_fd_event_signaller();
if (should_test_function("timer_format")) test_timer_format();
// history_tests_t::test_history_speed();
if (should_test_function("termsize")) termsize_tester_t::test();
if (should_test_function("killring")) test_killring();
say(L"Encountered %d errors in low-level tests", err_count);
if (s_test_run_count == 0) say(L"*** No Tests Were Actually Run! ***");

View file

@ -21,8 +21,14 @@ die() {
# To keep things sane and to make error messages comprehensible, do not use relative paths anywhere
# in this script. Instead, make all paths relative to one of these or $homedir."
TESTS_ROOT="$(realpath "$(dirname "$0")")"
BUILD_ROOT="$(realpath "${TESTS_ROOT}/..")"
TESTS_ROOT="$(dirname "$0")"
BUILD_ROOT="${TESTS_ROOT}/.."
# macOS (still!) doesn't have `readlink -f` or `realpath`. That's OK, this is just for aesthetics.
if command -v realpath 1>/dev/null 2>/dev/null; then
TESTS_ROOT="$(realpath --no-symlinks "${TESTS_ROOT}")"
BUILD_ROOT="$(realpath --no-symlinks "${BUILD_ROOT}")"
fi
if ! test -z "$__fish_is_running_tests"; then
echo "Recursive test invocation detected!" 1>&2