TAP test logger: move messages into the diagnostics block

Our TAP output was delivering messages as comments before the test line, where TAP clearly expects the details of a test to follow its test line. Version 13 provides a YAML block to deliver diagnostics and encourages use of this, so accumulate our messages in a QTestCharBuffer instead of emitting them one by one. However, messages produced after a test has produced its test line belong to that test, but are too late to be included in its diagnostics block, so should be emitted immediately as before, albeit now with a type prefix. This at least separates such messages, from the end of one test, from messages produced early in the next. In the process, add a type-prefix to each, to make clear what type of message it was. Since the Yamlish supported by TAP consumers doesn't support a way to have many messages, use the extensions: top-level hash tag with a messages: sub-tag to gather our messages as a list. (This expands at least one expected output file significantly and substantially rewrites some others.) Add methods to QTestCharBuffer, and a helper function, to support this. Task-number: QTBUG-96844 Change-Id: If44a33da5879ed1670ef0980042599afd516f9d2 Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>
author: Edward Welbourne <edward.welbourne@qt.io> 2022-03-14 19:16:51 +0100
committer: Edward Welbourne <edward.welbourne@qt.io> 2022-03-29 00:05:12 +0100
commit: ae37fa0464694c9770bd35472e2afac83a74564c (patch)
tree: 28a4b3ff158b4829d8dcddae850eacf74976c321 /src
parent: d329a98fa9fade95537a6ff35b5898611cfadd0b (diff)
4 files changed, 148 insertions, 29 deletions
diff --git a/src/testlib/qabstracttestlogger.cpp b/src/testlib/qabstracttestlogger.cpp
index cb9a0493570..1a0596ec98b 100644
--- a/src/testlib/qabstracttestlogger.cpp
+++ b/src/testlib/qabstracttestlogger.cpp
@@ -1,6 +1,6 @@
 /****************************************************************************
 **
-** Copyright (C) 2021 The Qt Company Ltd.
+** Copyright (C) 2022 The Qt Company Ltd.
 ** Contact: https://www.qt.io/licensing/
 **
 ** This file is part of the QtTest module of the Qt Toolkit.
@@ -399,6 +399,11 @@ void QAbstractTestLogger::addMessage(QtMsgType type, const QMessageLogContext &c
     addMessage(messageType, formattedMessage);
 }
 
+namespace
+{
+    constexpr int MAXSIZE = 1024 * 1024 * 2;
+}
+
 namespace QTest
 {
 
@@ -408,7 +413,6 @@ namespace QTest
  */
 int qt_asprintf(QTestCharBuffer *str, const char *format, ...)
 {
-    constexpr int MAXSIZE = 1024 * 1024 * 2;
     Q_ASSERT(str);
     int size = str->size();
     Q_ASSERT(size > 0);
@@ -456,6 +460,28 @@ void generateTestIdentifier(QTestCharBuffer *identifier, int parts)
         globalDataTag, tagFiller, dataTag, testFuctionEnd);
 }
 
+// strcat() for QTestCharBuffer objects:
+bool appendCharBuffer(QTestCharBuffer *accumulator, const QTestCharBuffer &more)
+{
+    const auto bufsize = [](const QTestCharBuffer &buf) -> int {
+        const int max = buf.size();
+        return max > 0 ? int(qstrnlen(buf.constData(), max)) : 0;
+    };
+    const int extra = bufsize(more);
+    if (extra <= 0)
+        return true; // Nothing to do, fatuous success
+
+    const int oldsize = bufsize(*accumulator);
+    const int newsize = oldsize + extra + 1; // 1 for final '\0'
+    if (newsize > MAXSIZE || !accumulator->resize(newsize))
+        return false; // too big or unable to grow
+
+    char *tail = accumulator->data() + oldsize;
+    memcpy(tail, more.constData(), extra);
+    tail[extra] = '\0';
+    return true;
+}
+
 }
 
 QT_END_NAMESPACE
