From a4f2c38f73b4cd5a9c5acfeacb593110b42b0215 Mon Sep 17 00:00:00 2001 From: Mitch Curtis Date: Wed, 20 Feb 2019 15:50:52 +0100 Subject: 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 --- tests/manual/gifs/gifrecorder.cpp | 67 ++++++++++++++++++++++++++------------- 1 file changed, 45 insertions(+), 22 deletions(-) (limited to 'tests/manual') 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)); - } } } -- cgit v1.2.3