From cd4038fe94caa6f9a18291081a6e8801b91a2368 Mon Sep 17 00:00:00 2001 From: Brad King Date: Wed, 3 May 2023 13:12:06 -0400 Subject: cmCTestTestHandler: Use in-class initialization of properties and results --- Source/CTest/cmCTestRunTest.cxx | 5 ----- Source/CTest/cmCTestTestHandler.cxx | 11 ----------- Source/CTest/cmCTestTestHandler.h | 34 +++++++++++++++++----------------- 3 files changed, 17 insertions(+), 33 deletions(-) diff --git a/Source/CTest/cmCTestRunTest.cxx b/Source/CTest/cmCTestRunTest.cxx index 5efe69f..23f437a 100644 --- a/Source/CTest/cmCTestRunTest.cxx +++ b/Source/CTest/cmCTestRunTest.cxx @@ -30,11 +30,6 @@ cmCTestRunTest::cmCTestRunTest(cmCTestMultiProcessHandler& multiHandler) { this->CTest = multiHandler.CTest; this->TestHandler = multiHandler.TestHandler; - this->TestResult.ExecutionTime = cmDuration::zero(); - this->TestResult.ReturnValue = 0; - this->TestResult.Status = cmCTestTestHandler::NOT_RUN; - this->TestResult.TestCount = 0; - this->TestResult.Properties = nullptr; } void cmCTestRunTest::CheckOutput(std::string const& line) diff --git a/Source/CTest/cmCTestTestHandler.cxx b/Source/CTest/cmCTestTestHandler.cxx index 3a1cb64..bfc6bd6 100644 --- a/Source/CTest/cmCTestTestHandler.cxx +++ b/Source/CTest/cmCTestTestHandler.cxx @@ -2431,17 +2431,6 @@ bool cmCTestTestHandler::AddTest(const std::vector& args) "Set test directory: " << test.Directory << std::endl, this->Quiet); - test.IsInBasedOnREOptions = true; - test.WillFail = false; - test.Disabled = false; - test.RunSerial = false; - test.Timeout = cmDuration::zero(); - test.ExplicitTimeout = false; - test.Cost = 0; - test.Processors = 1; - test.WantAffinity = false; - test.SkipReturnCode = -1; - test.PreviousRuns = 0; if (this->UseIncludeRegExpFlag && (!this->IncludeTestsRegularExpression.find(testname) || (!this->UseExcludeRegExpFirst && diff --git a/Source/CTest/cmCTestTestHandler.h b/Source/CTest/cmCTestTestHandler.h index d0049da..29d24e0 100644 --- a/Source/CTest/cmCTestTestHandler.h +++ b/Source/CTest/cmCTestTestHandler.h @@ -139,22 +139,22 @@ public: std::vector> TimeoutRegularExpressions; std::map Measurements; - bool IsInBasedOnREOptions; - bool WillFail; - bool Disabled; - float Cost; - int PreviousRuns; - bool RunSerial; - cmDuration Timeout; - bool ExplicitTimeout; + bool IsInBasedOnREOptions = true; + bool WillFail = false; + bool Disabled = false; + float Cost = 0; + int PreviousRuns = 0; + bool RunSerial = false; + cmDuration Timeout = cmDuration::zero(); + bool ExplicitTimeout = false; cmDuration AlternateTimeout; - int Index; + int Index = 0; // Requested number of process slots - int Processors; - bool WantAffinity; + int Processors = 1; + bool WantAffinity = false; std::vector Affinity; // return code of test which will mark test as "not run" - int SkipReturnCode; + int SkipReturnCode = -1; std::vector Environment; std::vector EnvironmentModification; std::vector Labels; @@ -175,17 +175,17 @@ public: std::string Reason; std::string FullCommandLine; std::string Environment; - cmDuration ExecutionTime; - std::int64_t ReturnValue; - int Status; + cmDuration ExecutionTime = cmDuration::zero(); + std::int64_t ReturnValue = 0; + int Status = NOT_RUN; std::string ExceptionStatus; bool CompressOutput; std::string CompletionStatus; std::string CustomCompletionStatus; std::string Output; std::string TestMeasurementsOutput; - int TestCount; - cmCTestTestProperties* Properties; + int TestCount = 0; + cmCTestTestProperties* Properties = nullptr; }; struct cmCTestTestResultLess -- cgit v0.12 From 39a20a56ddcde1eecd44b32ab0610b579a7ac4eb Mon Sep 17 00:00:00 2001 From: Brad King Date: Wed, 3 May 2023 10:29:05 -0400 Subject: Tests: Move `CTestTestZeroTimeout` into `RunCMake.CTestTimeout` --- Tests/CMakeLists.txt | 11 ----------- Tests/CTestTestZeroTimeout/CMakeLists.txt | 8 -------- Tests/CTestTestZeroTimeout/CTestConfig.cmake | 4 ---- Tests/CTestTestZeroTimeout/sleep.c | 16 ---------------- Tests/CTestTestZeroTimeout/test.cmake.in | 22 ---------------------- Tests/RunCMake/CMakeLists.txt | 5 ++++- Tests/RunCMake/CTestTimeout/CMakeLists.txt.in | 2 +- Tests/RunCMake/CTestTimeout/RunCMakeTest.cmake | 12 +++++++++++- .../CTestTimeout/ZeroOverridesVar-stdout.txt | 6 ++++++ 9 files changed, 22 insertions(+), 64 deletions(-) delete mode 100644 Tests/CTestTestZeroTimeout/CMakeLists.txt delete mode 100644 Tests/CTestTestZeroTimeout/CTestConfig.cmake delete mode 100644 Tests/CTestTestZeroTimeout/sleep.c delete mode 100644 Tests/CTestTestZeroTimeout/test.cmake.in create mode 100644 Tests/RunCMake/CTestTimeout/ZeroOverridesVar-stdout.txt diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt index e3b5ec4..2fa962e 100644 --- a/Tests/CMakeLists.txt +++ b/Tests/CMakeLists.txt @@ -3192,17 +3192,6 @@ if(BUILD_TESTING) WORKING_DIRECTORY ${CMake_BINARY_DIR}/Tests/CTestTestTimeout) configure_file( - "${CMake_SOURCE_DIR}/Tests/CTestTestZeroTimeout/test.cmake.in" - "${CMake_BINARY_DIR}/Tests/CTestTestZeroTimeout/test.cmake" - @ONLY ESCAPE_QUOTES) - add_test(CTestTestZeroTimeout ${CMAKE_CTEST_COMMAND} - -S "${CMake_BINARY_DIR}/Tests/CTestTestZeroTimeout/test.cmake" -V - --output-log - "${CMake_BINARY_DIR}/Tests/CTestTestZeroTimeout/testOutput.log") - set_tests_properties(CTestTestZeroTimeout PROPERTIES - FAIL_REGULAR_EXPRESSION "\\*\\*\\*Timeout") - - configure_file( "${CMake_SOURCE_DIR}/Tests/CTestTestDepends/test.cmake.in" "${CMake_BINARY_DIR}/Tests/CTestTestDepends/test.cmake" @ONLY ESCAPE_QUOTES) diff --git a/Tests/CTestTestZeroTimeout/CMakeLists.txt b/Tests/CTestTestZeroTimeout/CMakeLists.txt deleted file mode 100644 index 51ef807..0000000 --- a/Tests/CTestTestZeroTimeout/CMakeLists.txt +++ /dev/null @@ -1,8 +0,0 @@ -cmake_minimum_required (VERSION 3.5) -project (CTestTestZeroTimeout) -include (CTest) - -add_executable (Sleep sleep.c) - -add_test (TestExplicitZeroTimeout Sleep) -set_tests_properties(TestExplicitZeroTimeout PROPERTIES TIMEOUT 0) diff --git a/Tests/CTestTestZeroTimeout/CTestConfig.cmake b/Tests/CTestTestZeroTimeout/CTestConfig.cmake deleted file mode 100644 index bd265f9..0000000 --- a/Tests/CTestTestZeroTimeout/CTestConfig.cmake +++ /dev/null @@ -1,4 +0,0 @@ -set(CTEST_NIGHTLY_START_TIME "21:00:00 EDT") -set(CTEST_DROP_METHOD "http") -set(CTEST_DROP_SITE "open.cdash.org") -set(CTEST_DROP_LOCATION "/submit.php?project=PublicDashboard") diff --git a/Tests/CTestTestZeroTimeout/sleep.c b/Tests/CTestTestZeroTimeout/sleep.c deleted file mode 100644 index 5d0b89b..0000000 --- a/Tests/CTestTestZeroTimeout/sleep.c +++ /dev/null @@ -1,16 +0,0 @@ -#if defined(_WIN32) -# include -#else -# include -#endif - -/* sleeps for 5 seconds */ -int main(int argc, char** argv) -{ -#if defined(_WIN32) - Sleep(5000); -#else - sleep(5); -#endif - return 0; -} diff --git a/Tests/CTestTestZeroTimeout/test.cmake.in b/Tests/CTestTestZeroTimeout/test.cmake.in deleted file mode 100644 index e0dbbc6..0000000 --- a/Tests/CTestTestZeroTimeout/test.cmake.in +++ /dev/null @@ -1,22 +0,0 @@ -cmake_minimum_required(VERSION 3.5) - -# Settings: -set(CTEST_DASHBOARD_ROOT "@CMake_BINARY_DIR@/Tests/CTestTest") -set(CTEST_SITE "@SITE@") -set(CTEST_BUILD_NAME "CTestTest-@BUILDNAME@-ZeroTimeout") - -set(CTEST_SOURCE_DIRECTORY "@CMake_SOURCE_DIR@/Tests/CTestTestZeroTimeout") -set(CTEST_BINARY_DIRECTORY "@CMake_BINARY_DIR@/Tests/CTestTestZeroTimeout") -set(CTEST_CVS_COMMAND "@CVSCOMMAND@") -set(CTEST_CMAKE_GENERATOR "@CMAKE_GENERATOR@") -set(CTEST_CMAKE_GENERATOR_PLATFORM "@CMAKE_GENERATOR_PLATFORM@") -set(CTEST_CMAKE_GENERATOR_TOOLSET "@CMAKE_GENERATOR_TOOLSET@") -set(CTEST_BUILD_CONFIGURATION "$ENV{CMAKE_CONFIG_TYPE}") -set(CTEST_COVERAGE_COMMAND "@COVERAGE_COMMAND@") -set(CTEST_NOTES_FILES "${CTEST_SCRIPT_DIRECTORY}/${CTEST_SCRIPT_NAME}") -set(CTEST_TEST_TIMEOUT 2) - -CTEST_START(Experimental) -CTEST_CONFIGURE(BUILD "${CTEST_BINARY_DIRECTORY}" RETURN_VALUE res) -CTEST_BUILD(BUILD "${CTEST_BINARY_DIRECTORY}" RETURN_VALUE res) -CTEST_TEST(BUILD "${CTEST_BINARY_DIRECTORY}" RETURN_VALUE res) diff --git a/Tests/RunCMake/CMakeLists.txt b/Tests/RunCMake/CMakeLists.txt index ada9132..3007660 100644 --- a/Tests/RunCMake/CMakeLists.txt +++ b/Tests/RunCMake/CMakeLists.txt @@ -562,7 +562,10 @@ add_RunCMake_test(alias_targets) add_RunCMake_test(InterfaceLibrary) add_RunCMake_test(no_install_prefix) add_RunCMake_test(configure_file) -add_RunCMake_test(CTestTimeout -DTIMEOUT=${CTestTestTimeout_TIME}) +if(CTestTestTimeout_TIME) + set(CTestTimeout_ARGS -DTIMEOUT=${CTestTestTimeout_TIME}) +endif() +add_RunCMake_test(CTestTimeout) add_RunCMake_test(CTestTimeoutAfterMatch) if(CMake_TEST_CUDA) add_RunCMake_test(CUDA_architectures) diff --git a/Tests/RunCMake/CTestTimeout/CMakeLists.txt.in b/Tests/RunCMake/CTestTimeout/CMakeLists.txt.in index 20faa94..ee3323c 100644 --- a/Tests/RunCMake/CTestTimeout/CMakeLists.txt.in +++ b/Tests/RunCMake/CTestTimeout/CMakeLists.txt.in @@ -4,7 +4,7 @@ include(CTest) add_executable(TestTimeout TestTimeout.c) -if(NOT TIMEOUT) +if(NOT DEFINED TIMEOUT) set(TIMEOUT 4) endif() target_compile_definitions(TestTimeout PRIVATE TIMEOUT=${TIMEOUT}) diff --git a/Tests/RunCMake/CTestTimeout/RunCMakeTest.cmake b/Tests/RunCMake/CTestTimeout/RunCMakeTest.cmake index 7e96b6d..e55ba27 100644 --- a/Tests/RunCMake/CTestTimeout/RunCMakeTest.cmake +++ b/Tests/RunCMake/CTestTimeout/RunCMakeTest.cmake @@ -1,6 +1,6 @@ include(RunCTest) -if(NOT TIMEOUT) +if(NOT DEFINED TIMEOUT) # Give the process time to load and start running. set(TIMEOUT 4) endif() @@ -20,3 +20,13 @@ if(UNIX) run_ctest_timeout(Fork) unset(CASE_CMAKELISTS_SUFFIX_CODE) endif() + +block() + # An explicit zero TIMEOUT test property means "no timeout". + set(TIMEOUT 0) + # The test sleeps for 4 seconds longer than the TIMEOUT value. + # Set a default timeout to less than that so that the test will + # timeout if the zero TIMEOUT does not suppress it. + set(CASE_TEST_PREFIX_CODE "set(CTEST_TEST_TIMEOUT 2)") + run_ctest_timeout(ZeroOverridesVar) +endblock() diff --git a/Tests/RunCMake/CTestTimeout/ZeroOverridesVar-stdout.txt b/Tests/RunCMake/CTestTimeout/ZeroOverridesVar-stdout.txt new file mode 100644 index 0000000..7192055 --- /dev/null +++ b/Tests/RunCMake/CTestTimeout/ZeroOverridesVar-stdout.txt @@ -0,0 +1,6 @@ +Test project [^ +]*/Tests/RunCMake/CTestTimeout/ZeroOverridesVar-build + Start 1: TestTimeout +1/1 Test #1: TestTimeout ...................... Passed +[1-9][0-9.]* sec ++ +100% tests passed, 0 tests failed out of 1 -- cgit v0.12 From 3edf7fbb418d0da1e1ef1717b35b828d2003c427 Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 4 May 2023 16:25:48 -0400 Subject: ctest: Fix TIMEOUT test property with value 0 with --timeout flag An explicit zero TIMEOUT test property value should not be overridden by the `--timeout` flag. --- Source/CTest/cmCTestTestHandler.cxx | 2 +- Tests/RunCMake/CTestTimeout/RunCMakeTest.cmake | 3 ++- Tests/RunCMake/CTestTimeout/ZeroOverridesFlag-stdout.txt | 6 ++++++ 3 files changed, 9 insertions(+), 2 deletions(-) create mode 100644 Tests/RunCMake/CTestTimeout/ZeroOverridesFlag-stdout.txt diff --git a/Source/CTest/cmCTestTestHandler.cxx b/Source/CTest/cmCTestTestHandler.cxx index bfc6bd6..cd92e0a 100644 --- a/Source/CTest/cmCTestTestHandler.cxx +++ b/Source/CTest/cmCTestTestHandler.cxx @@ -1379,7 +1379,7 @@ bool cmCTestTestHandler::ProcessDirectory(std::vector& passed, p.Cost = static_cast(rand()); } - if (p.Timeout == cmDuration::zero() && + if (p.Timeout == cmDuration::zero() && !p.ExplicitTimeout && this->CTest->GetGlobalTimeout() != cmDuration::zero()) { p.Timeout = this->CTest->GetGlobalTimeout(); } diff --git a/Tests/RunCMake/CTestTimeout/RunCMakeTest.cmake b/Tests/RunCMake/CTestTimeout/RunCMakeTest.cmake index e55ba27..a4080e3 100644 --- a/Tests/RunCMake/CTestTimeout/RunCMakeTest.cmake +++ b/Tests/RunCMake/CTestTimeout/RunCMakeTest.cmake @@ -8,7 +8,7 @@ endif() function(run_ctest_timeout CASE_NAME) configure_file(${RunCMake_SOURCE_DIR}/TestTimeout.c ${RunCMake_BINARY_DIR}/${CASE_NAME}/TestTimeout.c COPYONLY) - run_ctest(${CASE_NAME}) + run_ctest(${CASE_NAME} ${ARGN}) endfunction() run_ctest_timeout(Basic) @@ -27,6 +27,7 @@ block() # The test sleeps for 4 seconds longer than the TIMEOUT value. # Set a default timeout to less than that so that the test will # timeout if the zero TIMEOUT does not suppress it. + run_ctest_timeout(ZeroOverridesFlag --timeout 2) set(CASE_TEST_PREFIX_CODE "set(CTEST_TEST_TIMEOUT 2)") run_ctest_timeout(ZeroOverridesVar) endblock() diff --git a/Tests/RunCMake/CTestTimeout/ZeroOverridesFlag-stdout.txt b/Tests/RunCMake/CTestTimeout/ZeroOverridesFlag-stdout.txt new file mode 100644 index 0000000..746bc21 --- /dev/null +++ b/Tests/RunCMake/CTestTimeout/ZeroOverridesFlag-stdout.txt @@ -0,0 +1,6 @@ +Test project [^ +]*/Tests/RunCMake/CTestTimeout/ZeroOverridesFlag-build + Start 1: TestTimeout +1/1 Test #1: TestTimeout ...................... Passed +[1-9][0-9.]* sec ++ +100% tests passed, 0 tests failed out of 1 -- cgit v0.12 From 07b5087ba7ac3f55ee0f739717306a028d4244e1 Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 4 May 2023 13:28:03 -0400 Subject: Help: Document meaning of TIMEOUT test property with value 0 Document the behavior added by commit 51bb493574 (Test TIMEOUT property explicitly set to zero should be honored, 2011-01-03, v2.8.4~118^2). --- Help/prop_test/TIMEOUT.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Help/prop_test/TIMEOUT.rst b/Help/prop_test/TIMEOUT.rst index d1cb90d..385539b 100644 --- a/Help/prop_test/TIMEOUT.rst +++ b/Help/prop_test/TIMEOUT.rst @@ -7,3 +7,6 @@ This property if set will limit a test to not take more than the specified number of seconds to run. If it exceeds that the test process will be killed and ctest will move to the next test. This setting takes precedence over :variable:`CTEST_TEST_TIMEOUT`. + +An explicit ``0`` value means the test has no timeout, except as +necessary to honor :option:`ctest --stop-time`. -- cgit v0.12 From 59336b29bd713c288423d8330c74b74cfc0eaf58 Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 4 May 2023 15:21:02 -0400 Subject: cmCTestRunTest: Remove unnecessary arguments to ForkProcess --- Source/CTest/cmCTestRunTest.cxx | 24 +++++++++--------------- Source/CTest/cmCTestRunTest.h | 5 +---- 2 files changed, 10 insertions(+), 19 deletions(-) diff --git a/Source/CTest/cmCTestRunTest.cxx b/Source/CTest/cmCTestRunTest.cxx index 23f437a..307312d 100644 --- a/Source/CTest/cmCTestRunTest.cxx +++ b/Source/CTest/cmCTestRunTest.cxx @@ -636,10 +636,7 @@ bool cmCTestRunTest::StartTest(size_t completed, size_t total) } } - return this->ForkProcess(timeout, this->TestProperties->ExplicitTimeout, - &this->TestProperties->Environment, - &this->TestProperties->EnvironmentModification, - &this->TestProperties->Affinity); + return this->ForkProcess(timeout); } void cmCTestRunTest::ComputeArguments() @@ -737,11 +734,7 @@ void cmCTestRunTest::ParseOutputForMeasurements() } } -bool cmCTestRunTest::ForkProcess( - cmDuration testTimeOut, bool explicitTimeout, - std::vector* environment, - std::vector* environment_modification, - std::vector* affinity) +bool cmCTestRunTest::ForkProcess(cmDuration testTimeOut) { this->TestProcess->SetId(this->Index); this->TestProcess->SetWorkingDirectory(this->TestProperties->Directory); @@ -766,7 +759,8 @@ bool cmCTestRunTest::ForkProcess( timeout = std::chrono::seconds(1); } // handle timeout explicitly set to 0 - if (testTimeOut == cmDuration::zero() && explicitTimeout) { + if (testTimeOut == cmDuration::zero() && + this->TestProperties->ExplicitTimeout) { timeout = cmDuration::zero(); } cmCTestOptionalLog(this->CTest, HANDLER_VERBOSE_OUTPUT, @@ -784,17 +778,17 @@ bool cmCTestRunTest::ForkProcess( // We split processing ENVIRONMENT and ENVIRONMENT_MODIFICATION into two // phases to ensure that MYVAR=reset: in the latter phase resets to the // former phase's settings, rather than to the original environment. - if (environment && !environment->empty()) { + if (!this->TestProperties->Environment.empty()) { cmSystemTools::EnvDiff diff; - diff.AppendEnv(*environment); + diff.AppendEnv(this->TestProperties->Environment); diff.ApplyToCurrentEnv(&envMeasurement); } - if (environment_modification && !environment_modification->empty()) { + if (!this->TestProperties->EnvironmentModification.empty()) { cmSystemTools::EnvDiff diff; bool env_ok = true; - for (auto const& envmod : *environment_modification) { + for (auto const& envmod : this->TestProperties->EnvironmentModification) { env_ok &= diff.ParseOperation(envmod); } @@ -823,7 +817,7 @@ bool cmCTestRunTest::ForkProcess( 1); return this->TestProcess->StartProcess(this->MultiTestHandler.Loop, - affinity); + &this->TestProperties->Affinity); } void cmCTestRunTest::SetupResourcesEnvironment(std::vector* log) diff --git a/Source/CTest/cmCTestRunTest.h b/Source/CTest/cmCTestRunTest.h index 7a97fa9..b2f4377 100644 --- a/Source/CTest/cmCTestRunTest.h +++ b/Source/CTest/cmCTestRunTest.h @@ -110,10 +110,7 @@ private: bool NeedsToRepeat(); void ParseOutputForMeasurements(); void ExeNotFound(std::string exe); - bool ForkProcess(cmDuration testTimeOut, bool explicitTimeout, - std::vector* environment, - std::vector* environment_modification, - std::vector* affinity); + bool ForkProcess(cmDuration testTimeOut); void WriteLogOutputTop(size_t completed, size_t total); // Run post processing of the process output for MemCheck void MemCheckPostProcess(); -- cgit v0.12 From 426e38cc104673e8056caa243950c330a87e20af Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 4 May 2023 16:51:31 -0400 Subject: cmCTestRunTest: Adopt decision for starting cmProcess timer --- Source/CTest/cmCTestRunTest.cxx | 6 +++++- Source/CTest/cmProcess.cxx | 10 +++------- Source/CTest/cmProcess.h | 4 +++- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/Source/CTest/cmCTestRunTest.cxx b/Source/CTest/cmCTestRunTest.cxx index 307312d..46cb54e 100644 --- a/Source/CTest/cmCTestRunTest.cxx +++ b/Source/CTest/cmCTestRunTest.cxx @@ -770,7 +770,11 @@ bool cmCTestRunTest::ForkProcess(cmDuration testTimeOut) << "\n", this->TestHandler->GetQuiet()); - this->TestProcess->SetTimeout(timeout); + // An explicit TIMEOUT=0 test property means "no timeout". + if (timeout != cmDuration::zero() || + !this->TestProperties->ExplicitTimeout) { + this->TestProcess->SetTimeout(timeout); + } cmSystemTools::SaveRestoreEnvironment sre; std::ostringstream envMeasurement; diff --git a/Source/CTest/cmProcess.cxx b/Source/CTest/cmProcess.cxx index e14a4e1..780d626 100644 --- a/Source/CTest/cmProcess.cxx +++ b/Source/CTest/cmProcess.cxx @@ -13,7 +13,6 @@ #include "cmCTest.h" #include "cmCTestRunTest.h" -#include "cmCTestTestHandler.h" #include "cmGetPipes.h" #include "cmStringAlgorithms.h" #if defined(_WIN32) @@ -26,7 +25,6 @@ cmProcess::cmProcess(std::unique_ptr runner) : Runner(std::move(runner)) , Conv(cmProcessOutput::UTF8, CM_PROCESS_BUF_SIZE) { - this->Timeout = cmDuration::zero(); this->TotalTime = cmDuration::zero(); this->ExitValue = 0; this->Id = 0; @@ -152,11 +150,9 @@ bool cmProcess::StartProcess(uv_loop_t& loop, std::vector* affinity) void cmProcess::StartTimer() { - auto* properties = this->Runner->GetTestProperties(); - auto msec = - std::chrono::duration_cast(this->Timeout); - - if (msec != std::chrono::milliseconds(0) || !properties->ExplicitTimeout) { + if (this->Timeout) { + auto msec = + std::chrono::duration_cast(*this->Timeout); this->Timer.start(&cmProcess::OnTimeoutCB, static_cast(msec.count()), 0); } diff --git a/Source/CTest/cmProcess.h b/Source/CTest/cmProcess.h index be030e4..1578687 100644 --- a/Source/CTest/cmProcess.h +++ b/Source/CTest/cmProcess.h @@ -12,6 +12,8 @@ #include #include +#include + #include #include "cmDuration.h" @@ -76,7 +78,7 @@ public: } private: - cmDuration Timeout; + cm::optional Timeout; std::chrono::steady_clock::time_point StartTime; cmDuration TotalTime; bool ReadHandleClosed = false; -- cgit v0.12 From 0a5aeaf302369ab62f89ab35b0f0fb690f71c05a Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 4 May 2023 16:55:13 -0400 Subject: cmCTestRunTest: Consolidate test timeout selection logic Test timeout selection was previously spread out over several locations. Consolidate it in a single place to make it easier to follow. --- Source/CTest/cmCTestMultiProcessHandler.cxx | 5 +- Source/CTest/cmCTestRunTest.cxx | 111 ++++++++++++++++------------ Source/CTest/cmCTestRunTest.h | 3 +- Source/CTest/cmCTestTestHandler.cxx | 6 -- Source/CTest/cmCTestTestHandler.h | 5 +- 5 files changed, 71 insertions(+), 59 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index abd1aa6..44eccb2 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -19,6 +19,7 @@ #include #include +#include #include #include @@ -1095,9 +1096,9 @@ static Json::Value DumpCTestProperties( properties.append( DumpCTestProperty("SKIP_RETURN_CODE", testProperties.SkipReturnCode)); } - if (testProperties.ExplicitTimeout) { + if (testProperties.Timeout) { properties.append( - DumpCTestProperty("TIMEOUT", testProperties.Timeout.count())); + DumpCTestProperty("TIMEOUT", testProperties.Timeout->count())); } if (!testProperties.TimeoutRegularExpressions.empty()) { properties.append(DumpCTestProperty( diff --git a/Source/CTest/cmCTestRunTest.cxx b/Source/CTest/cmCTestRunTest.cxx index 46cb54e..cd2b230 100644 --- a/Source/CTest/cmCTestRunTest.cxx +++ b/Source/CTest/cmCTestRunTest.cxx @@ -14,12 +14,14 @@ #include #include +#include #include "cmsys/RegularExpression.hxx" #include "cmCTest.h" #include "cmCTestMemCheckHandler.h" #include "cmCTestMultiProcessHandler.h" +#include "cmDuration.h" #include "cmProcess.h" #include "cmStringAlgorithms.h" #include "cmSystemTools.h" @@ -618,25 +620,7 @@ bool cmCTestRunTest::StartTest(size_t completed, size_t total) } this->StartTime = this->CTest->CurrentTime(); - auto timeout = this->TestProperties->Timeout; - - this->TimeoutIsForStopTime = false; - std::chrono::system_clock::time_point stop_time = this->CTest->GetStopTime(); - if (stop_time != std::chrono::system_clock::time_point()) { - std::chrono::duration stop_timeout = - (stop_time - std::chrono::system_clock::now()) % std::chrono::hours(24); - - if (stop_timeout <= std::chrono::duration::zero()) { - stop_timeout = std::chrono::duration::zero(); - } - if (timeout == std::chrono::duration::zero() || - stop_timeout < timeout) { - this->TimeoutIsForStopTime = true; - timeout = stop_timeout; - } - } - - return this->ForkProcess(timeout); + return this->ForkProcess(); } void cmCTestRunTest::ComputeArguments() @@ -734,46 +718,79 @@ void cmCTestRunTest::ParseOutputForMeasurements() } } -bool cmCTestRunTest::ForkProcess(cmDuration testTimeOut) +bool cmCTestRunTest::ForkProcess() { this->TestProcess->SetId(this->Index); this->TestProcess->SetWorkingDirectory(this->TestProperties->Directory); this->TestProcess->SetCommand(this->ActualCommand); this->TestProcess->SetCommandArguments(this->Arguments); - // determine how much time we have - cmDuration timeout = this->CTest->GetRemainingTimeAllowed(); - if (timeout != cmCTest::MaxDuration()) { - timeout -= std::chrono::minutes(2); + cm::optional timeout; + + // Check TIMEOUT test property. + if (this->TestProperties->Timeout && + *this->TestProperties->Timeout >= cmDuration::zero()) { + timeout = this->TestProperties->Timeout; } - if (this->CTest->GetTimeOut() > cmDuration::zero() && - this->CTest->GetTimeOut() < timeout) { - timeout = this->CTest->GetTimeOut(); + + // An explicit TIMEOUT=0 test property means "no timeout". + if (timeout && *timeout == std::chrono::duration::zero()) { + timeout = cm::nullopt; + } else { + // Check --timeout. + if (!timeout && this->CTest->GetGlobalTimeout() > cmDuration::zero()) { + timeout = this->CTest->GetGlobalTimeout(); + } + + // Check CTEST_TEST_TIMEOUT. + cmDuration ctestTestTimeout = this->CTest->GetTimeOut(); + if (ctestTestTimeout > cmDuration::zero() && + (!timeout || ctestTestTimeout < *timeout)) { + timeout = ctestTestTimeout; + } + } + + // Check CTEST_TIME_LIMIT. + cmDuration timeRemaining = this->CTest->GetRemainingTimeAllowed(); + if (timeRemaining != cmCTest::MaxDuration()) { + // This two minute buffer is historical. + timeRemaining -= std::chrono::minutes(2); } - if (testTimeOut > cmDuration::zero() && - testTimeOut < this->CTest->GetRemainingTimeAllowed()) { - timeout = testTimeOut; + + // Check --stop-time. + std::chrono::system_clock::time_point stop_time = this->CTest->GetStopTime(); + if (stop_time != std::chrono::system_clock::time_point()) { + cmDuration timeUntilStop = + (stop_time - std::chrono::system_clock::now()) % std::chrono::hours(24); + if (timeUntilStop < timeRemaining) { + timeRemaining = timeUntilStop; + } } - // always have at least 1 second if we got to here - if (timeout <= cmDuration::zero()) { - timeout = std::chrono::seconds(1); + + // Enforce remaining time even over explicit TIMEOUT=0. + if (timeRemaining <= cmDuration::zero()) { + timeRemaining = cmDuration::zero(); } - // handle timeout explicitly set to 0 - if (testTimeOut == cmDuration::zero() && - this->TestProperties->ExplicitTimeout) { - timeout = cmDuration::zero(); + if (!timeout || timeRemaining < *timeout) { + this->TimeoutIsForStopTime = true; + timeout = timeRemaining; } - cmCTestOptionalLog(this->CTest, HANDLER_VERBOSE_OUTPUT, - this->Index << ": " - << "Test timeout computed to be: " - << cmDurationTo(timeout) - << "\n", - this->TestHandler->GetQuiet()); - // An explicit TIMEOUT=0 test property means "no timeout". - if (timeout != cmDuration::zero() || - !this->TestProperties->ExplicitTimeout) { - this->TestProcess->SetTimeout(timeout); + if (timeout) { + cmCTestOptionalLog(this->CTest, HANDLER_VERBOSE_OUTPUT, + this->Index << ": " + << "Test timeout computed to be: " + << cmDurationTo(*timeout) + << "\n", + this->TestHandler->GetQuiet()); + + this->TestProcess->SetTimeout(*timeout); + } else { + cmCTestOptionalLog(this->CTest, HANDLER_VERBOSE_OUTPUT, + this->Index + << ": " + << "Test timeout suppressed by TIMEOUT property.\n", + this->TestHandler->GetQuiet()); } cmSystemTools::SaveRestoreEnvironment sre; diff --git a/Source/CTest/cmCTestRunTest.h b/Source/CTest/cmCTestRunTest.h index b2f4377..6a507f4 100644 --- a/Source/CTest/cmCTestRunTest.h +++ b/Source/CTest/cmCTestRunTest.h @@ -14,7 +14,6 @@ #include "cmCTest.h" #include "cmCTestMultiProcessHandler.h" #include "cmCTestTestHandler.h" -#include "cmDuration.h" #include "cmProcess.h" /** \class cmRunTest @@ -110,7 +109,7 @@ private: bool NeedsToRepeat(); void ParseOutputForMeasurements(); void ExeNotFound(std::string exe); - bool ForkProcess(cmDuration testTimeOut); + bool ForkProcess(); void WriteLogOutputTop(size_t completed, size_t total); // Run post processing of the process output for MemCheck void MemCheckPostProcess(); diff --git a/Source/CTest/cmCTestTestHandler.cxx b/Source/CTest/cmCTestTestHandler.cxx index cd92e0a..7764f2b 100644 --- a/Source/CTest/cmCTestTestHandler.cxx +++ b/Source/CTest/cmCTestTestHandler.cxx @@ -1379,11 +1379,6 @@ bool cmCTestTestHandler::ProcessDirectory(std::vector& passed, p.Cost = static_cast(rand()); } - if (p.Timeout == cmDuration::zero() && !p.ExplicitTimeout && - this->CTest->GetGlobalTimeout() != cmDuration::zero()) { - p.Timeout = this->CTest->GetGlobalTimeout(); - } - if (!p.Depends.empty()) { for (std::string const& i : p.Depends) { for (cmCTestTestProperties const& it2 : this->TestList) { @@ -2252,7 +2247,6 @@ bool cmCTestTestHandler::SetTestsProperties( rt.FixturesRequired.insert(lval.begin(), lval.end()); } else if (key == "TIMEOUT"_s) { rt.Timeout = cmDuration(atof(val.c_str())); - rt.ExplicitTimeout = true; } else if (key == "COST"_s) { rt.Cost = static_cast(atof(val.c_str())); } else if (key == "REQUIRED_FILES"_s) { diff --git a/Source/CTest/cmCTestTestHandler.h b/Source/CTest/cmCTestTestHandler.h index 29d24e0..b7c0faf 100644 --- a/Source/CTest/cmCTestTestHandler.h +++ b/Source/CTest/cmCTestTestHandler.h @@ -14,6 +14,8 @@ #include #include +#include + #include "cmsys/RegularExpression.hxx" #include "cmCTest.h" @@ -145,8 +147,7 @@ public: float Cost = 0; int PreviousRuns = 0; bool RunSerial = false; - cmDuration Timeout = cmDuration::zero(); - bool ExplicitTimeout = false; + cm::optional Timeout; cmDuration AlternateTimeout; int Index = 0; // Requested number of process slots -- cgit v0.12