From d205a61949ae8491f5ce9b2019cda79d2fe7b216 Mon Sep 17 00:00:00 2001 From: David Steele Date: Fri, 1 Dec 2023 11:54:30 -0300 Subject: [PATCH] Fix flapping test on older ninja versions in test unit. Older versions of ninja may fail to rebuild correctly when changes are made to the configuration. In this case there is an automatic retry but the unexpected log output would cause the test to fail. For tests that are expected to succeed, check that the log is empty but also accept a retry message as long as the test does eventually succeed. Add a new harness function, harnessLogResultEmptyOrContains(), to make this work and also clean up some adjacent code. --- test/src/common/harnessLog.c | 36 ++++++++++++++++++++++++++------- test/src/common/harnessLog.h | 4 ++++ test/src/common/harnessTest.h | 14 +++++++++++++ test/src/module/test/testTest.c | 12 +++++++++++ 4 files changed, 59 insertions(+), 7 deletions(-) diff --git a/test/src/common/harnessLog.c b/test/src/common/harnessLog.c index 08dc68584..738ffacd5 100644 --- a/test/src/common/harnessLog.c +++ b/test/src/common/harnessLog.c @@ -390,20 +390,16 @@ hrnLogReplace(void) FUNCTION_HARNESS_RETURN_VOID(); } -/*********************************************************************************************************************************** -Compare log to a static string - -After the comparison the log is cleared so the next result can be compared. -***********************************************************************************************************************************/ +/**********************************************************************************************************************************/ void harnessLogResult(const char *expected) { FUNCTION_HARNESS_BEGIN(); FUNCTION_HARNESS_PARAM(STRINGZ, expected); - - FUNCTION_HARNESS_ASSERT(expected != NULL); FUNCTION_HARNESS_END(); + ASSERT(expected != NULL); + harnessLogLoad(logFile); hrnLogReplace(); @@ -420,6 +416,32 @@ harnessLogResult(const char *expected) FUNCTION_HARNESS_RETURN_VOID(); } +/**********************************************************************************************************************************/ +void +harnessLogResultEmptyOrContains(const char *const contains) +{ + FUNCTION_HARNESS_BEGIN(); + FUNCTION_HARNESS_PARAM(STRINGZ, contains); + FUNCTION_HARNESS_END(); + + ASSERT(contains != NULL); + + harnessLogLoad(logFile); + hrnLogReplace(); + + if (strlen(harnessLogBuffer) != 0 && strstr(harnessLogBuffer, contains) == NULL) + { + THROW_FMT( + AssertError, "\nLOG MUST CONTAIN:\n\n%s\n\nBUT WAS ACTUALLY:\n\n%s", + contains, harnessLogBuffer); + } + + close(logFdFile); + logFdFile = harnessLogOpen(logFile, O_WRONLY | O_CREAT | O_TRUNC, 0640); + + FUNCTION_HARNESS_RETURN_VOID(); +} + /*********************************************************************************************************************************** Make sure nothing is left in the log after all tests have completed ***********************************************************************************************************************************/ diff --git a/test/src/common/harnessLog.h b/test/src/common/harnessLog.h index 8dea36580..981867163 100644 --- a/test/src/common/harnessLog.h +++ b/test/src/common/harnessLog.h @@ -17,8 +17,12 @@ void hrnLogReplaceAdd(const char *expression, const char *expressionSub, const c // Clear (remove) all log replacements void hrnLogReplaceClear(void); +// Compare log to a static string void harnessLogResult(const char *expected); +// Check that log contains a substring +void harnessLogResultEmptyOrContains(const char *const contains); + /*********************************************************************************************************************************** Getters/Setters ***********************************************************************************************************************************/ diff --git a/test/src/common/harnessTest.h b/test/src/common/harnessTest.h index 406f2e183..f9488f94b 100644 --- a/test/src/common/harnessTest.h +++ b/test/src/common/harnessTest.h @@ -323,6 +323,20 @@ Test log result TRY_END(); \ } while (0) +#define TEST_RESULT_LOG_EMPTY_OR_CONTAINS(expected) \ + do \ + { \ + TRY_BEGIN() \ + { \ + harnessLogResultEmptyOrContains(expected); \ + } \ + CATCH_ANY() \ + { \ + THROW_FMT(AssertError, "LOG RESULT CONTAINS FAILED WITH:\n%s", errorMessage()); \ + } \ + TRY_END(); \ + } while (0) + #define TEST_RESULT_LOG_FMT(...) \ do \ { \ diff --git a/test/src/module/test/testTest.c b/test/src/module/test/testTest.c index 23c1078c0..ad9f39cdc 100644 --- a/test/src/module/test/testTest.c +++ b/test/src/module/test/testTest.c @@ -267,6 +267,9 @@ testRun(void) STRDEF("common/stack-trace"), 0, 1, logLevelDebug, true, NULL, false, false, false, true), "new build"); + // Older versions of ninja may error on a rebuild so a retry may occur + TEST_RESULT_LOG_EMPTY_OR_CONTAINS("WARN: build failed for unit"); + const Storage *storageUnit = storagePosixNewP(STRDEF(TEST_PATH "/test/unit-3/none")); StringList *fileList = testStorageList(storageUnit); @@ -388,6 +391,9 @@ testRun(void) STRDEF("common/error"), 5, 1, logLevelDebug, true, NULL, false, false, false, true), "new build"); + // Older versions of ninja may error on a rebuild so a retry may occur + TEST_RESULT_LOG_EMPTY_OR_CONTAINS("WARN: build failed for unit"); + fileList = testStorageList(storageUnit); TEST_RESULT_STRLST_Z( @@ -579,6 +585,9 @@ testRun(void) STRDEF("test/shim"), 0, 1, logLevelDebug, true, NULL, true, true, true, true), "new build"); + // Older versions of ninja may error on a rebuild so a retry may occur + TEST_RESULT_LOG_EMPTY_OR_CONTAINS("WARN: build failed for unit"); + storageUnit = storagePosixNewP(STRDEF(TEST_PATH "/test/unit-3/uXX")); fileList = testStorageList(storageUnit); @@ -885,6 +894,9 @@ testRun(void) STRDEF("performance/type"), 0, 1, logLevelDebug, true, STRDEF("America/New_York"), false, false, false, false), "new build"); + // Older versions of ninja may error on a rebuild so a retry may occur + TEST_RESULT_LOG_EMPTY_OR_CONTAINS("WARN: build failed for unit"); + storageUnit = storagePosixNewP(STRDEF(TEST_PATH "/test/unit-3/uXX")); // Test one file to make sure profiling was not enabled