From ac14a02825875fb857ed071dc24ab5623e2f7022 Mon Sep 17 00:00:00 2001 From: Tex Riddell Date: Tue, 26 May 2026 19:17:50 -0700 Subject: [PATCH] Improve WEX logging through gtest on linux Previously, all important test logs from failing tests were omitted, only surfacing a generic failure message for the test method. Figuring out which test within that method failed, or how it failed required debugging the test failure. This change adds buffering for Comment logging, where all of those important details were kept. It throws away the comment logs if no failure occurs, but emits them if the current test fails within the StartGroup()/EndGroup() pair. This pair of calls is used around each test file in tests like the CompilerTest.Batch* tests. Code authored by copilot. --- include/dxc/Test/WEXAdapter.h | 35 +++-- tools/clang/unittests/HLSL/TestMain.cpp | 47 +++++- .../unittests/HLSLTestLib/CMakeLists.txt | 1 + .../unittests/HLSLTestLib/WEXAdapterLog.cpp | 142 ++++++++++++++++++ 4 files changed, 209 insertions(+), 16 deletions(-) create mode 100644 tools/clang/unittests/HLSLTestLib/WEXAdapterLog.cpp diff --git a/include/dxc/Test/WEXAdapter.h b/include/dxc/Test/WEXAdapter.h index f180c01a99..0054ce9b30 100644 --- a/include/dxc/Test/WEXAdapter.h +++ b/include/dxc/Test/WEXAdapter.h @@ -171,21 +171,26 @@ HRESULT TryGetValue(const wchar_t *param, Common::String &retStr); } // namespace TestExecution namespace Logging { namespace Log { -inline void StartGroup(const wchar_t *name) { - wprintf(L"BEGIN TEST(S): <%ls>\n", name); -} -inline void EndGroup(const wchar_t *name) { - wprintf(L"END TEST(S): <%ls>\n", name); -} -inline void Comment(const wchar_t *msg) { - fputws(msg, stdout); - fputwc(L'\n', stdout); -} -inline void Error(const wchar_t *msg) { - fputws(msg, stderr); - fputwc(L'\n', stderr); - ADD_FAILURE(); -} + +// Implementations live in tools/clang/unittests/HLSLTestLib/WEXAdapterLog.cpp. +// Comment messages are buffered per test and only printed when the test fails; +// Error messages are printed immediately and flush any buffered comments. See +// FailurePrinter in tools/clang/unittests/HLSL/TestMain.cpp for the listener +// that wires this up to GoogleTest's lifecycle. +void StartGroup(const wchar_t *name); +void EndGroup(const wchar_t *name); +void Comment(const wchar_t *msg); +void Error(const wchar_t *msg); + +// Accessors used by the gtest event listener. Not for direct use by tests. +const char *GetBufferedLog(); +void ClearBufferedLog(); +bool HasBufferedLog(); +// Called by the FailurePrinter listener when GoogleTest records a part +// failure; attributes that failure to the currently open StartGroup scope, +// if any. +void NotifyTestPartFailed(); + } // namespace Log } // namespace Logging } // namespace WEX diff --git a/tools/clang/unittests/HLSL/TestMain.cpp b/tools/clang/unittests/HLSL/TestMain.cpp index 8346f55390..df5401eeab 100644 --- a/tools/clang/unittests/HLSL/TestMain.cpp +++ b/tools/clang/unittests/HLSL/TestMain.cpp @@ -15,6 +15,10 @@ #include "HLSLTestOptions.h" #include "dxc/Test/WEXAdapter.h" +#include +#include +#include + #if defined(_WIN32) #include #if defined(_MSC_VER) @@ -56,16 +60,49 @@ class FailurePrinter : public TestEventListener { void OnTestStart(const TestInfo &ti) override { // Do not output on test start // defaultListener->OnTestStart(ti); +#ifndef _WIN32 + // Each test starts with an empty log buffer; clear any leftovers from a + // previous test that may have failed before reaching the end-of-test + // flush path. + WEX::Logging::Log::ClearBufferedLog(); +#endif } void OnTestPartResult(const TestPartResult &result) override { +#ifndef _WIN32 + if (result.failed()) { + // Attribute the failure to any currently open WEX StartGroup scope. + WEX::Logging::Log::NotifyTestPartFailed(); + // Suppress the gtest-default "WEXAdapterLog.cpp:N Failure / Failed" + // noise; the failing group name and Error text from the WEX log are + // what's actually informative. + const char *FileName = result.file_name(); + if (FileName && std::strstr(FileName, "WEXAdapterLog.cpp")) + return; + } +#endif defaultListener->OnTestPartResult(result); } void OnTestEnd(const TestInfo &ti) override { // Only output if failure on test end - if (ti.result()->Failed()) + if (ti.result()->Failed()) { +#ifndef _WIN32 + // Flush any comments emitted after the last failing assertion. + if (WEX::Logging::Log::HasBufferedLog()) { + std::fputs(WEX::Logging::Log::GetBufferedLog(), stderr); + WEX::Logging::Log::ClearBufferedLog(); + } +#endif defaultListener->OnTestEnd(ti); + } +#ifndef _WIN32 + else { + // Test passed -- discard the buffered comments so they don't leak into + // the next test. + WEX::Logging::Log::ClearBufferedLog(); + } +#endif } void OnTestCaseEnd(const TestCase &tc) override { @@ -108,6 +145,14 @@ const char *TestMainArgv0; int main(int argc, char **argv) { llvm::sys::PrintStackTraceOnErrorSignal(true /* Disable crash reporting */); +#ifndef _WIN32 + // Pick up the user's locale so wcstombs can convert wide log strings to + // UTF-8 in WEXAdapterLog.cpp. Without this, fputws/%ls silently drop wide + // output in the default "C" locale, hiding the WEX::Logging::Log::Comment + // diagnostics that tests rely on for context on failure. + std::setlocale(LC_ALL, ""); +#endif + for (int i = 1; i < argc; ++i) { ARG_LIST(SAVE_ARG) } diff --git a/tools/clang/unittests/HLSLTestLib/CMakeLists.txt b/tools/clang/unittests/HLSLTestLib/CMakeLists.txt index 2ade2b3138..4c12283029 100644 --- a/tools/clang/unittests/HLSLTestLib/CMakeLists.txt +++ b/tools/clang/unittests/HLSLTestLib/CMakeLists.txt @@ -11,6 +11,7 @@ add_clang_library(HLSLTestLib DxcTestUtils.cpp FileCheckerTest.cpp FileCheckForTest.cpp + WEXAdapterLog.cpp ) add_dependencies(HLSLTestLib TablegenHLSLOptions) diff --git a/tools/clang/unittests/HLSLTestLib/WEXAdapterLog.cpp b/tools/clang/unittests/HLSLTestLib/WEXAdapterLog.cpp new file mode 100644 index 0000000000..ba9031db82 --- /dev/null +++ b/tools/clang/unittests/HLSLTestLib/WEXAdapterLog.cpp @@ -0,0 +1,142 @@ +/////////////////////////////////////////////////////////////////////////////// +// // +// WEXAdapterLog.cpp // +// // +// Implements the WEX::Logging::Log functions declared in WEXAdapter.h for // +// non-Windows builds. // +// // +// Logging model: // +// * Comment() and Error() append to a per-test buffer. // +// * StartGroup()/EndGroup() bracket a sub-test scope; messages and // +// failures inside a group are scoped to that group. When EndGroup() // +// fires, the group's content is appended to the outer buffer only if // +// at least one failure was recorded inside it; otherwise it is // +// discarded. This matches the TAEF/WEX behavior on Windows. // +// * Error() records a non-fatal GoogleTest failure so the test is // +// marked failed; the originating file/line of the ADD_FAILURE is // +// suppressed by the FailurePrinter listener in TestMain.cpp so it // +// doesn't add noise -- the group name and Error message are what is // +// informative. // +// // +/////////////////////////////////////////////////////////////////////////////// + +#ifndef _WIN32 + +#include "dxc/Test/WEXAdapter.h" + +#include +#include +#include +#include + +namespace { + +struct LogState { + std::string TestBuffer; + std::string GroupBuffer; + std::string GroupName; + unsigned GroupFailureCount = 0; +}; + +LogState &State() { + static LogState S; + return S; +} + +std::string WideToUtf8(const wchar_t *Msg) { + if (!Msg) + return {}; + std::mbstate_t MBState{}; + const wchar_t *Src = Msg; + size_t Bytes = std::wcsrtombs(nullptr, &Src, 0, &MBState); + if (Bytes != static_cast(-1)) { + std::string Result(Bytes, '\0'); + Src = Msg; + MBState = {}; + std::wcsrtombs(&Result[0], &Src, Bytes, &MBState); + return Result; + } + // Locale conversion unavailable -- copy ASCII verbatim, replace rest. + std::string Result; + for (const wchar_t *P = Msg; *P; ++P) + Result.push_back(*P < 0x80 ? static_cast(*P) : '?'); + return Result; +} + +std::string &ActiveBuffer(LogState &S) { + return S.GroupName.empty() ? S.TestBuffer : S.GroupBuffer; +} + +} // namespace + +namespace WEX { +namespace Logging { +namespace Log { + +void StartGroup(const wchar_t *Name) { + LogState &S = State(); + // If a previous group was left open (missing EndGroup), surface its + // contents conservatively rather than dropping them. + if (!S.GroupName.empty()) { + if (S.GroupFailureCount > 0 || !S.GroupBuffer.empty()) + S.TestBuffer.append(S.GroupBuffer); + S.GroupBuffer.clear(); + S.GroupFailureCount = 0; + } + S.GroupName = WideToUtf8(Name); +} + +void EndGroup(const wchar_t *Name) { + LogState &S = State(); + if (S.GroupName.empty()) + return; + if (S.GroupFailureCount > 0) { + S.TestBuffer.append("---- FAILED: "); + S.TestBuffer.append(S.GroupName); + S.TestBuffer.append(" ----\n"); + S.TestBuffer.append(S.GroupBuffer); + } + S.GroupBuffer.clear(); + S.GroupName.clear(); + S.GroupFailureCount = 0; + (void)Name; // EndGroup's name is informational; group state is single-deep. +} + +void Comment(const wchar_t *Msg) { + LogState &S = State(); + ActiveBuffer(S).append(WideToUtf8(Msg)); + ActiveBuffer(S).push_back('\n'); +} + +void Error(const wchar_t *Msg) { + LogState &S = State(); + std::string MsgUtf8 = WideToUtf8(Msg); + ActiveBuffer(S).append("ERROR: "); + ActiveBuffer(S).append(MsgUtf8); + ActiveBuffer(S).push_back('\n'); + if (!S.GroupName.empty()) + ++S.GroupFailureCount; + ADD_FAILURE(); +} + +const char *GetBufferedLog() { return State().TestBuffer.c_str(); } +bool HasBufferedLog() { return !State().TestBuffer.empty(); } +void ClearBufferedLog() { + LogState &S = State(); + S.TestBuffer.clear(); + S.GroupBuffer.clear(); + S.GroupName.clear(); + S.GroupFailureCount = 0; +} + +void NotifyTestPartFailed() { + LogState &S = State(); + if (!S.GroupName.empty()) + ++S.GroupFailureCount; +} + +} // namespace Log +} // namespace Logging +} // namespace WEX + +#endif // _WIN32