Add github workflow to upload test failure markdown summary report as a comment to PRs when tests fail

.github/workflows/run-tests.yml

@@ -416,11 +416,18 @@ jobs:
           mkdir -p /tmp/minio_tests/test-bucket && chmod -R 777 /tmp/minio_tests
           docker-compose -f .docker/$DOCKERFILE run qgis-deps /root/QGIS/.docker/docker-qgis-test.sh $TEST_BATCH
 
+      - name: Save PR number to test report
+        if: ${{ failure() }}
+        env:
+          PR_NUMBER: ${{ github.event.number }}
+        run: |
+          echo $PR_NUMBER > qgis_test_report/pr_number
+
       - name: Archive test results report
         if: ${{ failure() }}
         uses: actions/upload-artifact@v3
         with:
-          name: test-results-${{ matrix.distro-version }}-qt${{ matrix.qt-version }}
+          name: test-results-qt${{ matrix.qt-version }}
           path: qgis_test_report
 
   clang-tidy:

.github/workflows/write_failure_comment.yml

@@ -0,0 +1,55 @@
+name: Write test failure comment
+
+on:
+  workflow_run:
+    workflows: [🧪 QGIS tests]
+    types:
+      - completed
+
+jobs:
+  strategy:
+    matrix:
+      qt-version: [ 5, 6 ]
+
+  on-failure:
+    download:
+      runs-on: ubuntu-latest
+      steps:
+        - name: 'Download artifact'
+          uses: actions/github-script@v6
+          with:
+            script: |
+              let allArtifacts = await github.rest.actions.listWorkflowRunArtifacts({
+                 owner: context.repo.owner,
+                 repo: context.repo.repo,
+                 run_id: context.payload.workflow_run.id,
+              });
+              let matchArtifact = allArtifacts.data.artifacts.filter((artifact) => {
+                return artifact.name == "test-results-qt${{ matrix.qt-version }}"
+              })[0];
+              let download = await github.rest.actions.downloadArtifact({
+                 owner: context.repo.owner,
+                 repo: context.repo.repo,
+                 artifact_id: matchArtifact.id,
+                 archive_format: 'zip',
+              });
+              let fs = require('fs');
+              fs.writeFileSync(`${process.env.GITHUB_WORKSPACE}/test-results-qt${{ matrix.qt-version }}.zip`, Buffer.from(download.data));
+
+        - name: 'Unzip artifact'
+          run: unzip test-results-qt${{ matrix.qt-version }}.zip
+
+        - name: 'Post test report markdown summary as comment on PR'
+          uses: actions/github-script@v6
+          with:
+            github-token: ${{ secrets.GITHUB_TOKEN }}
+            script: |
+              let fs = require('fs');
+              let issue_number = Number(fs.readFileSync('./test-results-qt${{ matrix.qt-version }}'));
+              let body = String(fs.readFileSync('./summary.md'));
+              await github.rest.issues.createComment({
+                owner: context.repo.owner,
+                repo: context.repo.repo,
+                issue_number: issue_number,
+                body: body
+              });

python/core/auto_generated/qgsmultirenderchecker.sip.in

@@ -124,9 +124,22 @@ Test using renderer to generate the image to be compared.
 
     QString report() const;
 %Docstring
-Returns a report for this test.
+Returns a HTML report for this test.
 
 The report will be empty if the test was successfully run.
+
+.. seealso:: :py:func:`markdownReport`
+%End
+
+    QString markdownReport() const;
+%Docstring
+Returns a markdown report for this test.
+
+The report will be empty if the test was successfully run.
+
+.. seealso:: :py:func:`report`
+
+.. versionadded:: 3.34
 %End
 
     QString controlImagePath() const;

python/core/auto_generated/qgsrenderchecker.sip.in

@@ -72,6 +72,20 @@ Returns the HTML report describing the results of the test run.
 
 If ``ignoreSuccess`` is ``True`` then the report will always be empty if
 the test was successful.
+
+.. seealso:: :py:func:`markdownReport`
+%End
+
+    QString markdownReport( bool ignoreSuccess = true ) const;
+%Docstring
+Returns the markdown report describing the results of the test run.
+
+If ``ignoreSuccess`` is ``True`` then the report will always be empty if
+the test was successful.
+
+.. seealso:: :py:func:`report`
+
+.. versionadded:: 3.34
 %End
 
     float matchPercent() const;

src/core/qgsmultirenderchecker.cpp

