From 558ded62104096007beca1c6bf924bc95c4ad08c Mon Sep 17 00:00:00 2001 From: David Wells Date: Sat, 10 Aug 2019 13:10:36 -0600 Subject: [PATCH] Rmove the option to use diff in the test suite. Most tests won't work with plain diff since there are slight differences in floating point output: Hence, we should remove it as an option and just require numdiff. --- cmake/macros/macro_deal_ii_add_test.cmake | 9 ++- cmake/macros/macro_deal_ii_pickup_tests.cmake | 64 +++++++------------ cmake/scripts/run_test.sh | 43 ++++--------- .../incompatibilities/20190810DavidWells | 4 ++ doc/users/testsuite.html | 5 +- 5 files changed, 45 insertions(+), 80 deletions(-) create mode 100644 doc/news/changes/incompatibilities/20190810DavidWells diff --git a/cmake/macros/macro_deal_ii_add_test.cmake b/cmake/macros/macro_deal_ii_add_test.cmake index 333dad12c1..9b3c878a85 100644 --- a/cmake/macros/macro_deal_ii_add_test.cmake +++ b/cmake/macros/macro_deal_ii_add_test.cmake @@ -65,9 +65,8 @@ # # The following variables must be set: # -# NUMDIFF_EXECUTABLE, DIFF_EXECUTABLE -# - pointing to valid diff executables. If NUMDIFF_EXECUTABLE is not -# "numdiff" it will be ignored and DIFF_EXECUTABLE is used instead. +# NUMDIFF_EXECUTABLE +# - Complete path to the numdiff binary. # # TEST_TIME_LIMIT # - specifying the maximal wall clock time in seconds a test is allowed @@ -303,8 +302,8 @@ MACRO(DEAL_II_ADD_TEST _category _test_name _comparison_file) ADD_CUSTOM_COMMAND(OUTPUT ${_test_directory}/diff COMMAND sh ${DEAL_II_PATH}/${DEAL_II_SHARE_RELDIR}/scripts/run_test.sh - diff "${_test_full}" "${_test_diff}" - "${DIFF_EXECUTABLE}" "${_comparison_file}" ${_run_args} + diff "${_test_full}" "${NUMDIFF_EXECUTABLE}" + "${_comparison_file}" ${_run_args} WORKING_DIRECTORY ${_test_directory} DEPENDS diff --git a/cmake/macros/macro_deal_ii_pickup_tests.cmake b/cmake/macros/macro_deal_ii_pickup_tests.cmake index 4691443377..cd3cf6af97 100644 --- a/cmake/macros/macro_deal_ii_pickup_tests.cmake +++ b/cmake/macros/macro_deal_ii_pickup_tests.cmake @@ -20,7 +20,7 @@ # If TEST_PICKUP_REGEX is set, only tests matching the regex will be # processed. # -# Furthermore, the macro sets up (if necessary) deal.II, perl, a diff tool +# Furthermore, the macro sets up (if necessary) deal.II, perl, numdiff, # and the following variables, that can be overwritten by environment or # command line: # @@ -40,9 +40,8 @@ # - A regular expression to select only a subset of tests during setup. # An empty string is interpreted as a catchall (this is the default). # -# Either numdiff (if available), or diff are used for the comparison of -# test results. Their location can be specified with NUMDIFF_DIR and -# DIFF_DIR. +# numdiff is used for the comparison of test results. Its location can be +# specified with NUMDIFF_DIR. # # Usage: # DEAL_II_PICKUP_TESTS() @@ -91,61 +90,42 @@ MACRO(DEAL_II_PICKUP_TESTS) FIND_PACKAGE(Perl REQUIRED) - FIND_PROGRAM(DIFF_EXECUTABLE - NAMES diff - HINTS ${DIFF_DIR} - PATH_SUFFIXES bin - ) - FIND_PROGRAM(NUMDIFF_EXECUTABLE NAMES numdiff HINTS ${NUMDIFF_DIR} PATH_SUFFIXES bin ) - MARK_AS_ADVANCED(DIFF_EXECUTABLE NUMDIFF_EXECUTABLE) + MARK_AS_ADVANCED(NUMDIFF_EXECUTABLE) - IF( NUMDIFF_EXECUTABLE MATCHES "-NOTFOUND" AND - DIFF_EXECUTABLE MATCHES "-NOTFOUND" ) + IF( NUMDIFF_EXECUTABLE MATCHES "-NOTFOUND") MESSAGE(FATAL_ERROR - "Could not find diff or numdiff. One of those are required for running the testsuite.\n" - "Please specify DIFF_DIR or NUMDIFF_DIR to a location containing the binaries." + "Could not find numdiff, which is required for running the testsuite.\n" + "Please specify NUMDIFF_DIR to a location containing the binary." ) ENDIF() - IF(DIFF_EXECUTABLE MATCHES "-NOTFOUND") - SET(DIFF_EXECUTABLE ${NUMDIFF_EXECUTABLE}) - ENDIF() - - IF(NUMDIFF_EXECUTABLE MATCHES "-NOTFOUND") - SET(NUMDIFF_EXECUTABLE ${DIFF_EXECUTABLE}) - ENDIF() - # - # Check that the diff programs can run and terminate successfully: + # Check that numdiff can run and terminate successfully: # - FOREACH(_diff_program ${NUMDIFF_EXECUTABLE} ${DIFF_EXECUTABLE}) - EXECUTE_PROCESS(COMMAND ${_diff_program} "-v" - TIMEOUT 4 # seconds - OUTPUT_QUIET - ERROR_QUIET - RESULT_VARIABLE _diff_program_status - ) + EXECUTE_PROCESS(COMMAND ${NUMDIFF_EXECUTABLE} "-v" + TIMEOUT 4 # seconds + OUTPUT_QUIET + ERROR_QUIET + RESULT_VARIABLE _diff_program_status + ) - IF(NOT "${_diff_program_status}" STREQUAL "0") - MESSAGE(FATAL_ERROR - "\nThe command \"${_diff_program} -v\" did not run correctly: it either " - "failed to exit after a few seconds or returned a nonzero exit code. " - "The test suite cannot be set up without this program, so please " - "reinstall it and then run the test suite setup command again.\n") - ENDIF() - ENDFOREACH() + IF(NOT "${_diff_program_status}" STREQUAL "0") + MESSAGE(FATAL_ERROR + "\nThe command \"${_diff_program} -v\" did not run correctly: it either " + "failed to exit after a few seconds or returned a nonzero exit code. " + "The test suite cannot be set up without this program, so please " + "reinstall it and then run the test suite setup command again.\n") + ENDIF() # # Also check that numdiff is not a symlink to diff by running a relative - # tolerance test. Note that we set NUMDIFF_EXECUTABLE to diff in case we were - # not able to find it above, but here we check that the executable is really - # named 'numdiff'. + # tolerance test. # STRING(FIND "${NUMDIFF_EXECUTABLE}" "numdiff" _found_numdiff_binary) IF(NOT "${_found_numdiff_binary}" STREQUAL "-1") diff --git a/cmake/scripts/run_test.sh b/cmake/scripts/run_test.sh index cd9279d79f..4251cb1846 100644 --- a/cmake/scripts/run_test.sh +++ b/cmake/scripts/run_test.sh @@ -20,8 +20,7 @@ # Usage: # run_test.sh run TEST_FULL [COMMAND and ARGS] # -# run_test.sh diff TEST_FULL NUMDIFF_EXECUTABLE DIFF_EXECUTABLE \ -# COMPARISON_FILE +# run_test.sh diff TEST_FULL NUMDIFF_EXECUTABLE COMPARISON_FILE # set -u @@ -83,8 +82,7 @@ case $STAGE in ## NUMDIFF_EXECUTABLE="$1" - DIFF_EXECUTABLE="$2" - COMPARISON_FILE="$3" + COMPARISON_FILE="$2" rm -f failing_diff* rm -f diff* @@ -101,27 +99,15 @@ case $STAGE in variant="${file#*.output}" # - # Run diff or numdiff (if available) to determine whether files are the - # same. Create a diff file "diff${variant}" for each variant file that - # is found (including the main comparison file). + # Configure numdiff with + # - absolute precision set to 1e-6, everything below is + # regarded as being equal to 0 + # - relative differences of 1e-8 + # - [space][tab][newline]=,:;<>[](){}^ as separators between + # numbers # - case "${NUMDIFF_EXECUTABLE}" in - *numdiff*) - # - # Configure numdiff with - # - absolute precision set to 1e-6, everything below is - # regarded as being equal to 0 - # - relative differences of 1e-8 - # - [space][tab][newline]=,:;<>[](){}^ as separators between - # numbers - # - "${NUMDIFF_EXECUTABLE}" -a 1e-6 -r 1e-8 -s ' \t\n=,:;<>[](){}^' \ - "${file}" output > diff${variant} - ;; - *) - "${DIFF_EXECUTABLE}" "${file}" output > diff${variant} - ;; - esac + "${NUMDIFF_EXECUTABLE}" -a 1e-6 -r 1e-8 -s ' \t\n=,:;<>[](){}^' \ + "${file}" output > diff${variant} if [ $? -eq 0 ]; then # @@ -146,8 +132,7 @@ case $STAGE in # # If none of the diffs succeeded, use the diff against the main comparison - # file. Output the first few lines of the output of numdiff, followed by - # the results of regular diff since the latter is just more readable. + # file. Output the first few lines of the output of numdiff. # if [ $test_successful = false ] ; then for file in diff*; do @@ -159,10 +144,8 @@ case $STAGE in echo "${TEST_FULL}: DIFF failed. ------ Result: `pwd`/output" echo "Check `pwd`/output ${COMPARISON_FILE}" echo "${TEST_FULL}: DIFF failed. ------ Diff: `pwd`/failing_diff" - echo "${TEST_FULL}: DIFF failed. ------ First 8 lines of numdiff/diff output:" - cat failing_diff | head -n 8 - echo "${TEST_FULL}: DIFF failed. ------ First 50 lines diff output:" - "${DIFF_EXECUTABLE}" -c "${COMPARISON_FILE}" output | head -n 50 + echo "${TEST_FULL}: DIFF failed. ------ First 20 lines of numdiff output:" + head -n 20 failing_diff exit 1 fi exit 0 diff --git a/doc/news/changes/incompatibilities/20190810DavidWells b/doc/news/changes/incompatibilities/20190810DavidWells new file mode 100644 index 0000000000..a9cef0c454 --- /dev/null +++ b/doc/news/changes/incompatibilities/20190810DavidWells @@ -0,0 +1,4 @@ +Changed: The test suite now requires numdiff (i.e., it cannot be run with +simply diff). +
+(David Wells, 2019/08/10) diff --git a/doc/users/testsuite.html b/doc/users/testsuite.html index a7776b3d0a..2b766ab372 100644 --- a/doc/users/testsuite.html +++ b/doc/users/testsuite.html @@ -301,9 +301,8 @@ TEST_TARGET or TEST_TARGET_DEBUG and TEST_TARGET_RELEASE - used instead of TEST_TARGET for debug/release configuration -NUMDIFF_EXECUTABLE, DIFF_EXECUTABLE - - pointing to valid diff executables. If NUMDIFF_EXECUTABLE is not - "numdiff" it will be ignored and DIFF_EXECUTABLE is used instead +NUMDIFF_EXECUTABLE + - pointing to a valid "numdiff" executable TEST_TIME_LIMIT - specifying the maximal wall clock time in seconds a test is allowed -- 2.39.5