summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRyan Thornton <ThorntonRyan@JohnDeere.com>2020-03-16 19:40:31 (GMT)
committerRyan Thornton <ThorntonRyan@JohnDeere.com>2020-03-27 14:39:47 (GMT)
commit1ba4cb565eb0bf16a717863ed4945e93468cb146 (patch)
tree410cad503a8e20fb839d45947866d989204dcee9
parent75e82a13db7627ad250baa48f315786e34330995 (diff)
downloadCMake-1ba4cb565eb0bf16a717863ed4945e93468cb146.zip
CMake-1ba4cb565eb0bf16a717863ed4945e93468cb146.tar.gz
CMake-1ba4cb565eb0bf16a717863ed4945e93468cb146.tar.bz2
GoogleTest: Parameterize tests to check PRE_TEST/POST_BUILD discovery mode
Now, the unit tests are ran twice -- once with POST_BUILD (i.e. default mode) and again with PRE_TEST (i.e. new discovery mode). Both modes of setting gtest discovery mode are also tested: 1. Using the global override (i.e. CMAKE_GTEST_DISCOVER_TESTS_DISCOVERY_MODE) 2. Explicitly passing DISCOVERY_MODE in calls to gtest_discover_tests (in GoogleTestDiscoveryTimeout.cmake) The goal is to show that the new PRE_TEST discovery mode does not break existing behavior (i.e. should not break POST_BUILD mode) and should also pass the same tests in the same way. A few non trivial implementation details worth noting: 1. Refactoring discovery_timeout_test into own project Originally, I tried doing: ``` run_GoogleTest(POST_BUILD) run_GoogleTest(PRE_TEST) ``` Without changing the internal structure of run_GoogleTest. But since discovery_timeout_test is part of the same project as the other tests, and CTest include files always get evaluated and that's where test discovery occurs, this means every other test now notices the timeout problem when running in PRE_TEST mode. As a result, keeping the existing test structure meant that each existing test (and any new test) would need to have its own PRE_TEST / POST_BUILD variant for stderr and stdout in order to handle the case where discovery_timeout_test timed out. This exponential increase in test output files introduced unnecessary complexity and made it more cumbersome to work on test cases. Why should an unrelated test case care about discovery_timeout_test? So, to fix that issue, the tests were broken apart into two main groups: 1. run_GoogleTest_discovery_timeout (the test dealing with discovery_timeout_test) 2. run_GoogleTest (everything else) This isolates the PRE_TEST / POST_BUILD timeout variants to a single test case. And the other test cases remain unchanged -- further driving home the point that DISCOVERY_MODE shouldn't change existing behavior. 2. Different number of PRE_TEST / POST_BUILD file variants On the PRE_TEST path, different build systems / compilers (i.e. MSBuild and ninja/gcc) produces different build output when building discovery_timeout_test, but we don't actually care what it is, just as long as it builds successfully. This the fundamental difference in behavior between POST_BUILD (which would have failed) and PRE_TEST (which doesn't) and is the reason why we don't need a GoogleTest-discovery-build-result.txt or GoogleTest-discovery-build-stdout.txt 3. Fix flaky discovery timeout test The test expects to see: > Output: > timeout > case. But sometimes, the test would only produce: > Output: > timout In certain environments, specifically when built with OpenWatcom 1.4, and while the build server was under heavy load (i.e. running many tests in parallel), std::endl behaves inconsistently and doesn't completely flush std::cout when the program is terminated due to timeout. This results in inconsistent test failures because the actual output doesn't fully match what's expected. At first we tried adding an additional: std::cout << std::flush That didn't work. But using C-style printf() and fflush() appears to do the trick: > This time I managed to get on the machine while it was still busy doing other nightly builds > and could reproduce the problem reliably. With that I was finally able to find a fix. > It turns out my earlier hypothesis that C++ stream flushing was not working on the old compiler was correct, > but even .flush() is not enough. > I changed it to use C-style printf() and fflush() and now the test passes on that build. > -- Brad King <brad.king@kitware.com> Co-authored-by: Ryan Thornton <ThorntonRyan@JohnDeere.com> Co-authored-by: Kevin Puetz <PuetzKevinA@JohnDeere.com>
-rw-r--r--Tests/RunCMake/GoogleTest/GoogleTest-discovery-POST_BUILD-timeout-build-result.txt (renamed from Tests/RunCMake/GoogleTest/GoogleTest-discovery-timeout-test-result.txt)0
-rw-r--r--Tests/RunCMake/GoogleTest/GoogleTest-discovery-POST_BUILD-timeout-build-stdout.txt (renamed from Tests/RunCMake/GoogleTest/GoogleTest-discovery-timeout-build-stdout.txt)0
-rw-r--r--Tests/RunCMake/GoogleTest/GoogleTest-discovery-POST_BUILD-timeout-test-result.txt (renamed from Tests/RunCMake/GoogleTest/GoogleTest-discovery-timeout-build-result.txt)0
-rw-r--r--Tests/RunCMake/GoogleTest/GoogleTest-discovery-POST_BUILD-timeout-test-stderr.txt (renamed from Tests/RunCMake/GoogleTest/GoogleTest-discovery-timeout-test-stderr.txt)0
-rw-r--r--Tests/RunCMake/GoogleTest/GoogleTest-discovery-POST_BUILD-timeout-test-stdout.txt (renamed from Tests/RunCMake/GoogleTest/GoogleTest-discovery-timeout-test-stdout.txt)2
-rw-r--r--Tests/RunCMake/GoogleTest/GoogleTest-discovery-PRE_TEST-timeout-test-stderr.txt8
-rw-r--r--Tests/RunCMake/GoogleTest/GoogleTest-discovery-PRE_TEST-timeout-test-stdout.txt1
-rw-r--r--Tests/RunCMake/GoogleTest/GoogleTest.cmake8
-rw-r--r--Tests/RunCMake/GoogleTest/GoogleTestDiscoveryTimeout.cmake14
-rw-r--r--Tests/RunCMake/GoogleTest/RunCMakeTest.cmake62
-rw-r--r--Tests/RunCMake/GoogleTest/timeout_test.cpp7
11 files changed, 68 insertions, 34 deletions
diff --git a/Tests/RunCMake/GoogleTest/GoogleTest-discovery-timeout-test-result.txt b/Tests/RunCMake/GoogleTest/GoogleTest-discovery-POST_BUILD-timeout-build-result.txt
index d197c91..d197c91 100644
--- a/Tests/RunCMake/GoogleTest/GoogleTest-discovery-timeout-test-result.txt
+++ b/Tests/RunCMake/GoogleTest/GoogleTest-discovery-POST_BUILD-timeout-build-result.txt
diff --git a/Tests/RunCMake/GoogleTest/GoogleTest-discovery-timeout-build-stdout.txt b/Tests/RunCMake/GoogleTest/GoogleTest-discovery-POST_BUILD-timeout-build-stdout.txt
index 3a6572c..3a6572c 100644
--- a/Tests/RunCMake/GoogleTest/GoogleTest-discovery-timeout-build-stdout.txt
+++ b/Tests/RunCMake/GoogleTest/GoogleTest-discovery-POST_BUILD-timeout-build-stdout.txt
diff --git a/Tests/RunCMake/GoogleTest/GoogleTest-discovery-timeout-build-result.txt b/Tests/RunCMake/GoogleTest/GoogleTest-discovery-POST_BUILD-timeout-test-result.txt
index d197c91..d197c91 100644
--- a/Tests/RunCMake/GoogleTest/GoogleTest-discovery-timeout-build-result.txt
+++ b/Tests/RunCMake/GoogleTest/GoogleTest-discovery-POST_BUILD-timeout-test-result.txt
diff --git a/Tests/RunCMake/GoogleTest/GoogleTest-discovery-timeout-test-stderr.txt b/Tests/RunCMake/GoogleTest/GoogleTest-discovery-POST_BUILD-timeout-test-stderr.txt
index f6be939..f6be939 100644
--- a/Tests/RunCMake/GoogleTest/GoogleTest-discovery-timeout-test-stderr.txt
+++ b/Tests/RunCMake/GoogleTest/GoogleTest-discovery-POST_BUILD-timeout-test-stderr.txt
diff --git a/Tests/RunCMake/GoogleTest/GoogleTest-discovery-timeout-test-stdout.txt b/Tests/RunCMake/GoogleTest/GoogleTest-discovery-POST_BUILD-timeout-test-stdout.txt
index d4c4e7b..d9de3f8 100644
--- a/Tests/RunCMake/GoogleTest/GoogleTest-discovery-timeout-test-stdout.txt
+++ b/Tests/RunCMake/GoogleTest/GoogleTest-discovery-POST_BUILD-timeout-test-stdout.txt
@@ -1,4 +1,4 @@
-Test project .*GoogleTest-build
+Test project .*GoogleTest-discovery-timeout
[ \t]*Start [0-9]+: discovery_timeout_test_NOT_BUILT
Could not find executable discovery_timeout_test_NOT_BUILT
Looked in the following places:
diff --git a/Tests/RunCMake/GoogleTest/GoogleTest-discovery-PRE_TEST-timeout-test-stderr.txt b/Tests/RunCMake/GoogleTest/GoogleTest-discovery-PRE_TEST-timeout-test-stderr.txt
new file mode 100644
index 0000000..75afe4a
--- /dev/null
+++ b/Tests/RunCMake/GoogleTest/GoogleTest-discovery-PRE_TEST-timeout-test-stderr.txt
@@ -0,0 +1,8 @@
+CMake Error at .*GoogleTestAddTests.cmake:[0-9]+ \(message\):
+[ \t]*Error running test executable.
++
+[ \t]*Path: '.*discovery_timeout_test(\.exe)?'
+[ \t]*Result: Process terminated due to timeout
+[ \t]*Output:
+[ \t]*timeout.
+[ \t]*case
diff --git a/Tests/RunCMake/GoogleTest/GoogleTest-discovery-PRE_TEST-timeout-test-stdout.txt b/Tests/RunCMake/GoogleTest/GoogleTest-discovery-PRE_TEST-timeout-test-stdout.txt
new file mode 100644
index 0000000..d65061f
--- /dev/null
+++ b/Tests/RunCMake/GoogleTest/GoogleTest-discovery-PRE_TEST-timeout-test-stdout.txt
@@ -0,0 +1 @@
+Test project .*GoogleTest-discovery-timeout
diff --git a/Tests/RunCMake/GoogleTest/GoogleTest.cmake b/Tests/RunCMake/GoogleTest/GoogleTest.cmake
index 31808c6..4bc6b9d 100644
--- a/Tests/RunCMake/GoogleTest/GoogleTest.cmake
+++ b/Tests/RunCMake/GoogleTest/GoogleTest.cmake
@@ -49,11 +49,3 @@ gtest_discover_tests(
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
-)
diff --git a/Tests/RunCMake/GoogleTest/GoogleTestDiscoveryTimeout.cmake b/Tests/RunCMake/GoogleTest/GoogleTestDiscoveryTimeout.cmake
new file mode 100644
index 0000000..7398faf
--- /dev/null
+++ b/Tests/RunCMake/GoogleTest/GoogleTestDiscoveryTimeout.cmake
@@ -0,0 +1,14 @@
+project(test_include_dirs)
+include(CTest)
+include(GoogleTest)
+
+enable_testing()
+
+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
+ DISCOVERY_MODE ${DISCOVERY_MODE}
+)
diff --git a/Tests/RunCMake/GoogleTest/RunCMakeTest.cmake b/Tests/RunCMake/GoogleTest/RunCMakeTest.cmake
index 8070512..97a777b 100644
--- a/Tests/RunCMake/GoogleTest/RunCMakeTest.cmake
+++ b/Tests/RunCMake/GoogleTest/RunCMakeTest.cmake
@@ -1,6 +1,6 @@
include(RunCMake)
-function(run_GoogleTest)
+function(run_GoogleTest DISCOVERY_MODE)
# Use a single build tree for a few tests without cleaning.
set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/GoogleTest-build)
set(RunCMake_TEST_NO_CLEAN 1)
@@ -10,7 +10,7 @@ function(run_GoogleTest)
file(REMOVE_RECURSE "${RunCMake_TEST_BINARY_DIR}")
file(MAKE_DIRECTORY "${RunCMake_TEST_BINARY_DIR}")
- run_cmake(GoogleTest)
+ run_cmake_with_options(GoogleTest -DCMAKE_GTEST_DISCOVER_TESTS_DISCOVERY_MODE=${DISCOVERY_MODE})
run_cmake_command(GoogleTest-build
${CMAKE_COMMAND}
@@ -26,15 +26,6 @@ function(run_GoogleTest)
--target property_timeout_test
)
- set(RunCMake_TEST_OUTPUT_MERGE 1)
- run_cmake_command(GoogleTest-discovery-timeout-build
- ${CMAKE_COMMAND}
- --build .
- --config Debug
- --target discovery_timeout_test
- )
- set(RunCMake_TEST_OUTPUT_MERGE 0)
-
run_cmake_command(GoogleTest-test1
${CMAKE_CTEST_COMMAND}
-C Debug
@@ -69,16 +60,9 @@ function(run_GoogleTest)
-R property_timeout\\.case_with_discovery
--no-label-summary
)
-
- run_cmake_command(GoogleTest-discovery-timeout-test
- ${CMAKE_CTEST_COMMAND}
- -C Debug
- -R discovery_timeout_test
- --no-label-summary
- )
endfunction()
-function(run_GoogleTestXML)
+function(run_GoogleTestXML DISCOVERY_MODE)
# Use a single build tree for a few tests without cleaning.
set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/GoogleTestXML-build)
set(RunCMake_TEST_NO_CLEAN 1)
@@ -88,7 +72,7 @@ function(run_GoogleTestXML)
file(REMOVE_RECURSE "${RunCMake_TEST_BINARY_DIR}")
file(MAKE_DIRECTORY "${RunCMake_TEST_BINARY_DIR}")
- run_cmake(GoogleTestXML)
+ run_cmake_with_options(GoogleTestXML -DCMAKE_GTEST_DISCOVER_TESTS_DISCOVERY_MODE=${DISCOVERY_MODE})
run_cmake_command(GoogleTestXML-discovery
${CMAKE_COMMAND}
@@ -105,5 +89,39 @@ function(run_GoogleTestXML)
)
endfunction()
-run_GoogleTest()
-run_GoogleTestXML()
+function(run_GoogleTest_discovery_timeout DISCOVERY_MODE)
+ # Use a single build tree for a few tests without cleaning.
+ set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/GoogleTest-discovery-timeout)
+ set(RunCMake_TEST_NO_CLEAN 1)
+ if(NOT RunCMake_GENERATOR_IS_MULTI_CONFIG)
+ set(RunCMake_TEST_OPTIONS -DCMAKE_BUILD_TYPE=Debug)
+ endif()
+ file(REMOVE_RECURSE "${RunCMake_TEST_BINARY_DIR}")
+ file(MAKE_DIRECTORY "${RunCMake_TEST_BINARY_DIR}")
+
+ run_cmake_with_options(GoogleTestDiscoveryTimeout -DDISCOVERY_MODE=${DISCOVERY_MODE})
+
+ set(RunCMake_TEST_OUTPUT_MERGE 1)
+ run_cmake_command(GoogleTest-discovery-${DISCOVERY_MODE}-timeout-build
+ ${CMAKE_COMMAND}
+ --build .
+ --config Debug
+ --target discovery_timeout_test
+ )
+ set(RunCMake_TEST_OUTPUT_MERGE 0)
+
+ run_cmake_command(GoogleTest-discovery-${DISCOVERY_MODE}-timeout-test
+ ${CMAKE_CTEST_COMMAND}
+ -C Debug
+ -R discovery_timeout_test
+ --no-label-sumary
+ )
+endfunction()
+
+foreach(DISCOVERY_MODE POST_BUILD PRE_TEST)
+ message("Testing ${DISCOVERY_MODE} discovery mode via CMAKE_GTEST_DISCOVER_TESTS_DISCOVERY_MODE global override...")
+ run_GoogleTest(${DISCOVERY_MODE})
+ run_GoogleTestXML(${DISCOVERY_MODE})
+ message("Testing ${DISCOVERY_MODE} discovery mode via DISCOVERY_MODE option...")
+ run_GoogleTest_discovery_timeout(${DISCOVERY_MODE})
+endforeach()
diff --git a/Tests/RunCMake/GoogleTest/timeout_test.cpp b/Tests/RunCMake/GoogleTest/timeout_test.cpp
index b8ad055..5506269 100644
--- a/Tests/RunCMake/GoogleTest/timeout_test.cpp
+++ b/Tests/RunCMake/GoogleTest/timeout_test.cpp
@@ -4,9 +4,10 @@
# include <unistd.h>
#endif
-#include <iostream>
#include <string>
+#include <stdio.h>
+
void sleepFor(unsigned seconds)
{
#if defined(_WIN32)
@@ -23,8 +24,8 @@ int main(int argc, char** argv)
// 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;
+ printf("timeout.\n case\n");
+ fflush(stdout);
#ifdef discoverySleepSec
sleepFor(discoverySleepSec);
#endif