Commit 96fdde26 authored by Craig Scott's avatar Craig Scott Committed by Brad King
Browse files

GoogleTest: Rename TIMEOUT parameter to avoid clash

In gtest_discover_tests(), the TIMEOUT keyword was making it
impossible to set the TIMEOUT test property via the PROPERTIES
keyword. This would be a frequent case, but it doesn't complain
and instead silently does something different to what would
normally be expected. The TIMEOUT keyword has been renamed
to DISCOVERY_TIMEOUT, thereby removing the clash.

This is a breaking change. 3.10.1 and 3.10.2 were the only versions
that supported the TIMEOUT keyword and uses of it were likely
not working as intended.

Fixes: #17801
parent c1e087a9
......@@ -139,7 +139,8 @@ Modules
This is robust against unusual ways of labeling tests, provides much better
support for advanced features such as parameterized tests, and does not
require re-running CMake to discover added or removed tests within a test
executable.
executable. Note that a breaking change was made in CMake 3.10.3 to address
an ambiguity of the ``TIMEOUT`` keyword (see :ref:`Release Notes 3.10.3`).
* The :module:`InstallRequiredSystemLibraries` module gained support
for installing Intel compiler runtimes.
......@@ -267,3 +268,17 @@ Changes made since CMake 3.10.0 include the following.
* The :manual:`cmake-server(7)` ``codemodel`` response ``crossReferences``
field added by 3.10.0 has been dropped due to excessive memory usage.
Another approach will be needed to provide backtrace information.
.. _`Release Notes 3.10.3`:
3.10.3
------
* CMake 3.10.1 added a ``TIMEOUT`` option to :command:`gtest_discover_tests`
from the :module:`GoogleTest` module. That keyword clashed with the
``TIMEOUT`` test property, which is one of the common properties that
would be set with the command's ``PROPERTIES`` keyword, usually leading
to legal but unintended behavior. The keyword was changed to
``DISCOVERY_TIMEOUT`` in CMake 3.10.3 to address this problem. The
ambiguous behavior of the :command:`gtest_discover_tests` command's
``TIMEOUT`` keyword in 3.10.1 and 3.10.2 has not been preserved.
......@@ -150,6 +150,7 @@ same as the Google Test name (i.e. ``suite.testcase``); see also
[NO_PRETTY_TYPES] [NO_PRETTY_VALUES]
[PROPERTIES name1 value1...]
[TEST_LIST var]
[DISCOVERY_TIMEOUT seconds]
)
``gtest_discover_tests`` sets up a post-build command on the test executable
......@@ -217,7 +218,7 @@ same as the Google Test name (i.e. ``suite.testcase``); see also
executable is being used in multiple calls to ``gtest_discover_tests()``.
Note that this variable is only available in CTest.
``TIMEOUT num``
``DISCOVERY_TIMEOUT num``
Specifies how long (in seconds) CMake will wait for the test to enumerate
available tests. If the test takes longer than this, discovery (and your
build) will fail. Most test executables will enumerate their tests very
......@@ -225,6 +226,16 @@ same as the Google Test name (i.e. ``suite.testcase``); see also
longer timeout. The default is 5. See also the ``TIMEOUT`` option of
:command:`execute_process`.
.. note::
In CMake versions 3.10.1 and 3.10.2, this option was called ``TIMEOUT``.
This clashed with the ``TIMEOUT`` test property, which is one of the
common properties that would be set with the ``PROPERTIES`` keyword,
usually leading to legal but unintended behavior. The keyword was
changed to ``DISCOVERY_TIMEOUT`` in CMake 3.10.3 to address this
problem. The ambiguous behavior of the ``TIMEOUT`` keyword in 3.10.1
and 3.10.2 has not been preserved.
#]=======================================================================]
#------------------------------------------------------------------------------
......@@ -357,7 +368,7 @@ function(gtest_discover_tests TARGET)
cmake_parse_arguments(
""
"NO_PRETTY_TYPES;NO_PRETTY_VALUES"
"TEST_PREFIX;TEST_SUFFIX;WORKING_DIRECTORY;TEST_LIST;TIMEOUT"
"TEST_PREFIX;TEST_SUFFIX;WORKING_DIRECTORY;TEST_LIST;DISCOVERY_TIMEOUT"
"EXTRA_ARGS;PROPERTIES"
${ARGN}
)
......@@ -368,8 +379,8 @@ function(gtest_discover_tests TARGET)
if(NOT _TEST_LIST)
set(_TEST_LIST ${TARGET}_TESTS)
endif()
if(NOT _TIMEOUT)
set(_TIMEOUT 5)
if(NOT _DISCOVERY_TIMEOUT)
set(_DISCOVERY_TIMEOUT 5)
endif()
get_property(
......@@ -418,7 +429,7 @@ function(gtest_discover_tests TARGET)
-D "NO_PRETTY_VALUES=${_NO_PRETTY_VALUES}"
-D "TEST_LIST=${_TEST_LIST}"
-D "CTEST_FILE=${ctest_tests_file}"
-D "TEST_DISCOVERY_TIMEOUT=${_TIMEOUT}"
-D "TEST_DISCOVERY_TIMEOUT=${_DISCOVERY_TIMEOUT}"
-P "${_GOOGLETEST_DISCOVER_TESTS_SCRIPT}"
VERBATIM
)
......
( *|[0-9]+>)CMake Error at .*GoogleTestAddTests.cmake:[0-9]+ \(message\):
( *|[0-9]+>) Error running test executable.
?( *|[0-9]+>)
( *|[0-9]+>) Path: '.*timeout_test(\.exe)?'
( *|[0-9]+>) Path: '.*discovery_timeout_test(\.exe)?'
( *|[0-9]+>) Result: Process terminated due to timeout
( *|[0-9]+>) Output:
( *|[0-9]+>) +
Test project .*GoogleTest-build
[ \t]*Start [0-9]+: property_timeout.case_no_discovery
[^\n]+property_timeout.case_no_discovery +\.+\*\*\*Timeout +[0-9.]+ sec
+
0% tests passed, 1 tests failed out of 1
+
Total Test time \(real\) = +[0-9.]+ sec
+
The following tests FAILED:
[^\n]*property_timeout.case_no_discovery \(Timeout\)
Test project .*GoogleTest-build
[ \t]*Start [0-9]+: property_timeout.case_with_discovery
[^\n]+property_timeout.case_with_discovery +\.+\*\*\*Timeout +[0-9.]+ sec
+
0% tests passed, 1 tests failed out of 1
+
Total Test time \(real\) = +[0-9.]+ sec
+
The following tests FAILED:
[^\n]*property_timeout.case_with_discovery \(Timeout\)
Unable to find executable: timeout_test_NOT_BUILT
Unable to find executable: no_tests_defined_NOT_BUILT
Errors while running CTest
......@@ -22,8 +22,38 @@ gtest_discover_tests(
PROPERTIES LABELS TEST2
)
add_executable(timeout_test timeout_test.cpp)
add_executable(no_tests_defined no_tests_defined.cpp)
gtest_discover_tests(
timeout_test
no_tests_defined
)
# Note change in behavior of TIMEOUT keyword in 3.10.3
# where it was renamed to DISCOVERY_TIMEOUT to prevent it
# from shadowing the TIMEOUT test property. Verify the
# 3.10.3 and later behavior, old behavior added in 3.10.1
# is not supported.
add_executable(property_timeout_test timeout_test.cpp)
target_compile_definitions(property_timeout_test PRIVATE sleepSec=10)
gtest_discover_tests(
property_timeout_test
TEST_PREFIX property_
TEST_SUFFIX _no_discovery
PROPERTIES TIMEOUT 2
)
gtest_discover_tests(
property_timeout_test
TEST_PREFIX property_
TEST_SUFFIX _with_discovery
DISCOVERY_TIMEOUT 20
PROPERTIES TIMEOUT 2
)
add_executable(discovery_timeout_test timeout_test.cpp)
target_compile_definitions(discovery_timeout_test PRIVATE discoverySleepSec=10)
gtest_discover_tests(
discovery_timeout_test
TEST_PREFIX discovery_
DISCOVERY_TIMEOUT 2
)
......@@ -19,12 +19,19 @@ function(run_GoogleTest)
--target fake_gtest
)
run_cmake_command(GoogleTest-property-timeout-exe
${CMAKE_COMMAND}
--build .
--config Debug
--target property_timeout_test
)
set(RunCMake_TEST_OUTPUT_MERGE 1)
run_cmake_command(GoogleTest-timeout
run_cmake_command(GoogleTest-discovery-timeout
${CMAKE_COMMAND}
--build .
--config Debug
--target timeout_test
--target discovery_timeout_test
)
set(RunCMake_TEST_OUTPUT_MERGE 0)
......@@ -45,7 +52,21 @@ function(run_GoogleTest)
run_cmake_command(GoogleTest-test-missing
${CMAKE_CTEST_COMMAND}
-C Debug
-R timeout
-R no_tests_defined
--no-label-summary
)
run_cmake_command(GoogleTest-property-timeout1
${CMAKE_CTEST_COMMAND}
-C Debug
-R property_timeout\\.case_no_discovery
--no-label-summary
)
run_cmake_command(GoogleTest-property-timeout2
${CMAKE_CTEST_COMMAND}
-C Debug
-R property_timeout\\.case_with_discovery
--no-label-summary
)
endfunction()
......
......@@ -4,12 +4,36 @@
#include <unistd.h>
#endif
int main()
#include <iostream>
#include <string>
void sleepFor(unsigned seconds)
{
#if defined(_WIN32)
Sleep(10000);
Sleep(seconds * 1000);
#else
sleep(10);
sleep(seconds);
#endif
}
int main(int argc, char** argv)
{
// Note: GoogleTest.cmake doesn't actually depend on Google Test as such;
// it only requires that we produce output in the expected format when
// invoked with --gtest_list_tests. Thus, we fake that here. This allows us
// to test the module without actually needing Google Test.
if (argc > 1 && std::string(argv[1]) == "--gtest_list_tests") {
std::cout << "timeout." << std::endl;
std::cout << " case" << std::endl;
#ifdef discoverySleepSec
sleepFor(discoverySleepSec);
#endif
return 0;
}
#ifdef sleepSec
sleepFor(sleepSec);
#endif
return 0;
}
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment