Pq test harness usability and error reporting improvements.

Pq script errors are now printed in test output in case they are being masked by a later error.

Once a script error occurs, the same error will be thrown forever rather than throwing a new error on the next item in the script.

HRNPQ_MACRO_CLOSE() is not required in scripts unless harnessPqScriptStrictSet(true) is called.  Most higher-level tests should not need to run in strict mode.

The command/check test seems to require strict mode but there's no apparent reason why it should.  This would be a good thing to look into at some point.

test/src/common/harnessPq.c

@@ -12,16 +12,27 @@ Pq Test Harness
 #include "common/type/variantList.h"
 
 #include "common/harnessPq.h"
+#include "common/harnessTest.h"
+
+/***********************************************************************************************************************************
+Pq shim error prefix
+***********************************************************************************************************************************/
+#define PQ_ERROR_PREFIX                                             "PQ SHIM ERROR"
 
 /***********************************************************************************************************************************
 Script that defines how shim functions operate
 ***********************************************************************************************************************************/
-HarnessPq *harnessPqScript;
+HarnessPq harnessPqScript[1024];
+bool harnessPqScriptDone = true;
 unsigned int harnessPqScriptIdx;
 
+// Is PQfinish scripting required?
+bool harnessPqStrict = false;
+
 // If there is a script failure change the behavior of cleanup functions to return immediately so the real error will be reported
 // rather than a bogus scripting error during cleanup
 bool harnessPqScriptFail;