diff --git a/src/testlib/qabstracttestlogger_p.h b/src/testlib/qabstracttestlogger_p.h
index 31fc3d60bab..d645340c614 100644
--- a/src/testlib/qabstracttestlogger_p.h
+++ b/src/testlib/qabstracttestlogger_p.h
@@ -1,6 +1,6 @@
 /****************************************************************************
 **
-** Copyright (C) 2021 The Qt Company Ltd.
+** Copyright (C) 2022 The Qt Company Ltd.
 ** Contact: https://www.qt.io/licensing/
 **
 ** This file is part of the QtTest module of the Qt Toolkit.
@@ -53,6 +53,7 @@
 
 #include <QtTest/qttestglobal.h>
 #include <QtCore/private/qglobal_p.h>
+#include <QtCore/qbytearrayalgorithms.h>
 
 #include <stdio.h>
 #include <stdlib.h>
@@ -155,12 +156,14 @@ struct QTestCharBuffer
         return _size;
     }
 
-    inline bool reset(int newSize)
+    bool reset(int newSize, bool copy = false)
     {
         char *newBuf = nullptr;
         if (buf == staticBuf) {
             // if we point to our internal buffer, we need to malloc first
             newBuf = reinterpret_cast<char *>(malloc(newSize));
+            if (copy && newBuf)
+                qstrncpy(newBuf, buf, _size);
         } else {
             // if we already malloc'ed, just realloc
             newBuf = reinterpret_cast<char *>(realloc(buf, newSize));
@@ -175,6 +178,13 @@ struct QTestCharBuffer
         return true;
     }
 
+    bool resize(int newSize) {
+        return newSize <= _size || reset(newSize, true);
+    }
+
+    void clear() { buf[0] = '\0'; }
+    bool isEmpty() { return buf[0] == '\0'; }
+
 private:
     int _size = InitialSize;
     char* buf;
@@ -190,6 +200,7 @@ namespace QTestPrivate
 {
     enum IdentifierPart { TestObject = 0x1, TestFunction = 0x2, TestDataTag = 0x4, AllParts = 0xFFFF };
     void Q_TESTLIB_EXPORT generateTestIdentifier(QTestCharBuffer *identifier, int parts = AllParts);
+    bool appendCharBuffer(QTestCharBuffer *accumulator, const QTestCharBuffer &more);
 }
 
 QT_END_NAMESPACE
diff --git a/src/testlib/qtaptestlogger.cpp b/src/testlib/qtaptestlogger.cpp
index dfe167974f9..c1a74eb52c0 100644
--- a/src/testlib/qtaptestlogger.cpp
+++ b/src/testlib/qtaptestlogger.cpp
@@ -164,33 +164,84 @@ void QTapTestLogger::stopLogging()
 
 void QTapTestLogger::enterTestFunction(const char *function)
 {
-    Q_UNUSED(function);
     m_wasExpectedFail = false;
+    Q_ASSERT(!m_gatherMessages);
+    Q_ASSERT(!hasMessages());
+    m_gatherMessages = function != nullptr;
 }
 
 void QTapTestLogger::enterTestData(QTestData *data)
 {
-    Q_UNUSED(data);
     m_wasExpectedFail = false;
+    if (hasMessages())
+        flushMessages();
+    m_gatherMessages = data != nullptr;
 }
 
 using namespace QTestPrivate;
 
-void QTapTestLogger::outputTestLine(bool ok, int testNumber, QTestCharBuffer &directive)
+void QTapTestLogger::outputTestLine(bool ok, int testNumber, const QTestCharBuffer &directive)
 {
     QTestCharBuffer testIdentifier;
     QTestPrivate::generateTestIdentifier(&testIdentifier, TestFunction | TestDataTag);
 
     QTestCharBuffer testLine;
-    QTest::qt_asprintf(&testLine, "%s %d - %s%s\n",
-        ok ? "ok" : "not ok", testNumber, testIdentifier.data(), directive.data());
+    QTest::qt_asprintf(&testLine, "%s %d - %s%s\n", ok ? "ok" : "not ok",
+                       testNumber, testIdentifier.data(), directive.constData());
 
     outputString(testLine.data());
 }
 
+// The indent needs to be two spaces for maximum compatibility.
+// This matches the width of the "- " prefix on a list item's first line.
+#define YAML_INDENT "  "
+
+void QTapTestLogger::outputBuffer(const QTestCharBuffer &buffer)
+{
+    if (!m_gatherMessages)
+        outputString(buffer.constData());
+    else if (buffer.constData()[strlen(YAML_INDENT)] == '#')
+        QTestPrivate::appendCharBuffer(&m_comments, buffer);
+    else
+        QTestPrivate::appendCharBuffer(&m_messages, buffer);
+}
+
+void QTapTestLogger::beginYamlish()
+{
+    outputString(YAML_INDENT "---\n");
+    // Flush any accumulated messages:
+    if (!m_comments.isEmpty()) {
+        outputString(m_comments.constData());
+        m_comments.clear();
+    }
+    if (!m_messages.isEmpty()) {
+        outputString(YAML_INDENT "extensions:\n");
+        outputString(YAML_INDENT YAML_INDENT "messages:\n");
+        outputString(m_messages.constData());
+        m_messages.clear();
+    }
+}
+
+void QTapTestLogger::endYamlish()
+{
+    outputString(YAML_INDENT "...\n");
+}
+
+void QTapTestLogger::flushMessages()
+{
+    /* A _data() function's messages show up here. */
+    QTestCharBuffer dataLine;
+    QTest::qt_asprintf(&dataLine, "ok %d - %s() # Data prepared\n",
+                       QTestLog::totalCount(), QTestResult::currentTestFunction());
+    outputString(dataLine.constData());
+    beginYamlish();
+    endYamlish();
+}
+
 void QTapTestLogger::addIncident(IncidentTypes type, const char *description,
                                  const char *file, int line)
 {
+    m_gatherMessages = false;
     if (m_wasExpectedFail && (type == Pass || type == BlacklistedPass
                               || type == XFail || type == BlacklistedXFail)) {
         // XFail comes with a corresponding Pass incident, but we only want
@@ -233,16 +284,12 @@ void QTapTestLogger::addIncident(IncidentTypes type, const char *description,
 
     outputTestLine(ok, testNumber, directive);
 
-    if (!ok) {
-        // All failures need a diagnostics section to not confuse consumers
-
-        // The indent needs to be two spaces for maximum compatibility.
-        // This matches the width of the "- " prefix on a list item's first line.
-        #define YAML_INDENT "  "
+    if (!ok || hasMessages()) {
+        // All failures need a diagnostics section to not confuse consumers.
+        // We also need a diagnostics section when we have messages to report.
+        beginYamlish();
 
-        outputString(YAML_INDENT "---\n");
-
-        if (type != XFail && type != BlacklistedXFail) {
+        if (!ok && type != XFail && type != BlacklistedXFail) {
 #if QT_CONFIG(regularexpression)
             // This is fragile, but unfortunately testlib doesn't plumb
             // the expected and actual values to the loggers (yet).
@@ -297,17 +344,17 @@ void QTapTestLogger::addIncident(IncidentTypes type, const char *description,
                     qPrintable(expected), qPrintable(actual)
                 );
 
-                outputString(diagnosticsYamlish.data());
+                outputBuffer(diagnosticsYamlish);
             } else
 #endif
             if (description && !incident) {
                 QTestCharBuffer unparsableDescription;
                 QTest::qt_asprintf(&unparsableDescription, YAML_INDENT "# %s\n", description);
-                outputString(unparsableDescription.data());
+                outputBuffer(unparsableDescription);
             }
         }
 
-        if (file) {
+        if (!ok && file) {
             QTestCharBuffer location;
             QTest::qt_asprintf(&location,
                 // The generic 'at' key is understood by most consumers.
@@ -322,10 +369,10 @@ void QTapTestLogger::addIncident(IncidentTypes type, const char *description,
                 QTestResult::currentTestFunction(),
                 file, line, file, line
             );
-            outputString(location.data());
+            outputBuffer(location);
         }
 
-        outputString(YAML_INDENT "...\n");
+        endYamlish();
     }
 }
 
@@ -334,11 +381,38 @@ void QTapTestLogger::addMessage(MessageTypes type, const QString &message,
 {
     Q_UNUSED(file);
     Q_UNUSED(line);
-    Q_UNUSED(type);
+    const char *const flavor = [type]() {
+        switch (type) {
+        case QDebug: return "debug";
+        case QInfo: return "info";
+        case QWarning: return "warning";
+        case QCritical: return "critical";
+        case QFatal: return "fatal";
+        // Handle internal messages as comments
+        case Info: return "# inform";
+        case Warn: return "# warn";
+        }
+        return "unrecognised message";
+    }();
 
-    QTestCharBuffer diagnostics;
-    QTest::qt_asprintf(&diagnostics, "# %s\n", qPrintable(message));
-    outputString(diagnostics.data());
+    QTestCharBuffer diagnostic;
+    if (!m_gatherMessages) {
+        QTest::qt_asprintf(&diagnostic, "%s%s: %s\n",
+                           flavor[0] == '#' ? "" : "# ",
+                           flavor, qPrintable(message));
+        outputString(diagnostic.constData());
+    } else if (flavor[0] == '#') {
+        QTest::qt_asprintf(&diagnostic, YAML_INDENT "%s: %s\n",
+                           flavor, qPrintable(message));
+        QTestPrivate::appendCharBuffer(&m_comments, diagnostic);
+    } else {
+        // These shall appear in a messages: sub-block of the extensions: block,
+        // so triple-indent.
+        QTest::qt_asprintf(&diagnostic, YAML_INDENT YAML_INDENT "- severity: %s\n"
+                           YAML_INDENT YAML_INDENT YAML_INDENT "message: %s\n",
+                           flavor, qPrintable(message));
+        QTestPrivate::appendCharBuffer(&m_messages, diagnostic);
+    }
 }
 
 QT_END_NAMESPACE
diff --git a/src/testlib/qtaptestlogger_p.h b/src/testlib/qtaptestlogger_p.h
index 72334bb5d33..99902b2f082 100644
--- a/src/testlib/qtaptestlogger_p.h
+++ b/src/testlib/qtaptestlogger_p.h
@@ -1,6 +1,6 @@
 /****************************************************************************
 **
-** Copyright (C) 2018 The Qt Company Ltd.
+** Copyright (C) 2022 The Qt Company Ltd.
 ** Contact: https://www.qt.io/licensing/
 **
 ** This file is part of the QtTest module of the Qt Toolkit.
@@ -76,8 +76,16 @@ public:
 
     void addBenchmarkResult(const QBenchmarkResult &) override {}
 private:
-    void outputTestLine(bool ok, int testNumber, QTestCharBuffer &directive);
+    void outputTestLine(bool ok, int testNumber, const QTestCharBuffer &directive);
+    void outputBuffer(const QTestCharBuffer &buffer);
+    bool hasMessages() const { return m_comments.constData()[0] || m_messages.constData()[0]; }
+    void flushMessages();
+    void beginYamlish();
+    void endYamlish();
     bool m_wasExpectedFail;
+    QTestCharBuffer m_comments;
+    QTestCharBuffer m_messages;
+    bool m_gatherMessages = false;
 };
 
 QT_END_NAMESPACE
author	Edward Welbourne <edward.welbourne@qt.io>	2022-03-14 19:16:51 +0100
committer	Edward Welbourne <edward.welbourne@qt.io>	2022-03-29 00:05:12 +0100
commit	ae37fa0464694c9770bd35472e2afac83a74564c (patch)
tree	28a4b3ff158b4829d8dcddae850eacf74976c321 /src
parent	d329a98fa9fade95537a6ff35b5898611cfadd0b (diff)