@@ -44,6 +44,7 @@ bool QgsMultiRenderChecker::runTest( const QString &testName, unsigned int misma
   mResult = false;
 
   mReport += "<h2>" + testName + "</h2>\n";
+  mMarkdownReport += QStringLiteral( "### %1\n\n" ).arg( testName );
 
   const QString baseDir = controlImagePath();
   if ( !QFile::exists( baseDir ) )
@@ -97,6 +98,10 @@ bool QgsMultiRenderChecker::runTest( const QString &testName, unsigned int misma
     dartMeasurements << checker.dartMeasurements();
 
     mReport += checker.report( false );
+    if ( subDirs.count() > 1 )
+      mMarkdownReport += QStringLiteral( "* " ) + checker.markdownReport( false );
+    else
+      mMarkdownReport += checker.markdownReport( false );
 
     if ( !mResult && diffImageFile.isEmpty() )
     {
@@ -173,6 +178,11 @@ QString QgsMultiRenderChecker::report() const
   return !mResult ? mReport : QString();
 }
 
+QString QgsMultiRenderChecker::markdownReport() const
+{
+  return !mResult ? mMarkdownReport : QString();
+}
+
 QString QgsMultiRenderChecker::controlImagePath() const
 {
   QString myDataDir( TEST_DATA_DIR ); //defined in CmakeLists.txt

src/core/qgsmultirenderchecker.h

@@ -128,12 +128,24 @@ class CORE_EXPORT QgsMultiRenderChecker
     bool runTest( const QString &testName, unsigned int mismatchCount = 0 );
 
     /**
-     * Returns a report for this test.
+     * Returns a HTML report for this test.
      *
      * The report will be empty if the test was successfully run.
+     *
+     * \see markdownReport()
      */
     QString report() const;
 
+    /**
+     * Returns a markdown report for this test.
+     *
+     * The report will be empty if the test was successfully run.
+     *
+     * \see report()
+     * \since QGIS 3.34
+     */
+    QString markdownReport() const;
+
     /**
      * Returns the path to the control images.
      */
@@ -148,6 +160,7 @@ class CORE_EXPORT QgsMultiRenderChecker
   private:
     bool mResult = false;
     QString mReport;
+    QString mMarkdownReport;
     QString mRenderedImage;
     QString mControlName;
     QString mControlPathPrefix;

src/core/qgsrenderchecker.cpp

@@ -62,6 +62,11 @@ QString QgsRenderChecker::report( bool ignoreSuccess ) const
   return ( ( ignoreSuccess && mResult ) || ( mExpectFail && !mResult ) ) ? QString() : mReport;
 }
 
+QString QgsRenderChecker::markdownReport( bool ignoreSuccess ) const
+{
+  return ( ( ignoreSuccess && mResult ) || ( mExpectFail && !mResult ) ) ? QString() : mMarkdownReport;
+}
+
 void QgsRenderChecker::setControlName( const QString &name )
 {
   mControlName = name;
@@ -252,6 +257,7 @@ bool QgsRenderChecker::runTest( const QString &testName,
               "<tr><td>Test Result:</td><td>Expected Result:</td></tr>\n"
               "<tr><td>Nothing rendered</td>\n<td>Failed because Expected "
               "Image File not set.</td></tr></table>\n";
+    mMarkdownReport = QStringLiteral( "Failed because expected image file not set\n" );
     performPostTestActions( flags );
     return mResult;
   }
@@ -266,6 +272,7 @@ bool QgsRenderChecker::runTest( const QString &testName,
               "<tr><td>Test Result:</td><td>Expected Result:</td></tr>\n"
               "<tr><td>Nothing rendered</td>\n<td>Failed because Expected "
               "Image File could not be loaded.</td></tr></table>\n";
+    mMarkdownReport = QStringLiteral( "Failed because expected image file (%1) could not be loaded\n" ).arg( mExpectedImageFile );
     performPostTestActions( flags );
     return mResult;
   }
@@ -304,6 +311,8 @@ bool QgsRenderChecker::runTest( const QString &testName,
               "<tr><td>Test Result:</td><td>Expected Result:</td></tr>\n"
               "<tr><td>Nothing rendered</td>\n<td>Failed because Rendered "
               "Image File could not be saved.</td></tr></table>\n";
+    mMarkdownReport = QStringLiteral( "Failed because rendered image file could not be saved to %1\n" ).arg( mRenderedImageFile );
+
     performPostTestActions( flags );
     return mResult;
   }
@@ -340,6 +349,8 @@ bool QgsRenderChecker::compareImages( const QString &testName,
               "<tr><td>Test Result:</td><td>Expected Result:</td></tr>\n"
               "<tr><td>Nothing rendered</td>\n<td>Failed because Expected "
               "Image File not set.</td></tr></table>\n";
+    mMarkdownReport = QStringLiteral( "Failed because expected image file was not set\n" );
+
     performPostTestActions( flags );
     return mResult;
   }
@@ -365,6 +376,7 @@ bool QgsRenderChecker::compareImages( const QString &testName, const QString &re
               "<tr><td>Test Result:</td><td>Expected Result:</td></tr>\n"
               "<tr><td>Nothing rendered</td>\n<td>Failed because Rendered "
               "Image File not set.</td></tr></table>\n";
+    mMarkdownReport = QStringLiteral( "Failed because rendered image file was not set\n" );
     performPostTestActions( flags );
     return mResult;
   }
@@ -380,6 +392,7 @@ bool QgsRenderChecker::compareImages( const QString &testName, const QString &re
               "<tr><td>Test Result:</td><td>Expected Result:</td></tr>\n"
               "<tr><td>Nothing rendered</td>\n<td>Failed because control "
               "image file could not be loaded.</td></tr></table>\n";
+    mMarkdownReport = QStringLiteral( "Failed because expected image file (%1) could not be loaded\n" ).arg( referenceImageFile );
     performPostTestActions( flags );
     return mResult;
   }
@@ -400,6 +413,7 @@ bool QgsRenderChecker::compareImages( const QString &testName, const QString &re
                               "<tr><td>Test Result:</td><td>%1:</td></tr>\n"
                               "<tr><td>Nothing rendered</td>\n<td>Failed because Rendered "
                               "Image File could not be loaded.</td></tr></table>\n" ).arg( upperFirst( expectedImageString ) );
+    mMarkdownReport = QStringLiteral( "Failed because rendered image (%1) could not be loaded\n" ).arg( mRenderedImageFile );
     performPostTestActions( flags );
     return mResult;
   }
@@ -508,6 +522,11 @@ bool QgsRenderChecker::compareImages( const QString &testName, const QString &re
       mReport += QLatin1String( "<tr><td colspan=3>" );
       mReport += QStringLiteral( "<font color=red>%1 and %2 for " ).arg( upperFirst( expectedImageString ), renderedImageString ) + testName + " are different dimensions - FAILING!</font>";
       mReport += QLatin1String( "</td></tr>" );
+      mMarkdownReport += QStringLiteral( "Failed because rendered image and expected image are different dimensions (%1x%2 v2 %3x%4)\n" )
+                         .arg( myResultImage.width() )
+                         .arg( myResultImage.height() )
+                         .arg( expectedImage.width() )
+                         .arg( expectedImage.height() );
 
       const QString diffSizeImagesString = QString(
                                              "<tr>"
@@ -548,6 +567,8 @@ bool QgsRenderChecker::compareImages( const QString &testName, const QString &re
       mReport += "<font color=red>Expected image and rendered image for " + testName + " have different formats (8bit format is expected) - FAILING!</font>";
       mReport += QLatin1String( "</td></tr>" );
       mReport += myImagesString;
+
+      mMarkdownReport += QStringLiteral( "Failed because rendered image and expected image have different formats (8bit format is expected)\n" );
       performPostTestActions( flags );
       return mResult;
     }
@@ -655,6 +676,9 @@ bool QgsRenderChecker::compareImages( const QString &testName, const QString &re
       mReport += QLatin1String( "<font color=red>Test failed because render step took too long</font>" );
       mReport += QLatin1String( "</td></tr>" );
       mReport += myImagesString;
+
+      mMarkdownReport += QStringLiteral( "Test failed because render step took too long\n" );
+
       performPostTestActions( flags );
       return mResult;
     }
@@ -680,6 +704,8 @@ bool QgsRenderChecker::compareImages( const QString &testName, const QString &re
   mReport += QLatin1String( "</td></tr>" );
   mReport += myImagesString;
 
+  mMarkdownReport += QStringLiteral( "Rendered image did not match %1 (found %2 pixels different)\n" ).arg( referenceImageFile ).arg( mMismatchCount );
+
   performPostTestActions( flags );
   return mResult;
 }

src/core/qgsrenderchecker.h

@@ -86,9 +86,22 @@ class CORE_EXPORT QgsRenderChecker
      *
      * If \a ignoreSuccess is TRUE then the report will always be empty if
      * the test was successful.
+     *
+     * \see markdownReport()
      */
     QString report( bool ignoreSuccess = true ) const;
 
+    /**
+     * Returns the markdown report describing the results of the test run.
+     *
+     * If \a ignoreSuccess is TRUE then the report will always be empty if
+     * the test was successful.
+     *
+     * \see report()
+     * \since QGIS 3.34
+     */
+    QString markdownReport( bool ignoreSuccess = true ) const;
+
     /**
      * Returns the percent of pixels which matched the control image.
      */
@@ -277,7 +290,10 @@ class CORE_EXPORT QgsRenderChecker
     QVector<QgsDartMeasurement> dartMeasurements() const { return mDashMessages; }
 
   protected:
+    //! HTML format report
     QString mReport;
+    //! Markdown report
+    QString mMarkdownReport;
     unsigned int mMatchTarget = 0;
     int mElapsedTime = 0;
     QString mRenderedImageFile;