From 1dec35942215817ff2e7535239a89bc239abed16 Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Fri, 21 Feb 2020 16:53:48 -0500 Subject: Refactor: Require detail when calling cmCTestRunTest::StartFailure() --- Source/CTest/cmCTestMultiProcessHandler.cxx | 7 ++++--- Source/CTest/cmCTestRunTest.cxx | 10 ++++++---- Source/CTest/cmCTestRunTest.h | 2 +- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index 37679b9..bee7165 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -197,7 +197,7 @@ bool cmCTestMultiProcessHandler::StartTestProcess(int test) this->LockResources(test); if (!this->TestsHaveSufficientResources[test]) { - testRun->StartFailure("Insufficient resources"); + testRun->StartFailure("Insufficient resources", "Failed to start"); this->FinishTestProcess(testRun, false); return false; } @@ -205,8 +205,9 @@ bool cmCTestMultiProcessHandler::StartTestProcess(int test) cmWorkingDirectory workdir(this->Properties[test]->Directory); if (workdir.Failed()) { testRun->StartFailure("Failed to change working directory to " + - this->Properties[test]->Directory + " : " + - std::strerror(workdir.GetLastResult())); + this->Properties[test]->Directory + " : " + + std::strerror(workdir.GetLastResult()), + "Failed to start"); } else { if (testRun->StartTest(this->Completed, this->Total)) { // Ownership of 'testRun' has moved to another structure. diff --git a/Source/CTest/cmCTestRunTest.cxx b/Source/CTest/cmCTestRunTest.cxx index cc5de43..f0a3af1 100644 --- a/Source/CTest/cmCTestRunTest.cxx +++ b/Source/CTest/cmCTestRunTest.cxx @@ -324,8 +324,9 @@ bool cmCTestRunTest::StartAgain(size_t completed) cmWorkingDirectory workdir(this->TestProperties->Directory); if (workdir.Failed()) { this->StartFailure("Failed to change working directory to " + - this->TestProperties->Directory + " : " + - std::strerror(workdir.GetLastResult())); + this->TestProperties->Directory + " : " + + std::strerror(workdir.GetLastResult()), + "Failed to start"); return true; } @@ -381,7 +382,8 @@ void cmCTestRunTest::MemCheckPostProcess() handler->PostProcessTest(this->TestResult, this->Index); } -void cmCTestRunTest::StartFailure(std::string const& output) +void cmCTestRunTest::StartFailure(std::string const& output, + std::string const& detail) { // Still need to log the Start message so the test summary records our // attempt to start this test @@ -404,7 +406,7 @@ void cmCTestRunTest::StartFailure(std::string const& output) this->TestResult.ExecutionTime = cmDuration::zero(); this->TestResult.CompressOutput = false; this->TestResult.ReturnValue = -1; - this->TestResult.CompletionStatus = "Failed to start"; + this->TestResult.CompletionStatus = detail; this->TestResult.Status = cmCTestTestHandler::NOT_RUN; this->TestResult.TestCount = this->TestProperties->Index; this->TestResult.Name = this->TestProperties->Name; diff --git a/Source/CTest/cmCTestRunTest.h b/Source/CTest/cmCTestRunTest.h index 7eeaebd..4988839 100644 --- a/Source/CTest/cmCTestRunTest.h +++ b/Source/CTest/cmCTestRunTest.h @@ -76,7 +76,7 @@ public: bool StartAgain(size_t completed); - void StartFailure(std::string const& output); + void StartFailure(std::string const& output, std::string const& detail); cmCTest* GetCTest() const { return this->CTest; } -- cgit v0.12 From f1c34443b7fa828150e4e13b0dea3c5fbd29b6eb Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Fri, 21 Feb 2020 16:58:08 -0500 Subject: CTest: Improve error reporting with bad working directory for tests --- Source/CTest/cmCTestMultiProcessHandler.cxx | 2 +- Source/CTest/cmCTestRunTest.cxx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index bee7165..758ea57 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -207,7 +207,7 @@ bool cmCTestMultiProcessHandler::StartTestProcess(int test) testRun->StartFailure("Failed to change working directory to " + this->Properties[test]->Directory + " : " + std::strerror(workdir.GetLastResult()), - "Failed to start"); + "Failed to change working directory"); } else { if (testRun->StartTest(this->Completed, this->Total)) { // Ownership of 'testRun' has moved to another structure. diff --git a/Source/CTest/cmCTestRunTest.cxx b/Source/CTest/cmCTestRunTest.cxx index f0a3af1..ec54960 100644 --- a/Source/CTest/cmCTestRunTest.cxx +++ b/Source/CTest/cmCTestRunTest.cxx @@ -326,7 +326,7 @@ bool cmCTestRunTest::StartAgain(size_t completed) this->StartFailure("Failed to change working directory to " + this->TestProperties->Directory + " : " + std::strerror(workdir.GetLastResult()), - "Failed to start"); + "Failed to change working directory"); return true; } -- cgit v0.12 From f0df3ed5b99257ab8109fcbe09e8ff97565c00c8 Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Mon, 24 Feb 2020 10:53:32 -0500 Subject: Refactor: Provide more detailed error information from TryAllocateResources() --- Source/CTest/cmCTestMultiProcessHandler.cxx | 41 +++++++++++++++++++---------- Source/CTest/cmCTestMultiProcessHandler.h | 12 +++++++-- 2 files changed, 37 insertions(+), 16 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index 758ea57..6343353 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -196,7 +196,7 @@ bool cmCTestMultiProcessHandler::StartTestProcess(int test) // working directory because FinishTestProcess() will try to unlock them this->LockResources(test); - if (!this->TestsHaveSufficientResources[test]) { + if (!this->ResourceAllocationErrors[test].empty()) { testRun->StartFailure("Insufficient resources", "Failed to start"); this->FinishTestProcess(testRun, false); return false; @@ -250,7 +250,8 @@ bool cmCTestMultiProcessHandler::AllocateResources(int index) bool cmCTestMultiProcessHandler::TryAllocateResources( int index, - std::map>& allocations) + std::map>& allocations, + std::map* errors) { allocations.clear(); @@ -265,18 +266,28 @@ bool cmCTestMultiProcessHandler::TryAllocateResources( ++processIndex; } + bool result = true; auto const& availableResources = this->ResourceAllocator.GetResources(); for (auto& it : allocations) { if (!availableResources.count(it.first)) { - return false; - } - if (!cmAllocateCTestResourcesRoundRobin(availableResources.at(it.first), - it.second)) { - return false; + if (errors) { + (*errors)[it.first] = ResourceAllocationError::NoResourceType; + result = false; + } else { + return false; + } + } else if (!cmAllocateCTestResourcesRoundRobin( + availableResources.at(it.first), it.second)) { + if (errors) { + (*errors)[it.first] = ResourceAllocationError::InsufficientResources; + result = false; + } else { + return false; + } } } - return true; + return result; } void cmCTestMultiProcessHandler::DeallocateResources(int index) @@ -317,11 +328,13 @@ bool cmCTestMultiProcessHandler::AllResourcesAvailable() void cmCTestMultiProcessHandler::CheckResourcesAvailable() { - for (auto test : this->SortedTests) { - std::map> allocations; - this->TestsHaveSufficientResources[test] = - !this->TestHandler->UseResourceSpec || - this->TryAllocateResources(test, allocations); + if (this->TestHandler->UseResourceSpec) { + for (auto test : this->SortedTests) { + std::map> + allocations; + this->TryAllocateResources(test, allocations, + &this->ResourceAllocationErrors[test]); + } } } @@ -408,7 +421,7 @@ bool cmCTestMultiProcessHandler::StartTest(int test) } // Allocate resources - if (this->TestsHaveSufficientResources[test] && + if (this->ResourceAllocationErrors[test].empty() && !this->AllocateResources(test)) { this->DeallocateResources(test); return false; diff --git a/Source/CTest/cmCTestMultiProcessHandler.h b/Source/CTest/cmCTestMultiProcessHandler.h index 4837401..5b429d4 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.h +++ b/Source/CTest/cmCTestMultiProcessHandler.h @@ -143,11 +143,18 @@ protected: void LockResources(int index); void UnlockResources(int index); + enum class ResourceAllocationError + { + NoResourceType, + InsufficientResources, + }; + bool AllocateResources(int index); bool TryAllocateResources( int index, std::map>& - allocations); + allocations, + std::map* errors = nullptr); void DeallocateResources(int index); bool AllResourcesAvailable(); @@ -174,7 +181,8 @@ protected: std::map>>> AllocatedResources; - std::map TestsHaveSufficientResources; + std::map> + ResourceAllocationErrors; cmCTestResourceAllocator ResourceAllocator; std::vector* TestResults; size_t ParallelLevel; // max number of process that can be run at once -- cgit v0.12 From a5be3916eeee82d5237f9ace85a74fc052240717 Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Mon, 24 Feb 2020 11:41:06 -0500 Subject: CTest: Provide more detailed information on resource allocation error --- Source/CTest/cmCTestMultiProcessHandler.cxx | 34 +++++++++++++++++++++- Source/CTest/cmCTestTestHandler.cxx | 1 + Source/CTest/cmCTestTestHandler.h | 1 + .../notenough1-ctest-s-res-stderr.txt | 12 +++++++- .../notenough2-ctest-s-res-stderr.txt | 9 +++++- .../notenough3-ctest-s-res-stderr.txt | 19 +++++++++++- 6 files changed, 72 insertions(+), 4 deletions(-) diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index 6343353..2192843 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -197,7 +197,39 @@ bool cmCTestMultiProcessHandler::StartTestProcess(int test) this->LockResources(test); if (!this->ResourceAllocationErrors[test].empty()) { - testRun->StartFailure("Insufficient resources", "Failed to start"); + std::ostringstream e; + e << "Insufficient resources for test " << this->Properties[test]->Name + << ":\n\n"; + for (auto const& it : this->ResourceAllocationErrors[test]) { + switch (it.second) { + case ResourceAllocationError::NoResourceType: + e << " Test requested resources of type '" << it.first + << "' which does not exist\n"; + break; + + case ResourceAllocationError::InsufficientResources: + e << " Test requested resources of type '" << it.first + << "' in the following amounts:\n"; + for (auto const& group : this->Properties[test]->ResourceGroups) { + for (auto const& requirement : group) { + if (requirement.ResourceType == it.first) { + e << " " << requirement.SlotsNeeded + << (requirement.SlotsNeeded == 1 ? " slot\n" : " slots\n"); + } + } + } + e << " but only the following units were available:\n"; + for (auto const& res : + this->ResourceAllocator.GetResources().at(it.first)) { + e << " '" << res.first << "': " << res.second.Total + << (res.second.Total == 1 ? " slot\n" : " slots\n"); + } + break; + } + e << "\n"; + } + e << "Resource spec file:\n\n " << this->TestHandler->ResourceSpecFile; + testRun->StartFailure(e.str(), "Insufficient resources"); this->FinishTestProcess(testRun, false); return false; } diff --git a/Source/CTest/cmCTestTestHandler.cxx b/Source/CTest/cmCTestTestHandler.cxx index 78c68be..4f324ea 100644 --- a/Source/CTest/cmCTestTestHandler.cxx +++ b/Source/CTest/cmCTestTestHandler.cxx @@ -548,6 +548,7 @@ bool cmCTestTestHandler::ProcessOptions() val = this->GetOption("ResourceSpecFile"); if (val) { this->UseResourceSpec = true; + this->ResourceSpecFile = val; auto result = this->ResourceSpec.ReadFromJSONFile(val); if (result != cmCTestResourceSpec::ReadFileResult::READ_OK) { cmCTestLog(this->CTest, ERROR_MESSAGE, diff --git a/Source/CTest/cmCTestTestHandler.h b/Source/CTest/cmCTestTestHandler.h index 55237f9..b1c8755 100644 --- a/Source/CTest/cmCTestTestHandler.h +++ b/Source/CTest/cmCTestTestHandler.h @@ -338,6 +338,7 @@ private: bool UseResourceSpec; cmCTestResourceSpec ResourceSpec; + std::string ResourceSpecFile; void GenerateRegressionImages(cmXMLWriter& xml, const std::string& dart); cmsys::RegularExpression DartStuff1; diff --git a/Tests/RunCMake/CTestResourceAllocation/notenough1-ctest-s-res-stderr.txt b/Tests/RunCMake/CTestResourceAllocation/notenough1-ctest-s-res-stderr.txt index 41df5af..91d7ef9 100644 --- a/Tests/RunCMake/CTestResourceAllocation/notenough1-ctest-s-res-stderr.txt +++ b/Tests/RunCMake/CTestResourceAllocation/notenough1-ctest-s-res-stderr.txt @@ -1,4 +1,14 @@ -^Insufficient resources +^Insufficient resources for test Test1: + + Test requested resources of type 'fluxcapacitors' in the following amounts: + 200 slots + but only the following units were available: + 'outatime': 121 slots + +Resource spec file: + + [^ +]*/Tests/RunCMake/CTestResourceAllocation/resspec.json CMake Error at [^ ]*/Tests/RunCMake/CTestResourceAllocation/notenough1-ctest-s-res/test\.cmake:[0-9]+ \(message\): Tests did not pass$ diff --git a/Tests/RunCMake/CTestResourceAllocation/notenough2-ctest-s-res-stderr.txt b/Tests/RunCMake/CTestResourceAllocation/notenough2-ctest-s-res-stderr.txt index 6c2f554..5c75a3d 100644 --- a/Tests/RunCMake/CTestResourceAllocation/notenough2-ctest-s-res-stderr.txt +++ b/Tests/RunCMake/CTestResourceAllocation/notenough2-ctest-s-res-stderr.txt @@ -1,4 +1,11 @@ -^Insufficient resources +^Insufficient resources for test Test1: + + Test requested resources of type 'terminators' which does not exist + +Resource spec file: + + [^ +]*/Tests/RunCMake/CTestResourceAllocation/resspec.json CMake Error at [^ ]*/Tests/RunCMake/CTestResourceAllocation/notenough2-ctest-s-res/test\.cmake:[0-9]+ \(message\): Tests did not pass$ diff --git a/Tests/RunCMake/CTestResourceAllocation/notenough3-ctest-s-res-stderr.txt b/Tests/RunCMake/CTestResourceAllocation/notenough3-ctest-s-res-stderr.txt index 82dfdef..4902a19 100644 --- a/Tests/RunCMake/CTestResourceAllocation/notenough3-ctest-s-res-stderr.txt +++ b/Tests/RunCMake/CTestResourceAllocation/notenough3-ctest-s-res-stderr.txt @@ -1,4 +1,21 @@ -^Insufficient resources +^Insufficient resources for test Test1: + + Test requested resources of type 'widgets' in the following amounts: + 12 slots + but only the following units were available: + '0': 4 slots + '1': 2 slots + '2': 4 slots + '3': 8 slots + '4': 1 slot + '5': 1 slot + '6': 1 slot + '7': 1 slot + +Resource spec file: + + [^ +]*/Tests/RunCMake/CTestResourceAllocation/resspec.json CMake Error at [^ ]*/Tests/RunCMake/CTestResourceAllocation/notenough3-ctest-s-res/test\.cmake:[0-9]+ \(message\): Tests did not pass$ -- cgit v0.12