gifrecorder: improve the code

- Rename startProcess() to waitForProcessToStart(), since that's what
  it does, and it makes it consistent with waitForProcessToFinish().
- Add ProcessWaitResult struct for error reporting and make both
  waitForProcessToStart() and waitForProcessToFinish() use it.
  This allows us to stop using QFAIL in helper functions, which will
  not abort the test.
- Add some double quotes in debug output to help readability.
- Add code comments to explain stuff that I've forgotten and had to
  look up.

Change-Id: Ie54c0ee752d57acd3501b72025fc1eb482dfa238
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>

1 files changed, 45 insertions, 22 deletions

diff --git a/tests/manual/gifs/gifrecorder.cpp b/tests/manual/gifs/gifrecorder.cpp
index ec63e155..f6b45257 100644
--- a/tests/manual/gifs/gifrecorder.cpp
+++ b/tests/manual/gifs/gifrecorder.cpp
@@ -142,29 +142,36 @@ QQuickWindow *GifRecorder::window() const
 }
 
 namespace {
-    void startProcess(QProcess &process, const QString &processName, const QString &args)
+    struct ProcessWaitResult {
+        bool success;
+        QString errorMessage;
+    };
+
+    ProcessWaitResult waitForProcessToStart(QProcess &process, const QString &processName, const QString &args)
     {
         qCDebug(lcGifRecorder) << "Starting" << processName << "with the following arguments:" << args;
         const QString command = processName + QLatin1Char(' ') + args;
         process.start(command);
         if (!process.waitForStarted(1000)) {
-            QString message = QString::fromLatin1("Could not launch %1 with the following arguments: %2\nError:\n%3");
-            message = message.arg(processName).arg(args).arg(process.errorString());
-            QFAIL(qPrintable(message));
-        } else {
-            qCDebug(lcGifRecorder) << "Successfully started" << processName;
+            QString errorMessage = QString::fromLatin1("Could not launch %1 with the following arguments: %2\nError:\n%3");
+            errorMessage = errorMessage.arg(processName).arg(args).arg(process.errorString());
+            return { false, errorMessage };
         }
+
+        qCDebug(lcGifRecorder) << "Successfully started" << processName;
+        return { true, QString() };
     }
 
-    void waitForProcessToFinish(QProcess &process, const QString &processName, int waitDuration)
+    ProcessWaitResult waitForProcessToFinish(QProcess &process, const QString &processName, int waitDuration)
     {
         if (!process.waitForFinished(waitDuration) || process.exitCode() != 0) {
-            QString message = QString::fromLatin1("%1 failed to finish (exit code %2): %3");
-            message = message.arg(processName).arg(process.exitCode()).arg(process.errorString());
-            QFAIL(qPrintable(message));
-        } else {
-            qCDebug(lcGifRecorder) << processName << "finished";
+            QString errorMessage = QString::fromLatin1("\"%1\" failed to finish (exit code %2): %3");
+            errorMessage = errorMessage.arg(processName).arg(process.exitCode()).arg(process.errorString());
+            return { false, errorMessage };
         }
+
+        qCDebug(lcGifRecorder) << processName << "finished";
+        return { true, QString() };
     }
 }
 
@@ -222,7 +229,9 @@ void GifRecorder::start()
     connect(&mEventTimer, SIGNAL(timeout()), mWindow, SLOT(update()));
     mEventTimer.start(100);
 
-    startProcess(mByzanzProcess, byzanzProcessName, args);
+    const ProcessWaitResult result = waitForProcessToStart(mByzanzProcess, byzanzProcessName, args);
+    if (!result.success)
+        QFAIL(qPrintable(result.errorMessage));
 }
 
 void GifRecorder::waitForFinish()
@@ -252,20 +261,31 @@ void GifRecorder::waitForFinish()
         QSignalSpy spy(mWindow, SIGNAL(frameSwapped()));
         QVERIFY(spy.wait());
 
+        // Start ffmpeg and send its output to imagemagick's convert command.
+        // Based on the example in the documentation for QProcess::setStandardOutputProcess().
         QProcess ffmpegProcess;
         QProcess convertProcess;
         ffmpegProcess.setStandardOutputProcess(&convertProcess);
 
         const QString ffmpegProcessName = QStringLiteral("ffmpeg");
         const QString ffmpegArgs = QString::fromLatin1("-i %1 -r 20 -f image2pipe -vcodec ppm -").arg(mByzanzOutputFileName);
-        startProcess(ffmpegProcess, ffmpegProcessName, ffmpegArgs);
+        ProcessWaitResult result = waitForProcessToStart(ffmpegProcess, ffmpegProcessName, ffmpegArgs);
+        if (!result.success)
+            QFAIL(qPrintable(result.errorMessage));
 
         const QString convertProcessName = QStringLiteral("convert");
         const QString convertArgs = QString::fromLatin1("-delay 5 -loop 0 - %1").arg(mGifFileName);
-        startProcess(convertProcess, convertProcessName, convertArgs);
 
-        waitForProcessToFinish(ffmpegProcess, ffmpegProcessName, waitDuration);
-        waitForProcessToFinish(convertProcess, convertProcessName, waitDuration);
+        result = waitForProcessToStart(convertProcess, convertProcessName, convertArgs);
+        if (!result.success)
+            QFAIL(qPrintable(result.errorMessage));
+
+        result = waitForProcessToFinish(ffmpegProcess, ffmpegProcessName, waitDuration);
+        if (!result.success)
+            QFAIL(qPrintable(result.errorMessage));
+        result = waitForProcessToFinish(convertProcess, convertProcessName, waitDuration);
+        if (!result.success)
+            QFAIL(qPrintable(result.errorMessage));
 
         const QString gifsicleProcessName = QStringLiteral("gifsicle");
         const QString verbose = lcGifRecorder().isDebugEnabled() ? QStringLiteral("-V") : QString();
@@ -281,12 +301,15 @@ void GifRecorder::waitForFinish()
         QProcess gifsicleProcess;
         if (lcGifRecorder().isDebugEnabled())
             gifsicleProcess.setProcessChannelMode(QProcess::ForwardedChannels);
-        startProcess(gifsicleProcess, gifsicleProcessName, gifsicleArgs);
-        waitForProcessToFinish(gifsicleProcess, gifsicleProcessName, waitDuration);
-
-        if (QFile::exists(mByzanzOutputFileName)) {
+        result = waitForProcessToStart(gifsicleProcess, gifsicleProcessName, gifsicleArgs);
+        if (!result.success)
+            QFAIL(qPrintable(result.errorMessage));
+        result = waitForProcessToFinish(gifsicleProcess, gifsicleProcessName, waitDuration);
+        if (!result.success)
+            QFAIL(qPrintable(result.errorMessage));
+
+        if (QFile::exists(mByzanzOutputFileName))
             QVERIFY(QFile::remove(mByzanzOutputFileName));
-        }
     }
 }