diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index 12700042f270..46a3e77d656f 100644 --- a/.github/workflows/run-tests.yml +++ b/.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: diff --git a/.github/workflows/write_failure_comment.yml b/.github/workflows/write_failure_comment.yml new file mode 100644 index 000000000000..ee148ea43520 --- /dev/null +++ b/.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 + }); diff --git a/python/core/auto_generated/qgsmultirenderchecker.sip.in b/python/core/auto_generated/qgsmultirenderchecker.sip.in index 5e14d27db78e..5d3fa1d859ef 100644 --- a/python/core/auto_generated/qgsmultirenderchecker.sip.in +++ b/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; diff --git a/python/core/auto_generated/qgsrenderchecker.sip.in b/python/core/auto_generated/qgsrenderchecker.sip.in index 7b52dcc66b90..42c65b30a987 100644 --- a/python/core/auto_generated/qgsrenderchecker.sip.in +++ b/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; diff --git a/src/core/qgsmultirenderchecker.cpp b/src/core/qgsmultirenderchecker.cpp index de4f5e05cd5e..881513782d1c 100644 --- a/src/core/qgsmultirenderchecker.cpp +++ b/src/core/qgsmultirenderchecker.cpp @@ -44,6 +44,7 @@ bool QgsMultiRenderChecker::runTest( const QString &testName, unsigned int misma mResult = false; mReport += "

" + testName + "

\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 diff --git a/src/core/qgsmultirenderchecker.h b/src/core/qgsmultirenderchecker.h index 5e34ebdaa371..46eacac9a3a0 100644 --- a/src/core/qgsmultirenderchecker.h +++ b/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; diff --git a/src/core/qgsrenderchecker.cpp b/src/core/qgsrenderchecker.cpp index 2d3b1f1603ae..9152ed505fbd 100644 --- a/src/core/qgsrenderchecker.cpp +++ b/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, "Test Result:Expected Result:\n" "Nothing rendered\nFailed because Expected " "Image File not set.\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, "Test Result:Expected Result:\n" "Nothing rendered\nFailed because Expected " "Image File could not be loaded.\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, "Test Result:Expected Result:\n" "Nothing rendered\nFailed because Rendered " "Image File could not be saved.\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, "Test Result:Expected Result:\n" "Nothing rendered\nFailed because Expected " "Image File not set.\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 "Test Result:Expected Result:\n" "Nothing rendered\nFailed because Rendered " "Image File not set.\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 "Test Result:Expected Result:\n" "Nothing rendered\nFailed because control " "image file could not be loaded.\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 "Test Result:%1:\n" "Nothing rendered\nFailed because Rendered " "Image File could not be loaded.\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( "" ); mReport += QStringLiteral( "%1 and %2 for " ).arg( upperFirst( expectedImageString ), renderedImageString ) + testName + " are different dimensions - FAILING!"; mReport += QLatin1String( "" ); + 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( "" @@ -548,6 +567,8 @@ bool QgsRenderChecker::compareImages( const QString &testName, const QString &re mReport += "Expected image and rendered image for " + testName + " have different formats (8bit format is expected) - FAILING!"; mReport += QLatin1String( "" ); 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( "Test failed because render step took too long" ); mReport += QLatin1String( "" ); 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( "" ); mReport += myImagesString; + mMarkdownReport += QStringLiteral( "Rendered image did not match %1 (found %2 pixels different)\n" ).arg( referenceImageFile ).arg( mMismatchCount ); + performPostTestActions( flags ); return mResult; } diff --git a/src/core/qgsrenderchecker.h b/src/core/qgsrenderchecker.h index 367ae541ab9f..d0763e1115d1 100644 --- a/src/core/qgsrenderchecker.h +++ b/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 dartMeasurements() const { return mDashMessages; } protected: + //! HTML format report QString mReport; + //! Markdown report + QString mMarkdownReport; unsigned int mMatchTarget = 0; int mElapsedTime = 0; QString mRenderedImageFile; diff --git a/src/test/qgstest.h b/src/test/qgstest.h index 5f678361392f..f5b1cddbcb08 100644 --- a/src/test/qgstest.h +++ b/src/test/qgstest.h @@ -151,6 +151,8 @@ class TEST_EXPORT QgsTest : public QObject { if ( !mReport.isEmpty() ) writeLocalHtmlReport( mReport ); + if ( !mMarkdownReport.isEmpty() ) + writeMarkdownReport( mMarkdownReport ); } /** @@ -264,7 +266,7 @@ class TEST_EXPORT QgsTest : public QObject const bool result = checker.runTest( name, allowedMismatch ); if ( !result ) { - appendToReport( name, checker.report() ); + appendToReport( name, checker.report(), checker.markdownReport() ); } return result; @@ -290,7 +292,7 @@ class TEST_EXPORT QgsTest : public QObject const bool result = checker.runTest( name, allowedMismatch ); if ( !result ) { - appendToReport( name, checker.report() ); + appendToReport( name, checker.report(), checker.markdownReport() ); } return result; } @@ -308,29 +310,42 @@ class TEST_EXPORT QgsTest : public QObject const bool result = checker.testLayout( report, page, allowedMismatch ); if ( !result ) { - appendToReport( name, report ); + appendToReport( name, report, checker.markdownReport() ); } return result; } /** - * Appends some \a content to the test report. + * Appends some \a html and \a markdown to the test report. * * This should be used only for appending useful information when a test fails. */ - void appendToReport( const QString &testName, const QString &content ) + void appendToReport( const QString &testName, const QString &html, const QString &markdown = QString() ) { QString testIdentifier; if ( QTest::currentDataTag() ) testIdentifier = QStringLiteral( "%1 (%2: %3)" ).arg( testName, QTest::currentTestFunction(), QTest::currentDataTag() ); else testIdentifier = QStringLiteral( "%1 (%2)" ).arg( testName, QTest::currentTestFunction() ); - mReport += QStringLiteral( "

%1

\n" ).arg( testIdentifier ); - mReport += content; + + if ( !html.isEmpty() ) + { + mReport += QStringLiteral( "

%1

\n" ).arg( testIdentifier ); + mReport += html; + } + + const QString markdownContent = markdown.isEmpty() ? html : markdown; + if ( !markdownContent.isEmpty() ) + { + mMarkdownReport += QStringLiteral( "## %1\n\n" ).arg( testIdentifier ); + mMarkdownReport += markdownContent + QStringLiteral( "\n\n" ); + } } private: + QString mMarkdownReport; + /** * Writes out a HTML report to a temporary file for visual comparison * of test results on a local build. @@ -363,6 +378,33 @@ class TEST_EXPORT QgsTest : public QObject } } + /** + * Writes out a markdown report to a temporary file for use on CI runs. + */ + void writeMarkdownReport( const QString &report ) + { + const QDir reportDir = QgsRenderChecker::testReportDir(); + if ( !reportDir.exists() ) + QDir().mkpath( reportDir.path() ); + + const QString reportFile = reportDir.filePath( "summary.md" ); + QFile file( reportFile ); + + QFile::OpenMode mode = QIODevice::WriteOnly; + if ( qgetenv( "QGIS_CONTINUOUS_INTEGRATION_RUN" ) == QStringLiteral( "true" ) + || qgetenv( "QGIS_APPEND_TO_TEST_REPORT" ) == QStringLiteral( "true" ) ) + mode |= QIODevice::Append; + else + mode |= QIODevice::Truncate; + + if ( file.open( mode ) ) + { + QTextStream stream( &file ); + stream << report; + file.close(); + } + } + }; /** diff --git a/tests/src/core/testqgscompositionconverter.cpp b/tests/src/core/testqgscompositionconverter.cpp index 65fee245680c..f7133a5ad1f1 100644 --- a/tests/src/core/testqgscompositionconverter.cpp +++ b/tests/src/core/testqgscompositionconverter.cpp @@ -22,7 +22,6 @@ #include "qgscompositionconverter.h" #include "qgsproject.h" #include "qgsreadwritecontext.h" -#include "qgsmultirenderchecker.h" #include "qgslayoutmanager.h" #include "qgslayoutpagecollection.h" #include "qgslayoutitemlabel.h" @@ -54,7 +53,7 @@ class TestQgsCompositionConverter: public QgsTest Q_OBJECT public: - TestQgsCompositionConverter() : QgsTest( QStringLiteral( "Composition Converter Tests" ) ) {} + TestQgsCompositionConverter() : QgsTest( QStringLiteral( "Composition Converter Tests" ), QStringLiteral( "compositionconverter" ) ) {} private slots: void initTestCase();// will be called before the first testfunction is executed. @@ -711,14 +710,16 @@ void TestQgsCompositionConverter::importComposerAtlas() void TestQgsCompositionConverter::checkRenderedImage( QgsLayout *layout, const QString &testName, const int pageNumber ) { - QgsLayoutChecker checker( testName + '_' + QString::number( pageNumber ), layout ); - QSize size( layout->pageCollection()->page( pageNumber )->sizeWithUnits().width() * 3.77, layout->pageCollection()->page( pageNumber )->sizeWithUnits().height() * 3.77 ); - checker.setSize( size ); - checker.setControlPathPrefix( QStringLiteral( "compositionconverter" ) ); - QVERIFY( checker.testLayout( mReport, pageNumber, 0, false ) ); + const QSize size( layout->pageCollection()->page( pageNumber )->sizeWithUnits().width() * 3.77, layout->pageCollection()->page( pageNumber )->sizeWithUnits().height() * 3.77 ); + + QVERIFY( + layoutCheck( + testName + '_' + QString::number( pageNumber ), + layout, + pageNumber, 0, size, 0 ) + ); } - QDomElement TestQgsCompositionConverter::loadComposer( const QString &name ) { QString templatePath( QStringLiteral( TEST_DATA_DIR ) + "/layouts/" + name );