mirror/CMake
1
0
Fork 0
mirror of https://github.com/Kitware/CMake.git synced 2026-01-07 22:30:13 -06:00
Code Issues Packages Projects Releases Wiki Activity

cmCTestRunTest: modernize memory management

Browse Source
This commit is contained in:
Marc Chevrier
2020-03-10 16:04:51 +01:00
parent e3571773ce
commit f964739ead
6 changed files with 100 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files

44
Source/CTest/cmCTestMultiProcessHandler.cxx
View File

@@ -12,13 +12,13 @@
#include <iomanip> #include <iomanip>
#include <iostream> #include <iostream>
#include <list> #include <list>
#include <memory>
#include <sstream> #include <sstream>
#include <stack> #include <stack>
#include <unordered_map> #include <unordered_map>
#include <utility> #include <utility>
#include <vector> #include <vector>
#include <cm/memory>
#include <cmext/algorithm> #include <cmext/algorithm>
#include "cmsys/FStream.hxx" #include "cmsys/FStream.hxx"
@@ -172,7 +172,8 @@ bool cmCTestMultiProcessHandler::StartTestProcess(int test)
this->EraseTest(test); this->EraseTest(test);
this->RunningCount += GetProcessorsUsed(test); this->RunningCount += GetProcessorsUsed(test);
cmCTestRunTest* testRun = new cmCTestRunTest(*this); auto testRun = cm::make_unique<cmCTestRunTest>(*this);
if (this->RepeatMode != cmCTest::Repeat::Never) { if (this->RepeatMode != cmCTest::Repeat::Never) {
testRun->SetRepeatMode(this->RepeatMode); testRun->SetRepeatMode(this->RepeatMode);
testRun->SetNumberOfRuns(this->RepeatCount); testRun->SetNumberOfRuns(this->RepeatCount);
@@ -229,28 +230,25 @@ bool cmCTestMultiProcessHandler::StartTestProcess(int test)
e << "\n"; e << "\n";
} }
e << "Resource spec file:\n\n " << this->TestHandler->ResourceSpecFile; e << "Resource spec file:\n\n " << this->TestHandler->ResourceSpecFile;
testRun->StartFailure(e.str(), "Insufficient resources"); cmCTestRunTest::StartFailure(std::move(testRun), e.str(),
this->FinishTestProcess(testRun, false); "Insufficient resources");
return false; return false;
} }
cmWorkingDirectory workdir(this->Properties[test]->Directory); cmWorkingDirectory workdir(this->Properties[test]->Directory);
if (workdir.Failed()) { if (workdir.Failed()) {
testRun->StartFailure("Failed to change working directory to " + cmCTestRunTest::StartFailure(std::move(testRun),
this->Properties[test]->Directory + " : " + "Failed to change working directory to " +
std::strerror(workdir.GetLastResult()), this->Properties[test]->Directory + " : " +
"Failed to change working directory"); std::strerror(workdir.GetLastResult()),
} else { "Failed to change working directory");
if (testRun->StartTest(this->Completed, this->Total)) { return false;
// Ownership of 'testRun' has moved to another structure.
// When the test finishes, FinishTestProcess will be called.
return true;
}
} }
// Pass ownership of 'testRun'. // Ownership of 'testRun' has moved to another structure.
this->FinishTestProcess(testRun, false); // When the test finishes, FinishTestProcess will be called.
return false; return cmCTestRunTest::StartTest(std::move(testRun), this->Completed,
this->Total);
} }
bool cmCTestMultiProcessHandler::AllocateResources(int index) bool cmCTestMultiProcessHandler::AllocateResources(int index)
@@ -540,7 +538,8 @@ void cmCTestMultiProcessHandler::StartNextTests()
if (this->SerialTestRunning) { if (this->SerialTestRunning) {
break; break;
} }
// We can only start a RUN_SERIAL test if no other tests are also running. // We can only start a RUN_SERIAL test if no other tests are also
// running.
if (this->Properties[test]->RunSerial && this->RunningCount > 0) { if (this->Properties[test]->RunSerial && this->RunningCount > 0) {
continue; continue;
} }
@@ -618,8 +617,8 @@ void cmCTestMultiProcessHandler::OnTestLoadRetryCB(uv_timer_t* timer)
self->StartNextTests(); self->StartNextTests();
} }
void cmCTestMultiProcessHandler::FinishTestProcess(cmCTestRunTest* runner, void cmCTestMultiProcessHandler::FinishTestProcess(
bool started) std::unique_ptr<cmCTestRunTest> runner, bool started)
{ {
this->Completed++; this->Completed++;
@@ -631,7 +630,8 @@ void cmCTestMultiProcessHandler::FinishTestProcess(cmCTestRunTest* runner,
this->SetStopTimePassed(); this->SetStopTimePassed();
} }
if (started) { if (started) {
if (!this->StopTimePassed && runner->StartAgain(this->Completed)) { if (!this->StopTimePassed &&
cmCTestRunTest::StartAgain(std::move(runner), this->Completed)) {
this->Completed--; // remove the completed test because run again this->Completed--; // remove the completed test because run again
return; return;
} }
@@ -659,7 +659,7 @@ void cmCTestMultiProcessHandler::FinishTestProcess(cmCTestRunTest* runner,
} }
properties->Affinity.clear(); properties->Affinity.clear();
delete runner; runner.reset();
if (started) { if (started) {
this->StartNextTests(); this->StartNextTests();
} }

3
Source/CTest/cmCTestMultiProcessHandler.h
View File

@@ -6,6 +6,7 @@
#include "cmConfigure.h" // IWYU pragma: keep #include "cmConfigure.h" // IWYU pragma: keep
#include <map> #include <map>
#include <memory>
#include <set> #include <set>
#include <string> #include <string>
#include <vector> #include <vector>
@@ -124,7 +125,7 @@ protected:
// Removes the checkpoint file // Removes the checkpoint file
void MarkFinished(); void MarkFinished();
void EraseTest(int index); void EraseTest(int index);
void FinishTestProcess(cmCTestRunTest* runner, bool started); void FinishTestProcess(std::unique_ptr<cmCTestRunTest> runner, bool started);
static void OnTestLoadRetryCB(uv_timer_t* timer); static void OnTestLoadRetryCB(uv_timer_t* timer);

61
Source/CTest/cmCTestRunTest.cxx
View File

@@ -314,23 +314,27 @@ bool cmCTestRunTest::EndTest(size_t completed, size_t total, bool started)
return passed || skipped; return passed || skipped;
} }
bool cmCTestRunTest::StartAgain(size_t completed) bool cmCTestRunTest::StartAgain(std::unique_ptr<cmCTestRunTest> runner,
size_t completed)
{ {
if (!this->RunAgain) { auto* testRun = runner.get();
if (!testRun->RunAgain) {
return false; return false;
} }
this->RunAgain = false; // reset testRun->RunAgain = false; // reset
testRun->TestProcess = cm::make_unique<cmProcess>(std::move(runner));
// change to tests directory // change to tests directory
cmWorkingDirectory workdir(this->TestProperties->Directory); cmWorkingDirectory workdir(testRun->TestProperties->Directory);
if (workdir.Failed()) { if (workdir.Failed()) {
this->StartFailure("Failed to change working directory to " + testRun->StartFailure("Failed to change working directory to " +
this->TestProperties->Directory + " : " + testRun->TestProperties->Directory + " : " +
std::strerror(workdir.GetLastResult()), std::strerror(workdir.GetLastResult()),
"Failed to change working directory"); "Failed to change working directory");
return true; return true;
} }
this->StartTest(completed, this->TotalNumberOfTests); testRun->StartTest(completed, testRun->TotalNumberOfTests);
return true; return true;
} }
@@ -382,6 +386,18 @@ void cmCTestRunTest::MemCheckPostProcess()
handler->PostProcessTest(this->TestResult, this->Index); handler->PostProcessTest(this->TestResult, this->Index);
} }
void cmCTestRunTest::StartFailure(std::unique_ptr<cmCTestRunTest> runner,
std::string const& output,
std::string const& detail)
{
auto* testRun = runner.get();
testRun->TestProcess = cm::make_unique<cmProcess>(std::move(runner));
testRun->StartFailure(output, detail);
testRun->FinalizeTest(false);
}
void cmCTestRunTest::StartFailure(std::string const& output, void cmCTestRunTest::StartFailure(std::string const& output,
std::string const& detail) std::string const& detail)
{ {
@@ -413,7 +429,6 @@ void cmCTestRunTest::StartFailure(std::string const& output,
this->TestResult.Path = this->TestProperties->Directory; this->TestResult.Path = this->TestProperties->Directory;
this->TestResult.Output = output; this->TestResult.Output = output;
this->TestResult.FullCommandLine.clear(); this->TestResult.FullCommandLine.clear();
this->TestProcess = cm::make_unique<cmProcess>(*this);
} }
std::string cmCTestRunTest::GetTestPrefix(size_t completed, size_t total) const std::string cmCTestRunTest::GetTestPrefix(size_t completed, size_t total) const
@@ -437,6 +452,21 @@ std::string cmCTestRunTest::GetTestPrefix(size_t completed, size_t total) const
return outputStream.str(); return outputStream.str();
} }
bool cmCTestRunTest::StartTest(std::unique_ptr<cmCTestRunTest> runner,
size_t completed, size_t total)
{
auto* testRun = runner.get();
testRun->TestProcess = cm::make_unique<cmProcess>(std::move(runner));
if (!testRun->StartTest(completed, total)) {
testRun->FinalizeTest(false);
return false;
}
return true;
}
// Starts the execution of a test. Returns once it has started // Starts the execution of a test. Returns once it has started
bool cmCTestRunTest::StartTest(size_t completed, size_t total) bool cmCTestRunTest::StartTest(size_t completed, size_t total)
{ {
@@ -468,7 +498,6 @@ bool cmCTestRunTest::StartTest(size_t completed, size_t total)
if (this->TestProperties->Disabled) { if (this->TestProperties->Disabled) {
this->TestResult.CompletionStatus = "Disabled"; this->TestResult.CompletionStatus = "Disabled";
this->TestResult.Status = cmCTestTestHandler::NOT_RUN; this->TestResult.Status = cmCTestTestHandler::NOT_RUN;
this->TestProcess = cm::make_unique<cmProcess>(*this);
this->TestResult.Output = "Disabled"; this->TestResult.Output = "Disabled";
this->TestResult.FullCommandLine.clear(); this->TestResult.FullCommandLine.clear();
return false; return false;
@@ -482,7 +511,6 @@ bool cmCTestRunTest::StartTest(size_t completed, size_t total)
// its arguments are irrelevant. This matters for the case where a fixture // its arguments are irrelevant. This matters for the case where a fixture
// dependency might be creating the executable we want to run. // dependency might be creating the executable we want to run.
if (!this->FailedDependencies.empty()) { if (!this->FailedDependencies.empty()) {
this->TestProcess = cm::make_unique<cmProcess>(*this);
std::string msg = "Failed test dependencies:"; std::string msg = "Failed test dependencies:";
for (std::string const& failedDep : this->FailedDependencies) { for (std::string const& failedDep : this->FailedDependencies) {
msg += " " + failedDep; msg += " " + failedDep;
@@ -499,7 +527,6 @@ bool cmCTestRunTest::StartTest(size_t completed, size_t total)
this->ComputeArguments(); this->ComputeArguments();
std::vector<std::string>& args = this->TestProperties->Args; std::vector<std::string>& args = this->TestProperties->Args;
if (args.size() >= 2 && args[1] == "NOT_AVAILABLE") { if (args.size() >= 2 && args[1] == "NOT_AVAILABLE") {
this->TestProcess = cm::make_unique<cmProcess>(*this);
std::string msg; std::string msg;
if (this->CTest->GetConfigType().empty()) { if (this->CTest->GetConfigType().empty()) {
msg = "Test not available without configuration. (Missing \"-C " msg = "Test not available without configuration. (Missing \"-C "
@@ -521,7 +548,6 @@ bool cmCTestRunTest::StartTest(size_t completed, size_t total)
for (std::string const& file : this->TestProperties->RequiredFiles) { for (std::string const& file : this->TestProperties->RequiredFiles) {
if (!cmSystemTools::FileExists(file)) { if (!cmSystemTools::FileExists(file)) {
// Required file was not found // Required file was not found
this->TestProcess = cm::make_unique<cmProcess>(*this);
*this->TestHandler->LogFile << "Unable to find required file: " << file *this->TestHandler->LogFile << "Unable to find required file: " << file
<< std::endl; << std::endl;
cmCTestLog(this->CTest, ERROR_MESSAGE, cmCTestLog(this->CTest, ERROR_MESSAGE,
@@ -537,7 +563,6 @@ bool cmCTestRunTest::StartTest(size_t completed, size_t total)
if (this->ActualCommand.empty()) { if (this->ActualCommand.empty()) {
// if the command was not found create a TestResult object // if the command was not found create a TestResult object
// that has that information // that has that information
this->TestProcess = cm::make_unique<cmProcess>(*this);
*this->TestHandler->LogFile << "Unable to find executable: " << args[1] *this->TestHandler->LogFile << "Unable to find executable: " << args[1]
<< std::endl; << std::endl;
cmCTestLog(this->CTest, ERROR_MESSAGE, cmCTestLog(this->CTest, ERROR_MESSAGE,
@@ -649,7 +674,6 @@ bool cmCTestRunTest::ForkProcess(cmDuration testTimeOut, bool explicitTimeout,
std::vector<std::string>* environment, std::vector<std::string>* environment,
std::vector<size_t>* affinity) std::vector<size_t>* affinity)
{ {
this->TestProcess = cm::make_unique<cmProcess>(*this);
this->TestProcess->SetId(this->Index); this->TestProcess->SetId(this->Index);
this->TestProcess->SetWorkingDirectory(this->TestProperties->Directory); this->TestProcess->SetWorkingDirectory(this->TestProperties->Directory);
this->TestProcess->SetCommand(this->ActualCommand); this->TestProcess->SetCommand(this->ActualCommand);
@@ -816,7 +840,8 @@ void cmCTestRunTest::WriteLogOutputTop(size_t completed, size_t total)
"Testing " << this->TestProperties->Name << " ... "); "Testing " << this->TestProperties->Name << " ... ");
} }
void cmCTestRunTest::FinalizeTest() void cmCTestRunTest::FinalizeTest(bool started)
{ {
this->MultiTestHandler.FinishTestProcess(this, true); this->MultiTestHandler.FinishTestProcess(this->TestProcess->GetRunner(),
started);
} }

13
Source/CTest/cmCTestRunTest.h
View File

@@ -65,6 +65,15 @@ public:
// Read and store output. Returns true if it must be called again. // Read and store output. Returns true if it must be called again.
void CheckOutput(std::string const& line); void CheckOutput(std::string const& line);
static bool StartTest(std::unique_ptr<cmCTestRunTest> runner,
size_t completed, size_t total);
static bool StartAgain(std::unique_ptr<cmCTestRunTest> runner,
size_t completed);
static void StartFailure(std::unique_ptr<cmCTestRunTest> runner,
std::string const& output,
std::string const& detail);
// launch the test process, return whether it started correctly // launch the test process, return whether it started correctly
bool StartTest(size_t completed, size_t total); bool StartTest(size_t completed, size_t total);
// capture and report the test results // capture and report the test results
@@ -74,8 +83,6 @@ public:
void ComputeWeightedCost(); void ComputeWeightedCost();
bool StartAgain(size_t completed);
void StartFailure(std::string const& output, std::string const& detail); void StartFailure(std::string const& output, std::string const& detail);
cmCTest* GetCTest() const { return this->CTest; } cmCTest* GetCTest() const { return this->CTest; }
@@ -84,7 +91,7 @@ public:
const std::vector<std::string>& GetArguments() { return this->Arguments; } const std::vector<std::string>& GetArguments() { return this->Arguments; }
void FinalizeTest(); void FinalizeTest(bool started = true);
bool TimedOutForStopTime() const { return this->TimeoutIsForStopTime; } bool TimedOutForStopTime() const { return this->TimeoutIsForStopTime; }

28
Source/CTest/cmProcess.cxx
View File

@@ -5,6 +5,7 @@
#include <csignal> #include <csignal>
#include <iostream> #include <iostream>
#include <string> #include <string>
#include <utility>
#include <cmext/algorithm> #include <cmext/algorithm>
@@ -18,12 +19,11 @@
#if defined(_WIN32) #if defined(_WIN32)
# include "cm_kwiml.h" # include "cm_kwiml.h"
#endif #endif
#include <utility>
#define CM_PROCESS_BUF_SIZE 65536 #define CM_PROCESS_BUF_SIZE 65536
cmProcess::cmProcess(cmCTestRunTest& runner) cmProcess::cmProcess(std::unique_ptr<cmCTestRunTest> runner)
: Runner(runner) : Runner(std::move(runner))
, Conv(cmProcessOutput::UTF8, CM_PROCESS_BUF_SIZE) , Conv(cmProcessOutput::UTF8, CM_PROCESS_BUF_SIZE)
{ {
this->Timeout = cmDuration::zero(); this->Timeout = cmDuration::zero();
@@ -69,7 +69,7 @@ bool cmProcess::StartProcess(uv_loop_t& loop, std::vector<size_t>* affinity)
cm::uv_timer_ptr timer; cm::uv_timer_ptr timer;
int status = timer.init(loop, this); int status = timer.init(loop, this);
if (status != 0) { if (status != 0) {
cmCTestLog(this->Runner.GetCTest(), ERROR_MESSAGE, cmCTestLog(this->Runner->GetCTest(), ERROR_MESSAGE,
"Error initializing timer: " << uv_strerror(status) "Error initializing timer: " << uv_strerror(status)
<< std::endl); << std::endl);
return false; return false;
@@ -84,7 +84,7 @@ bool cmProcess::StartProcess(uv_loop_t& loop, std::vector<size_t>* affinity)
int fds[2] = { -1, -1 }; int fds[2] = { -1, -1 };
status = cmGetPipes(fds); status = cmGetPipes(fds);
if (status != 0) { if (status != 0) {
cmCTestLog(this->Runner.GetCTest(), ERROR_MESSAGE, cmCTestLog(this->Runner->GetCTest(), ERROR_MESSAGE,
"Error initializing pipe: " << uv_strerror(status) "Error initializing pipe: " << uv_strerror(status)
<< std::endl); << std::endl);
return false; return false;
@@ -127,7 +127,7 @@ bool cmProcess::StartProcess(uv_loop_t& loop, std::vector<size_t>* affinity)
uv_read_start(pipe_reader, &cmProcess::OnAllocateCB, &cmProcess::OnReadCB); uv_read_start(pipe_reader, &cmProcess::OnAllocateCB, &cmProcess::OnReadCB);
if (status != 0) { if (status != 0) {
cmCTestLog(this->Runner.GetCTest(), ERROR_MESSAGE, cmCTestLog(this->Runner->GetCTest(), ERROR_MESSAGE,
"Error starting read events: " << uv_strerror(status) "Error starting read events: " << uv_strerror(status)
<< std::endl); << std::endl);
return false; return false;
@@ -135,7 +135,7 @@ bool cmProcess::StartProcess(uv_loop_t& loop, std::vector<size_t>* affinity)
status = this->Process.spawn(loop, options, this); status = this->Process.spawn(loop, options, this);
if (status != 0) { if (status != 0) {
cmCTestLog(this->Runner.GetCTest(), ERROR_MESSAGE, cmCTestLog(this->Runner->GetCTest(), ERROR_MESSAGE,
"Process not started\n " << this->Command << "\n[" "Process not started\n " << this->Command << "\n["
<< uv_strerror(status) << "]\n"); << uv_strerror(status) << "]\n");
return false; return false;
@@ -152,7 +152,7 @@ bool cmProcess::StartProcess(uv_loop_t& loop, std::vector<size_t>* affinity)
void cmProcess::StartTimer() void cmProcess::StartTimer()
{ {
auto properties = this->Runner.GetTestProperties(); auto properties = this->Runner->GetTestProperties();
auto msec = auto msec =
std::chrono::duration_cast<std::chrono::milliseconds>(this->Timeout); std::chrono::duration_cast<std::chrono::milliseconds>(this->Timeout);
@@ -222,7 +222,7 @@ void cmProcess::OnRead(ssize_t nread, const uv_buf_t* buf)
cm::append(this->Output, strdata); cm::append(this->Output, strdata);
while (this->Output.GetLine(line)) { while (this->Output.GetLine(line)) {
this->Runner.CheckOutput(line); this->Runner->CheckOutput(line);
line.clear(); line.clear();
} }
@@ -236,20 +236,20 @@ void cmProcess::OnRead(ssize_t nread, const uv_buf_t* buf)
// The process will provide no more data. // The process will provide no more data.
if (nread != UV_EOF) { if (nread != UV_EOF) {
auto error = static_cast<int>(nread); auto error = static_cast<int>(nread);
cmCTestLog(this->Runner.GetCTest(), ERROR_MESSAGE, cmCTestLog(this->Runner->GetCTest(), ERROR_MESSAGE,
"Error reading stream: " << uv_strerror(error) << std::endl); "Error reading stream: " << uv_strerror(error) << std::endl);
} }
// Look for partial last lines. // Look for partial last lines.
if (this->Output.GetLast(line)) { if (this->Output.GetLast(line)) {
this->Runner.CheckOutput(line); this->Runner->CheckOutput(line);
} }
this->ReadHandleClosed = true; this->ReadHandleClosed = true;
this->PipeReader.reset(); this->PipeReader.reset();
if (this->ProcessHandleClosed) { if (this->ProcessHandleClosed) {
uv_timer_stop(this->Timer); uv_timer_stop(this->Timer);
this->Runner.FinalizeTest(); this->Runner->FinalizeTest();
} }
} }
@@ -291,7 +291,7 @@ void cmProcess::OnTimeout()
// Our on-exit handler already ran but did not finish the test // Our on-exit handler already ran but did not finish the test
// because we were still reading output. We've just dropped // because we were still reading output. We've just dropped
// our read handler, so we need to finish the test now. // our read handler, so we need to finish the test now.
this->Runner.FinalizeTest(); this->Runner->FinalizeTest();
} }
} }
@@ -333,7 +333,7 @@ void cmProcess::OnExit(int64_t exit_status, int term_signal)
this->ProcessHandleClosed = true; this->ProcessHandleClosed = true;
if (this->ReadHandleClosed) { if (this->ReadHandleClosed) {
uv_timer_stop(this->Timer); uv_timer_stop(this->Timer);
this->Runner.FinalizeTest(); this->Runner->FinalizeTest();
} }
} }

11
Source/CTest/cmProcess.h
View File

@@ -6,7 +6,9 @@
#include "cmConfigure.h" // IWYU pragma: keep #include "cmConfigure.h" // IWYU pragma: keep
#include <chrono> #include <chrono>
#include <memory>
#include <string> #include <string>
#include <utility>
#include <vector> #include <vector>
#include <stddef.h> #include <stddef.h>
@@ -28,7 +30,7 @@ class cmCTestRunTest;
class cmProcess class cmProcess
{ {
public: public:
explicit cmProcess(cmCTestRunTest& runner); explicit cmProcess(std::unique_ptr<cmCTestRunTest> runner);
~cmProcess(); ~cmProcess();
void SetCommand(std::string const& command); void SetCommand(std::string const& command);
void SetCommandArguments(std::vector<std::string> const& arg); void SetCommandArguments(std::vector<std::string> const& arg);
@@ -70,6 +72,11 @@ public:
Exception GetExitException(); Exception GetExitException();
std::string GetExitExceptionString(); std::string GetExitExceptionString();
std::unique_ptr<cmCTestRunTest> GetRunner()
{
return std::move(this->Runner);
}
private: private:
cmDuration Timeout; cmDuration Timeout;
std::chrono::steady_clock::time_point StartTime; std::chrono::steady_clock::time_point StartTime;
@@ -82,7 +89,7 @@ private:
cm::uv_timer_ptr Timer; cm::uv_timer_ptr Timer;
std::vector<char> Buf; std::vector<char> Buf;
cmCTestRunTest& Runner; std::unique_ptr<cmCTestRunTest> Runner;
cmProcessOutput Conv; cmProcessOutput Conv;
int Signal = 0; int Signal = 0;
cmProcess::State ProcessState = cmProcess::State::Starting; cmProcess::State ProcessState = cmProcess::State::Starting;