From fd395b25b5858b3a90edab59a606c74a3f02e5b4 Mon Sep 17 00:00:00 2001 From: Tyler Yankee Date: Thu, 2 Jul 2026 17:29:30 -0400 Subject: [PATCH] ctest: Improve colorization in output Refactor CTest's logging to replace custom color code handling with the infrastructure introduced in commit 329d755dbd (StdIo: Add a Terminal abstraction to print color text, 2025-05-06). Avoid ANSI escape sequences in files passed to `--output-log-file`. Fixes: #27915 --- Source/CTest/cmCTestTestHandler.cxx | 53 ++++++++---------- Source/cmCTest.cxx | 55 ++++++++----------- Source/cmCTest.h | 27 ++++----- .../CTest/ColorOutput-test-check.cmake | 38 +++++++++++++ .../CTest/ColorOutput-test-result.txt | 1 + .../CTest/ColorOutput-test-stderr.txt | 1 + Tests/RunCMake/CTest/ColorOutput.cmake | 7 +++ Tests/RunCMake/CTest/RunCMakeTest.cmake | 11 ++++ 8 files changed, 114 insertions(+), 79 deletions(-) create mode 100644 Tests/RunCMake/CTest/ColorOutput-test-check.cmake create mode 100644 Tests/RunCMake/CTest/ColorOutput-test-result.txt create mode 100644 Tests/RunCMake/CTest/ColorOutput-test-stderr.txt create mode 100644 Tests/RunCMake/CTest/ColorOutput.cmake diff --git a/Source/CTest/cmCTestTestHandler.cxx b/Source/CTest/cmCTestTestHandler.cxx index 8baaebc7cc..a5cd760d1c 100644 --- a/Source/CTest/cmCTestTestHandler.cxx +++ b/Source/CTest/cmCTestTestHandler.cxx @@ -52,6 +52,7 @@ #include "cmMakefile.h" #include "cmState.h" #include "cmStateSnapshot.h" +#include "cmStdIoTerminal.h" #include "cmStringAlgorithms.h" #include "cmSystemTools.h" #include "cmTestDiscovery.h" @@ -655,27 +656,21 @@ void cmCTestTestHandler::LogTestSummary(std::vector const& passed, percent = 99; } - std::string passColorCode; - std::string failedColorCode; + cm::StdIo::TermAttrSet summaryAttrs; if (failed.empty()) { - passColorCode = this->CTest->GetColorCode(cmCTest::Color::GREEN); + summaryAttrs = cm::StdIo::TermAttr::ForegroundGreen; } else { - failedColorCode = this->CTest->GetColorCode(cmCTest::Color::RED); + summaryAttrs = cm::StdIo::TermAttr::ForegroundRed; } if (failed.empty()) { - cmCTestLog(this->CTest, HANDLER_OUTPUT, - std::endl - << passColorCode << std::lround(percent) << "% tests passed" - << this->CTest->GetColorCode(cmCTest::Color::CLEAR_COLOR) - << " out of " << total << std::endl); + cmCTestColorLog(this->CTest, HANDLER_OUTPUT, summaryAttrs, + cmStrCat("\n", std::lround(percent), + "% tests passed out of ", total, "\n")); } else { - cmCTestLog(this->CTest, HANDLER_OUTPUT, - std::endl - << passColorCode << std::lround(percent) << "% tests passed" - << this->CTest->GetColorCode(cmCTest::Color::CLEAR_COLOR) - << ", " << failedColorCode << failed.size() << " tests failed" - << this->CTest->GetColorCode(cmCTest::Color::CLEAR_COLOR) - << " out of " << total << std::endl); + cmCTestColorLog(this->CTest, HANDLER_OUTPUT, summaryAttrs, + cmStrCat("\n", std::lround(percent), "% tests passed, ", + failed.size(), " tests failed out of ", total, + "\n")); } if ((!this->CTest->GetLabelsForSubprojects().empty() && this->CTest->GetSubprojectSummary())) { @@ -702,8 +697,7 @@ void cmCTestTestHandler::LogDisabledTests( this->StartLogFile("TestsDisabled", ofs); char const* disabled_reason; - cmCTestLog(this->CTest, HANDLER_OUTPUT, - this->CTest->GetColorCode(cmCTest::Color::BLUE)); + cm::StdIo::TermAttrSet disabledAttrs = cm::StdIo::TermAttr::ForegroundBlue; for (cmCTestTestResult const& dt : disabledTests) { ofs << dt.TestCount << ":" << dt.Name << std::endl; if (dt.CompletionStatus == "Disabled") { @@ -711,12 +705,11 @@ void cmCTestTestHandler::LogDisabledTests( } else { disabled_reason = "Skipped"; } - cmCTestLog(this->CTest, HANDLER_OUTPUT, - "\t" << std::setw(3) << dt.TestCount << " - " << dt.Name - << " (" << disabled_reason << ")" << std::endl); + std::ostringstream msg; + msg << "\t" << std::setw(3) << dt.TestCount << " - " << dt.Name << " (" + << disabled_reason << ")\n"; + cmCTestColorLog(this->CTest, HANDLER_OUTPUT, disabledAttrs, msg.str()); } - cmCTestLog(this->CTest, HANDLER_OUTPUT, - this->CTest->GetColorCode(cmCTest::Color::CLEAR_COLOR)); } } @@ -735,9 +728,9 @@ void cmCTestTestHandler::LogFailedTests(std::vector const& failed, !cmHasLiteralPrefix(ft.CompletionStatus, "SKIP_") && ft.CompletionStatus != "Disabled") { ofs << ft.TestCount << ":" << ft.Name << std::endl; - auto testColor = cmCTest::Color::RED; + cm::StdIo::TermAttrSet testAttrs = cm::StdIo::TermAttr::ForegroundRed; if (this->GetTestStatus(ft) == "Not Run") { - testColor = cmCTest::Color::YELLOW; + testAttrs = cm::StdIo::TermAttr::ForegroundYellow; } std::string ft_name_and_status = cmStrCat(ft.Name, " (", this->GetTestStatus(ft), ')'); @@ -750,12 +743,10 @@ void cmCTestTestHandler::LogFailedTests(std::vector const& failed, : maxLen - ft_name_and_status.size(); labels = cmStrCat(std::string(ns, ' '), cmJoin(p.Labels, " ")); } - cmCTestLog( - this->CTest, HANDLER_OUTPUT, - "\t" << this->CTest->GetColorCode(testColor) << std::setw(3) - << ft.TestCount << " - " << ft_name_and_status - << this->CTest->GetColorCode(cmCTest::Color::CLEAR_COLOR) - << labels << std::endl); + std::ostringstream msg; + msg << "\t" << std::setw(3) << ft.TestCount << " - " + << ft_name_and_status << labels << "\n"; + cmCTestColorLog(this->CTest, HANDLER_OUTPUT, testAttrs, msg.str()); } } } diff --git a/Source/cmCTest.cxx b/Source/cmCTest.cxx index 3b707c508b..a0f24dbd93 100644 --- a/Source/cmCTest.cxx +++ b/Source/cmCTest.cxx @@ -11,7 +11,6 @@ #include #include #include -#include #include #include #include @@ -63,6 +62,7 @@ #include "cmStateSnapshot.h" #include "cmStateTypes.h" #include "cmStdIoStream.h" +#include "cmStdIoTerminal.h" #include "cmStringAlgorithms.h" #include "cmSystemTools.h" #include "cmUVHandlePtr.h" @@ -197,7 +197,6 @@ struct cmCTest::Private cm::optional OutputLogFileLastTag; bool OutputTestOutputOnTestFailure = false; - bool OutputColorCode = cmCTest::ColoredOutputSupportedByConsole(); std::map Definitions; @@ -1565,20 +1564,6 @@ bool cmCTest::ProgressOutputSupportedByConsole() return cm::StdIo::Out().Kind() == cm::StdIo::TermKind::VT100; } -bool cmCTest::ColoredOutputSupportedByConsole() -{ - std::string clicolor_force; - if (cmSystemTools::GetEnv("CLICOLOR_FORCE", clicolor_force) && - !clicolor_force.empty() && clicolor_force != "0") { - return true; - } - std::string clicolor; - if (cmSystemTools::GetEnv("CLICOLOR", clicolor) && clicolor == "0") { - return false; - } - return cm::StdIo::Out().Kind() == cm::StdIo::TermKind::VT100; -} - bool cmCTest::AddVariableDefinition(std::string const& arg) { std::string name; @@ -3559,6 +3544,13 @@ static char const* cmCTestStringLogType[] = { "DEBUG", "ERROR_MESSAGE" }; void cmCTest::Log(LogType logType, std::string msg, bool suppress) +{ + static cm::StdIo::TermAttrSet const noAttrs; + this->Log(logType, std::move(msg), noAttrs, suppress); +} + +void cmCTest::Log(LogType logType, std::string msg, + cm::StdIo::TermAttrSet const& attrs, bool suppress) { if (msg.empty()) { return; @@ -3598,7 +3590,7 @@ void cmCTest::Log(LogType logType, std::string msg, bool suppress) if (this->Impl->TestProgressOutput) { if (this->Impl->TestProgressNewlinePending) { this->Impl->TestProgressNewlinePending = false; - std::cout << '\r'; + cm::StdIo::Out().IOS() << '\r'; } if (msg.find('\n') != std::string::npos) { @@ -3608,7 +3600,7 @@ void cmCTest::Log(LogType logType, std::string msg, bool suppress) // ProgressOutputSupportedByConsole() already verified VT100 support. // Erase the rest of the line before printing the message. - std::cout << kVT100_EraseLine << msg << std::flush; + cm::StdIo::Out().IOS() << kVT100_EraseLine << msg << std::flush; return; } logType = HANDLER_OUTPUT; @@ -3617,42 +3609,39 @@ void cmCTest::Log(LogType logType, std::string msg, bool suppress) switch (logType) { case DEBUG: if (this->Impl->Debug) { - std::cout << msg << std::flush; + cm::StdIo::Print(cm::StdIo::Out(), attrs, msg); + cm::StdIo::Out().IOS() << std::flush; } break; case OUTPUT: case HANDLER_OUTPUT: if (this->Impl->Debug || this->Impl->Verbose) { - std::cout << msg << std::flush; + cm::StdIo::Print(cm::StdIo::Out(), attrs, msg); + cm::StdIo::Out().IOS() << std::flush; } break; case HANDLER_VERBOSE_OUTPUT: if (this->Impl->Debug || this->Impl->ExtraVerbose) { - std::cout << msg << std::flush; + cm::StdIo::Print(cm::StdIo::Out(), attrs, msg); + cm::StdIo::Out().IOS() << std::flush; } break; case WARNING: - std::cerr << msg << std::flush; + cm::StdIo::Print(cm::StdIo::Err(), attrs, msg); + cm::StdIo::Err().IOS() << std::flush; break; case ERROR_MESSAGE: - std::cerr << msg << std::flush; + cm::StdIo::Print(cm::StdIo::Err(), attrs, msg); + cm::StdIo::Err().IOS() << std::flush; cmSystemTools::SetErrorOccurred(); break; default: - std::cout << msg << std::flush; + cm::StdIo::Print(cm::StdIo::Out(), attrs, msg); + cm::StdIo::Out().IOS() << std::flush; } } } -std::string cmCTest::GetColorCode(Color color) const -{ - if (this->Impl->OutputColorCode) { - return cmStrCat("\033[0;", static_cast(color), 'm'); - } - - return {}; -} - void cmCTest::SetTimeLimit(cmValue val) { this->Impl->TimeLimit = diff --git a/Source/cmCTest.h b/Source/cmCTest.h index 72f49eb76f..9bdedf5b98 100644 --- a/Source/cmCTest.h +++ b/Source/cmCTest.h @@ -18,6 +18,7 @@ #include "cmDuration.h" #include "cmProcessOutput.h" +#include "cmStdIoTerminal.h" class cmake; class cmCMakePresetsArgs; @@ -346,18 +347,10 @@ public: /** Add log to the output */ void Log(LogType logType, std::string msg, bool suppress = false); - /** Color values */ - enum class Color - { - CLEAR_COLOR = 0, - RED = 31, - GREEN = 32, - YELLOW = 33, - BLUE = 34 - }; - - /** Get color code characters for a specific color */ - std::string GetColorCode(Color color) const; + /** Add log to the output with terminal attributes for console output only. + */ + void Log(LogType logType, std::string msg, + cm::StdIo::TermAttrSet const& attrs, bool suppress = false); /** The Build ID is assigned by CDash */ void SetBuildID(std::string const& id); @@ -464,9 +457,6 @@ private: /** returns true iff the console supports progress output */ static bool ProgressOutputSupportedByConsole(); - /** returns true iff the console supports colored output */ - static bool ColoredOutputSupportedByConsole(); - /** Create note from files. */ int GenerateCTestNotesOutput(cmXMLWriter& xml, cmake* cm, std::vector const& files); @@ -496,3 +486,10 @@ private: cmCTestLog_msg << msg; \ (ctSelf)->Log(cmCTest::logType, cmCTestLog_msg.str(), suppress); \ } while (false) + +#define cmCTestColorLog(ctSelf, logType, attrs, msg) \ + do { \ + std::ostringstream cmCTestLog_msg; \ + cmCTestLog_msg << msg; \ + (ctSelf)->Log(cmCTest::logType, cmCTestLog_msg.str(), attrs); \ + } while (false) diff --git a/Tests/RunCMake/CTest/ColorOutput-test-check.cmake b/Tests/RunCMake/CTest/ColorOutput-test-check.cmake new file mode 100644 index 0000000000..1de4ae7cb2 --- /dev/null +++ b/Tests/RunCMake/CTest/ColorOutput-test-check.cmake @@ -0,0 +1,38 @@ +set(log "${RunCMake_TEST_BINARY_DIR}/output-log.txt") +if(NOT EXISTS "${log}") + set(RunCMake_TEST_FAILED "The expected output log file is missing:\n ${log}") + return() +endif() + +file(READ "${log}" log_content) + +string(ASCII 27 esc) +set(esc_re "${esc}\\[[0-9;]*m") + +if(log_content MATCHES "${esc_re}") + set(RunCMake_TEST_FAILED + "output-log.txt contains ANSI color escape sequences from CTest output.") +endif() + +if(NOT actual_stdout MATCHES "${esc_re}" AND NOT actual_stderr MATCHES "${esc_re}") + set(RunCMake_TEST_FAILED + "Neither stdout nor stderr contains ANSI color escape sequences with CLICOLOR_FORCE=1.") + return() +endif() + +if(NOT actual_stdout MATCHES + "${esc_re}[\n][0-9]+% tests passed, [0-9]+ tests failed out of [0-9]+[\n]${esc_re}") + set(RunCMake_TEST_FAILED "stdout does not contain color-wrapped failed-summary line.") +endif() + +if(actual_stdout MATCHES "${esc_re}The following tests FAILED:") + set(RunCMake_TEST_FAILED "The failed-tests header is unexpectedly colorized.") +endif() +if(NOT actual_stdout MATCHES + "The following tests FAILED:[\n]${esc_re}[^\n]*OutputLogNoEscape-Fail \\\(Failed\\\)") + set(RunCMake_TEST_FAILED "stdout does not contain color-wrapped failed-test entry.") +endif() + +if(actual_stdout MATCHES "${esc_re}Total Test time \\\(real\\\)") + set(RunCMake_TEST_FAILED "The total test time line is unexpectedly colorized.") +endif() diff --git a/Tests/RunCMake/CTest/ColorOutput-test-result.txt b/Tests/RunCMake/CTest/ColorOutput-test-result.txt new file mode 100644 index 0000000000..45a4fb75db --- /dev/null +++ b/Tests/RunCMake/CTest/ColorOutput-test-result.txt @@ -0,0 +1 @@ +8 diff --git a/Tests/RunCMake/CTest/ColorOutput-test-stderr.txt b/Tests/RunCMake/CTest/ColorOutput-test-stderr.txt new file mode 100644 index 0000000000..ba4235defb --- /dev/null +++ b/Tests/RunCMake/CTest/ColorOutput-test-stderr.txt @@ -0,0 +1 @@ +Errors while running CTest diff --git a/Tests/RunCMake/CTest/ColorOutput.cmake b/Tests/RunCMake/CTest/ColorOutput.cmake new file mode 100644 index 0000000000..09b0aa09e4 --- /dev/null +++ b/Tests/RunCMake/CTest/ColorOutput.cmake @@ -0,0 +1,7 @@ +include(CTest) + +add_test(NAME OutputLogNoEscape-Pass + COMMAND ${CMAKE_COMMAND} -E true) + +add_test(NAME OutputLogNoEscape-Fail + COMMAND ${CMAKE_COMMAND} -E false) diff --git a/Tests/RunCMake/CTest/RunCMakeTest.cmake b/Tests/RunCMake/CTest/RunCMakeTest.cmake index 4c2c107647..9803f01afd 100644 --- a/Tests/RunCMake/CTest/RunCMakeTest.cmake +++ b/Tests/RunCMake/CTest/RunCMakeTest.cmake @@ -40,6 +40,17 @@ if(NOT RunCMake_GENERATOR_IS_MULTI_CONFIG) run_SingleConfig() endif() +function(run_ColorOutput) + set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/ColorOutput-build) + run_cmake(ColorOutput) + set(RunCMake_TEST_NO_CLEAN 1) + run_cmake_command(ColorOutput-build ${CMAKE_COMMAND} --build . --config Debug) + run_cmake_command(ColorOutput-test + ${CMAKE_COMMAND} -E env CLICOLOR_FORCE=1 + ${CMAKE_CTEST_COMMAND} -C Debug --output-log output-log.txt) +endfunction() +run_ColorOutput() + run_cmake(CMP0145-Dart-OLD) run_cmake(CMP0145-Dart-WARN) run_cmake(CMP0145-Dart-NEW)