+char harnessPqScriptError[4096];
 
 /***********************************************************************************************************************************
 Set pq script
@@ -29,30 +40,55 @@ Set pq script
 void
 harnessPqScriptSet(HarnessPq *harnessPqScriptParam)
 {
-    if (harnessPqScript != NULL)
+    if (!harnessPqScriptDone)
         THROW(AssertError, "previous pq script has not yet completed");
 
     if (harnessPqScriptParam[0].function == NULL)
         THROW(AssertError, "pq script must have entries");
 
-    harnessPqScript = harnessPqScriptParam;
+    // Copy records into local storage
+    unsigned int copyIdx = 0;
+
+    while (harnessPqScriptParam[copyIdx].function != NULL)
+    {
+        harnessPqScript[copyIdx] = harnessPqScriptParam[copyIdx];
+        copyIdx++;
+    }
+
+    harnessPqScript[copyIdx].function = NULL;
+    harnessPqScriptDone = false;
     harnessPqScriptIdx = 0;
 }
 
+/**********************************************************************************************************************************/
+void
+harnessPqScriptStrictSet(bool strict)
+{
+    harnessPqStrict = strict;
+}
+
 /***********************************************************************************************************************************
 Run pq script
 ***********************************************************************************************************************************/
 static HarnessPq *
 harnessPqScriptRun(const char *function, const VariantList *param, HarnessPq *parent)
 {
+    // If an error has already been thrown then throw the same error again
+    if (harnessPqScriptFail)
+        THROW(AssertError, harnessPqScriptError);
+
     // Convert params to json for comparison and reporting
     String *paramStr = param ? jsonFromVar(varNewVarLst(param)) : strNew("");
 
     // Ensure script has not ended
-    if (harnessPqScript == NULL)
+    if (harnessPqScriptDone)
     {
+        snprintf(harnessPqScriptError, sizeof(harnessPqScriptError), "pq script ended before %s (%s)", function, strPtr(paramStr));
+
+        TEST_LOG_FMT(PQ_ERROR_PREFIX ": %s", harnessPqScriptError);
         harnessPqScriptFail = true;
-        THROW_FMT(AssertError, "pq script ended before %s (%s)", function, strPtr(paramStr));
+
+        THROW(AssertError, harnessPqScriptError);
     }
 
     // Get current script item
@@ -61,30 +97,41 @@ harnessPqScriptRun(const char *function, const VariantList *param, HarnessPq *pa
     // Check that expected function was called
     if (strcmp(result->function, function) != 0)
     {
+        snprintf(
+            harnessPqScriptError, sizeof(harnessPqScriptError), "pq script [%u] expected function %s (%s) but got %s (%s)",
+            harnessPqScriptIdx, result->function, result->param == NULL ? "" : result->param, function, strPtr(paramStr));
+
+        TEST_LOG_FMT(PQ_ERROR_PREFIX ": %s", harnessPqScriptError);
         harnessPqScriptFail = true;
 
-        THROW_FMT(
+        THROW(AssertError, harnessPqScriptError);
-            AssertError, "pq script [%u] expected function %s (%s) but got %s (%s)", harnessPqScriptIdx, result->function,
-            result->param == NULL ? "" : result->param, function, strPtr(paramStr));
     }
 
     // Check that parameters match
     if ((param != NULL && result->param == NULL) || (param == NULL && result->param != NULL) ||
         (param != NULL && result->param != NULL && !strEqZ(paramStr, result->param)))
     {
+        snprintf(
+            harnessPqScriptError, sizeof(harnessPqScriptError), "pq script [%u] function '%s', expects param '%s' but got '%s'",
+            harnessPqScriptIdx, result->function, result->param ? result->param : "NULL", param ? strPtr(paramStr) : "NULL");
+
+        TEST_LOG_FMT(PQ_ERROR_PREFIX ": %s", harnessPqScriptError);
         harnessPqScriptFail = true;
 
-        THROW_FMT(
+        THROW(AssertError, harnessPqScriptError);
-            AssertError, "pq script [%u] function '%s', expects param '%s' but got '%s'", harnessPqScriptIdx, result->function,
-            result->param ? result->param : "NULL", param ? strPtr(paramStr) : "NULL");
     }
 
     // Make sure the session matches with the parent as a sanity check
     if (parent != NULL && result->session != parent->session)
     {
-        THROW_FMT(
+        snprintf(
-            AssertError, "pq script [%u] function '%s', expects session '%u' but got '%u'", harnessPqScriptIdx, result->function,
+            harnessPqScriptError, sizeof(harnessPqScriptError), "pq script [%u] function '%s', expects session '%u' but got '%u'",
-            result->session, parent->session);
+            harnessPqScriptIdx, result->function, result->session, parent->session);
+
+        TEST_LOG_FMT(PQ_ERROR_PREFIX ": %s", harnessPqScriptError);
+        harnessPqScriptFail = true;
+
+        THROW(AssertError, harnessPqScriptError);
     }
 
     // Sleep if requested
@@ -94,7 +141,7 @@ harnessPqScriptRun(const char *function, const VariantList *param, HarnessPq *pa
     harnessPqScriptIdx++;
 
     if (harnessPqScript[harnessPqScriptIdx].function == NULL)
-        harnessPqScript = NULL;
+        harnessPqScriptDone = true;
 
     return result;
 }
@@ -293,7 +340,7 @@ Shim for PQfinish()
 ***********************************************************************************************************************************/
 void PQfinish(PGconn *conn)
 {
-    if (!harnessPqScriptFail)
+    if (harnessPqStrict && !harnessPqScriptFail)
         harnessPqScriptRun(HRNPQ_FINISH, NULL, (HarnessPq *)conn);
 }
 

test/src/common/harnessPq.h

@@ -181,6 +181,11 @@ Functions
 ***********************************************************************************************************************************/
 void harnessPqScriptSet(HarnessPq *harnessPqScriptParam);
 
+// Are we strict about requiring PQfinish()?  Strict is a good idea for low-level testing of Pq code but is a nuissance for
+// higher-level testing since it can mask other errors.  When not strict, PGfinish() is allowed at any time and does not need to be
+// scripted.
+void harnessPqScriptStrictSet(bool strict);
+
 #endif // HARNESS_PQ_REAL
 
 #endif

test/src/module/command/checkTest.c

@@ -21,6 +21,9 @@ testRun(void)
 {
     FUNCTION_HARNESS_VOID();
 
+    // PQfinish() is strictly checked
+    harnessPqScriptStrictSet(true);
+
     Storage *storageTest = storagePosixNew(
         strNew(testPath()), STORAGE_MODE_FILE_DEFAULT, STORAGE_MODE_PATH_DEFAULT, true, NULL);
 

test/src/module/command/stanzaTest.c

@@ -393,7 +393,6 @@ testRun(void)
         harnessPqScriptSet((HarnessPq [])
         {
             HRNPQ_MACRO_OPEN_92(1, "dbname='postgres' port=5432", strPtr(pg1Path), false, NULL, NULL),
-            HRNPQ_MACRO_CLOSE(1),
             HRNPQ_MACRO_DONE()
         });
 
@@ -404,7 +403,6 @@ testRun(void)
         harnessPqScriptSet((HarnessPq [])
         {
             HRNPQ_MACRO_OPEN_92(1, "dbname='postgres' port=5432", strPtr(pg1Path), false, NULL, NULL),
-            HRNPQ_MACRO_CLOSE(1),
             HRNPQ_MACRO_DONE()
         });
 
@@ -421,7 +419,6 @@ testRun(void)
         harnessPqScriptSet((HarnessPq [])
         {
             HRNPQ_MACRO_OPEN_92(1, "dbname='postgres' port=5432", strPtr(pg1Path), false, NULL, NULL),
-            HRNPQ_MACRO_CLOSE(1),
             HRNPQ_MACRO_DONE()
         });
 
@@ -442,7 +439,6 @@ testRun(void)
         harnessPqScriptSet((HarnessPq [])
         {
             HRNPQ_MACRO_OPEN_92(1, "dbname='postgres' port=5432", strPtr(strNewFmt("%s/pg2", testPath())), false, NULL, NULL),
-            HRNPQ_MACRO_CLOSE(1),
             HRNPQ_MACRO_DONE()
         });
 
@@ -477,8 +473,6 @@ testRun(void)
         {
             HRNPQ_MACRO_OPEN_92(1, "dbname='postgres' port=5432", testPath(), true, NULL, NULL),
             HRNPQ_MACRO_OPEN_92(2, "dbname='postgres' port=5434", strPtr(pg1Path), false, NULL, NULL),
-            HRNPQ_MACRO_CLOSE(2),
-            HRNPQ_MACRO_CLOSE(1),
             HRNPQ_MACRO_DONE()
         });
 

test/src/module/db/dbTest.c

@@ -17,6 +17,9 @@ testRun(void)
 {
     FUNCTION_HARNESS_VOID();
 
+    // PQfinish() is strictly checked
+    harnessPqScriptStrictSet(true);
+
     // *****************************************************************************************************************************
     if (testBegin("Db and dbProtocol()"))
     {

test/src/module/postgres/clientTest.c

@@ -20,6 +20,9 @@ testRun(void)
 {
     FUNCTION_HARNESS_VOID();
 
+    // PQfinish() is strictly checked
+    harnessPqScriptStrictSet(true);
+
     // *****************************************************************************************************************************
     if (testBegin("pgClient"))
     {