From 13525853016e24ddd24b7d21c2b57a15d6924c20 Mon Sep 17 00:00:00 2001 From: Brad King Date: Wed, 15 Jan 2020 10:00:14 -0500 Subject: Tests: Extend CMake.FileDownload test internal timeouts On a busy machine running many tests in parallel the `file(DOWNLOAD)` step can take longer than 2 seconds even to simply copy a file. Raise the timeout to 4 seconds to reduce spurious failures. --- Tests/CMakeTests/FileDownloadTest.cmake.in | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/Tests/CMakeTests/FileDownloadTest.cmake.in b/Tests/CMakeTests/FileDownloadTest.cmake.in index 3935449..4d5145b 100644 --- a/Tests/CMakeTests/FileDownloadTest.cmake.in +++ b/Tests/CMakeTests/FileDownloadTest.cmake.in @@ -1,3 +1,5 @@ +set(timeout 4) + if(NOT "@CMAKE_CURRENT_SOURCE_DIR@" MATCHES "^/") set(slash /) endif() @@ -8,14 +10,14 @@ message(STATUS "FileDownload:1") file(DOWNLOAD ${url} ${dir}/file1.png - TIMEOUT 2 + TIMEOUT ${timeout} ) message(STATUS "FileDownload:2") file(DOWNLOAD ${url} ${dir}/file2.png - TIMEOUT 2 + TIMEOUT ${timeout} SHOW_PROGRESS ) @@ -31,7 +33,7 @@ message(STATUS "FileDownload:3") file(DOWNLOAD ${url} ${dir}/file3.png - TIMEOUT 2 + TIMEOUT ${timeout} EXPECTED_MD5 dbd330d52f4dbd60115d4191904ded92 ) @@ -39,7 +41,7 @@ message(STATUS "FileDownload:4") file(DOWNLOAD ${url} ${dir}/file3.png - TIMEOUT 2 + TIMEOUT ${timeout} STATUS status EXPECTED_HASH SHA1=67eee17f79d9ac557284fc0b8ad19f25723fb578 ) @@ -48,7 +50,7 @@ message(STATUS "FileDownload:5") file(DOWNLOAD ${url} ${dir}/file3.png - TIMEOUT 2 + TIMEOUT ${timeout} STATUS status EXPECTED_HASH SHA224=ba283726bbb602776818b463943189afd91836cb7ee5dd6e2c7b5ae4 ) @@ -57,7 +59,7 @@ message(STATUS "FileDownload:6") file(DOWNLOAD ${url} ${dir}/file3.png - TIMEOUT 2 + TIMEOUT ${timeout} STATUS status EXPECTED_HASH SHA256=cf3334b1275071e1da6e8c396ccb72cf1b2388d8c937526f3af26230affb9423 ) @@ -66,7 +68,7 @@ message(STATUS "FileDownload:7") file(DOWNLOAD ${url} ${dir}/file3.png - TIMEOUT 2 + TIMEOUT ${timeout} STATUS status EXPECTED_HASH SHA384=43a5d13978d97c660db44481aee0604cb4ff6ca0775cd5c2cd68cd8000e107e507c4caf6c228941231041e282ffb8950 ) @@ -75,7 +77,7 @@ message(STATUS "FileDownload:8") file(DOWNLOAD ${url} ${dir}/file3.png - TIMEOUT 2 + TIMEOUT ${timeout} STATUS status EXPECTED_HASH SHA512=6984e0909a1018030ccaa418e3be1654223cdccff0fe6adc745f9aea7e377f178be53b9fc7d54a6f81c2b62ef9ddcd38ba1978fedf4c5e7139baaf355eefad5b ) @@ -83,7 +85,7 @@ message(STATUS "FileDownload:9") file(DOWNLOAD ${url} ${dir}/file3.png - TIMEOUT 2 + TIMEOUT ${timeout} STATUS status EXPECTED_HASH MD5=dbd330d52f4dbd60115d4191904ded92 ) @@ -92,7 +94,7 @@ message(STATUS "FileDownload:10") file(DOWNLOAD ${url} ${dir}/file3.png - TIMEOUT 2 + TIMEOUT ${timeout} STATUS status EXPECTED_MD5 dbd330d52f4dbd60115d4191904ded92 ) @@ -102,7 +104,7 @@ message(STATUS "FileDownload:11") file(DOWNLOAD badhostname.png ${dir}/file11.png - TIMEOUT 2 + TIMEOUT ${timeout} STATUS status ) message(STATUS "${status}") -- cgit v0.12 From 3bc73803b4e460410fff71e2c11e0aad116e8763 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 14 Jan 2020 11:03:20 -0500 Subject: Tests: Fix CMake.FileDownload test in presence of proxy We do not actually need to contact any real http servers. The one attempt we make is to an intentionally bad domain name. Unset any proxy configuration that may change behavior. --- Tests/CMakeTests/FileDownloadTest.cmake.in | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Tests/CMakeTests/FileDownloadTest.cmake.in b/Tests/CMakeTests/FileDownloadTest.cmake.in index 4d5145b..f795eb1 100644 --- a/Tests/CMakeTests/FileDownloadTest.cmake.in +++ b/Tests/CMakeTests/FileDownloadTest.cmake.in @@ -1,3 +1,8 @@ +# We do not contact any real URLs, but do try a bogus one. +# Remove any proxy configuration that may change behavior. +unset(ENV{http_proxy}) +unset(ENV{https_proxy}) + set(timeout 4) if(NOT "@CMAKE_CURRENT_SOURCE_DIR@" MATCHES "^/") -- cgit v0.12 From b56d429324330a9eb8d3c1860333969140851dd7 Mon Sep 17 00:00:00 2001 From: Craig Scott Date: Mon, 23 Dec 2019 21:34:04 +1100 Subject: Tests: Fix CMake.FileDownload test failures not reported Because the PASS_REGULAR_EXPRESSION test property is set, the exit code of the test is ignored, so we can't just rely on using message(SEND_ERROR) or the command itself failing. We have to add an explicit error message for all unexpected status codes and check for such messages with a FAIL_REGULAR_EXPRESSION. --- Tests/CMakeTests/CMakeLists.txt | 3 ++- Tests/CMakeTests/FileDownloadTest.cmake.in | 36 ++++++++++++++++++++++++++---- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/Tests/CMakeTests/CMakeLists.txt b/Tests/CMakeTests/CMakeLists.txt index 1619081..e32d693 100644 --- a/Tests/CMakeTests/CMakeLists.txt +++ b/Tests/CMakeTests/CMakeLists.txt @@ -33,8 +33,9 @@ AddCMakeTest(While "") AddCMakeTest(CMakeHostSystemInformation "") AddCMakeTest(FileDownload "") -set_property(TEST CMake.FileDownload PROPERTY +set_tests_properties(CMake.FileDownload PROPERTIES PASS_REGULAR_EXPRESSION "file already exists with expected MD5 sum" + FAIL_REGULAR_EXPRESSION "Unexpected status" ) AddCMakeTest(FileDownloadBadHash "") set_property(TEST CMake.FileDownloadBadHash PROPERTY diff --git a/Tests/CMakeTests/FileDownloadTest.cmake.in b/Tests/CMakeTests/FileDownloadTest.cmake.in index f795eb1..6c1ee75 100644 --- a/Tests/CMakeTests/FileDownloadTest.cmake.in +++ b/Tests/CMakeTests/FileDownloadTest.cmake.in @@ -11,20 +11,40 @@ endif() set(url "file://${slash}@CMAKE_CURRENT_SOURCE_DIR@/FileDownloadInput.png") set(dir "@CMAKE_CURRENT_BINARY_DIR@/downloads") +# Beware Windows asynchronous file/directory removal, rename and then +# remove the renamed dir so we can be certain the dir isn't there when +# we get to the file() commands below +if(EXISTS "${dir}") + file(RENAME ${dir} "${dir}_beingRemoved") + file(REMOVE_RECURSE "${dir}_beingRemoved") +endif() + +function(__reportIfWrongStatus statusPair expectedStatusCode) + list(GET statusPair 0 statusCode) + if(NOT statusCode EQUAL expectedStatusCode) + message(SEND_ERROR + "Unexpected status: ${statusCode}, expected: ${expectedStatusCode}") + endif() +endfunction() + message(STATUS "FileDownload:1") file(DOWNLOAD ${url} ${dir}/file1.png TIMEOUT ${timeout} + STATUS status ) +__reportIfWrongStatus("${status}" 0) message(STATUS "FileDownload:2") file(DOWNLOAD ${url} ${dir}/file2.png TIMEOUT ${timeout} + STATUS status SHOW_PROGRESS ) +__reportIfWrongStatus("${status}" 0) # Two calls in a row, exactly the same arguments. # Since downloaded file should exist already for 2nd call, @@ -39,8 +59,10 @@ file(DOWNLOAD ${url} ${dir}/file3.png TIMEOUT ${timeout} + STATUS status EXPECTED_MD5 dbd330d52f4dbd60115d4191904ded92 ) +__reportIfWrongStatus("${status}" 0) message(STATUS "FileDownload:4") file(DOWNLOAD @@ -50,6 +72,7 @@ file(DOWNLOAD STATUS status EXPECTED_HASH SHA1=67eee17f79d9ac557284fc0b8ad19f25723fb578 ) +__reportIfWrongStatus("${status}" 0) message(STATUS "FileDownload:5") file(DOWNLOAD @@ -59,6 +82,7 @@ file(DOWNLOAD STATUS status EXPECTED_HASH SHA224=ba283726bbb602776818b463943189afd91836cb7ee5dd6e2c7b5ae4 ) +__reportIfWrongStatus("${status}" 0) message(STATUS "FileDownload:6") file(DOWNLOAD @@ -68,6 +92,7 @@ file(DOWNLOAD STATUS status EXPECTED_HASH SHA256=cf3334b1275071e1da6e8c396ccb72cf1b2388d8c937526f3af26230affb9423 ) +__reportIfWrongStatus("${status}" 0) message(STATUS "FileDownload:7") file(DOWNLOAD @@ -77,6 +102,7 @@ file(DOWNLOAD STATUS status EXPECTED_HASH SHA384=43a5d13978d97c660db44481aee0604cb4ff6ca0775cd5c2cd68cd8000e107e507c4caf6c228941231041e282ffb8950 ) +__reportIfWrongStatus("${status}" 0) message(STATUS "FileDownload:8") file(DOWNLOAD @@ -86,6 +112,8 @@ file(DOWNLOAD STATUS status EXPECTED_HASH SHA512=6984e0909a1018030ccaa418e3be1654223cdccff0fe6adc745f9aea7e377f178be53b9fc7d54a6f81c2b62ef9ddcd38ba1978fedf4c5e7139baaf355eefad5b ) +__reportIfWrongStatus("${status}" 0) + message(STATUS "FileDownload:9") file(DOWNLOAD ${url} @@ -94,6 +122,7 @@ file(DOWNLOAD STATUS status EXPECTED_HASH MD5=dbd330d52f4dbd60115d4191904ded92 ) +__reportIfWrongStatus("${status}" 0) message(STATUS "FileDownload:10") file(DOWNLOAD @@ -103,6 +132,8 @@ file(DOWNLOAD STATUS status EXPECTED_MD5 dbd330d52f4dbd60115d4191904ded92 ) +__reportIfWrongStatus("${status}" 0) +# Print status because we check its message too message(STATUS "${status}") message(STATUS "FileDownload:11") @@ -113,7 +144,4 @@ file(DOWNLOAD STATUS status ) message(STATUS "${status}") -list(GET status 0 status_code) -if(NOT ${status_code} EQUAL 6) - message(SEND_ERROR "error: expected status code 6 for bad host name, got: ${status_code}") -endif() +__reportIfWrongStatus("${status}" 6) # 6 corresponds to an unresolvable host name -- cgit v0.12 From c0da651c09e11f9868529b7f8c4dc8f97c4546c8 Mon Sep 17 00:00:00 2001 From: Johnny Jazeix Date: Mon, 23 Dec 2019 10:35:41 +0100 Subject: file(DOWNLOAD): Don't fail if given just a filename to write to Fixes: #17969 --- Source/cmFileCommand.cxx | 3 ++- Tests/CMakeTests/FileDownloadTest.cmake.in | 18 ++++++++++++++++++ Tests/CMakeTests/FileTestScript.cmake | 2 +- 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/Source/cmFileCommand.cxx b/Source/cmFileCommand.cxx index 1fdfa87..d1775a7 100644 --- a/Source/cmFileCommand.cxx +++ b/Source/cmFileCommand.cxx @@ -1707,7 +1707,8 @@ bool HandleDownloadCommand(std::vector const& args, // as we receive downloaded bits from curl... // std::string dir = cmSystemTools::GetFilenamePath(file); - if (!cmSystemTools::FileExists(dir) && !cmSystemTools::MakeDirectory(dir)) { + if (!dir.empty() && !cmSystemTools::FileExists(dir) && + !cmSystemTools::MakeDirectory(dir)) { std::string errstring = "DOWNLOAD error: cannot create directory '" + dir + "' - Specify file by full path name and verify that you " "have directory creation and file write privileges."; diff --git a/Tests/CMakeTests/FileDownloadTest.cmake.in b/Tests/CMakeTests/FileDownloadTest.cmake.in index 6c1ee75..5bd3803 100644 --- a/Tests/CMakeTests/FileDownloadTest.cmake.in +++ b/Tests/CMakeTests/FileDownloadTest.cmake.in @@ -145,3 +145,21 @@ file(DOWNLOAD ) message(STATUS "${status}") __reportIfWrongStatus("${status}" 6) # 6 corresponds to an unresolvable host name + +message(STATUS "FileDownload:12") +set(absFile "@CMAKE_CURRENT_BINARY_DIR@/file12.png") +if(EXISTS "${absFile}") + file(RENAME ${absFile} "${absFile}_beingRemoved") + file(REMOVE "${absFile}_beingRemoved") +endif() +file(DOWNLOAD + ${url} + file12.png + TIMEOUT ${timeout} + EXPECTED_MD5 dbd330d52f4dbd60115d4191904ded92 + STATUS status + ) +__reportIfWrongStatus("${status}" 0) +if(NOT EXISTS file12.png) + message(SEND_ERROR "file12.png not downloaded: ${status}") +endif() diff --git a/Tests/CMakeTests/FileTestScript.cmake b/Tests/CMakeTests/FileTestScript.cmake index 9a43569..145f28a 100644 --- a/Tests/CMakeTests/FileTestScript.cmake +++ b/Tests/CMakeTests/FileTestScript.cmake @@ -183,7 +183,7 @@ elseif(testname STREQUAL to_native_path) # pass elseif(testname STREQUAL download_wrong_number_of_args) # fail file(DOWNLOAD zzzz://bogus/ffff) -elseif(testname STREQUAL download_file_with_no_path) # fail +elseif(testname STREQUAL download_file_with_no_path) # pass file(DOWNLOAD zzzz://bogus/ffff ffff) elseif(testname STREQUAL download_missing_time) # fail -- cgit v0.12