From c8648c70161d00e5bb0e2e150bfd467295834046 Mon Sep 17 00:00:00 2001 From: NissimHadar Date: Fri, 8 Mar 2019 08:53:48 -0800 Subject: [PATCH 1/8] Added worst tile value to test. --- tools/nitpick/src/ImageComparer.cpp | 11 +++++++++++ tools/nitpick/src/ImageComparer.h | 2 ++ tools/nitpick/src/MismatchWindow.cpp | 2 +- tools/nitpick/src/Nitpick.cpp | 2 +- tools/nitpick/src/TestCreator.cpp | 14 +++++++++----- tools/nitpick/src/TestCreator.h | 3 ++- tools/nitpick/src/common.h | 10 +++++++--- tools/nitpick/ui/MismatchWindow.ui | 6 +++--- 8 files changed, 36 insertions(+), 14 deletions(-) diff --git a/tools/nitpick/src/ImageComparer.cpp b/tools/nitpick/src/ImageComparer.cpp index 7e3e6eaf63..b35c5d639d 100644 --- a/tools/nitpick/src/ImageComparer.cpp +++ b/tools/nitpick/src/ImageComparer.cpp @@ -43,6 +43,8 @@ void ImageComparer::compareImages(const QImage& resultImage, const QImage& expec int windowCounter{ 0 }; double ssim{ 0.0 }; + double worstTileValue{ 1.0 }; + double min { 1.0 }; double max { -1.0 }; @@ -108,6 +110,10 @@ void ImageComparer::compareImages(const QImage& resultImage, const QImage& expec if (value < min) min = value; if (value > max) max = value; + if (value < worstTileValue) { + worstTileValue = value; + } + ++windowCounter; y += WIN_SIZE; @@ -122,12 +128,17 @@ void ImageComparer::compareImages(const QImage& resultImage, const QImage& expec _ssimResults.min = min; _ssimResults.max = max; _ssimResults.ssim = ssim / windowCounter; + _ssimResults.worstTileValue = worstTileValue; }; double ImageComparer::getSSIMValue() { return _ssimResults.ssim; } +double ImageComparer::getWorstTileValue() { + return _ssimResults.worstTileValue; +} + SSIMResults ImageComparer::getSSIMResults() { return _ssimResults; } diff --git a/tools/nitpick/src/ImageComparer.h b/tools/nitpick/src/ImageComparer.h index fc14dab94d..a18e432a01 100644 --- a/tools/nitpick/src/ImageComparer.h +++ b/tools/nitpick/src/ImageComparer.h @@ -18,7 +18,9 @@ class ImageComparer { public: void compareImages(const QImage& resultImage, const QImage& expectedImage); + double getSSIMValue(); + double getWorstTileValue(); SSIMResults getSSIMResults(); diff --git a/tools/nitpick/src/MismatchWindow.cpp b/tools/nitpick/src/MismatchWindow.cpp index fd5df0dd4e..2a7aca9f2e 100644 --- a/tools/nitpick/src/MismatchWindow.cpp +++ b/tools/nitpick/src/MismatchWindow.cpp @@ -61,7 +61,7 @@ QPixmap MismatchWindow::computeDiffPixmap(const QImage& expectedImage, const QIm } void MismatchWindow::setTestResult(const TestResult& testResult) { - errorLabel->setText("Similarity: " + QString::number(testResult._error)); + errorLabel->setText("Similarity: " + QString::number(testResult._errorGlobal) + " (worst tile: " + QString::number(testResult._errorLocal) + ")"); imagePath->setText("Path to test: " + testResult._pathname); diff --git a/tools/nitpick/src/Nitpick.cpp b/tools/nitpick/src/Nitpick.cpp index cf50774617..e72de9d1ad 100644 --- a/tools/nitpick/src/Nitpick.cpp +++ b/tools/nitpick/src/Nitpick.cpp @@ -38,7 +38,7 @@ Nitpick::Nitpick(QWidget* parent) : QMainWindow(parent) { _ui.plainTextEdit->setReadOnly(true); - setWindowTitle("Nitpick - v3.1.2"); + setWindowTitle("Nitpick - v3.1.3"); clientProfiles << "VR-High" << "Desktop-High" << "Desktop-Low" << "Mobile-Touch" << "VR-Standalone"; _ui.clientProfileComboBox->insertItems(0, clientProfiles); diff --git a/tools/nitpick/src/TestCreator.cpp b/tools/nitpick/src/TestCreator.cpp index 089e84904a..a79a2b3b09 100644 --- a/tools/nitpick/src/TestCreator.cpp +++ b/tools/nitpick/src/TestCreator.cpp @@ -83,6 +83,7 @@ int TestCreator::compareImageLists() { QImage expectedImage(_expectedImagesFullFilenames[i]); double similarityIndex; // in [-1.0 .. 1.0], where 1.0 means images are identical + double worstTileValue; // in [-1.0 .. 1.0], where 1.0 means images are identical bool isInteractiveMode = (!_isRunningFromCommandLine && _checkBoxInteractiveMode->isChecked() && !_isRunningInAutomaticTestRun); @@ -93,10 +94,12 @@ int TestCreator::compareImageLists() { } else { _imageComparer.compareImages(resultImage, expectedImage); similarityIndex = _imageComparer.getSSIMValue(); + worstTileValue = _imageComparer.getWorstTileValue(); } TestResult testResult = TestResult{ - (float)similarityIndex, + similarityIndex, + worstTileValue, _expectedImagesFullFilenames[i].left(_expectedImagesFullFilenames[i].lastIndexOf("/") + 1), // path to the test (including trailing /) QFileInfo(_expectedImagesFullFilenames[i].toStdString().c_str()).fileName(), // filename of expected image QFileInfo(_resultImagesFullFilenames[i].toStdString().c_str()).fileName(), // filename of result image @@ -105,10 +108,9 @@ int TestCreator::compareImageLists() { _mismatchWindow.setTestResult(testResult); - if (similarityIndex < THRESHOLD) { - ++numberOfFailures; - + if (similarityIndex < THRESHOLD_GLOBAL || worstTileValue < THRESHOLD_LOCAL) { if (!isInteractiveMode) { + ++numberOfFailures; appendTestResultsToFile(testResult, _mismatchWindow.getComparisonImage(), _mismatchWindow.getSSIMResultsImage(testResult._ssimResults), true); } else { _mismatchWindow.exec(); @@ -117,6 +119,7 @@ int TestCreator::compareImageLists() { case USER_RESPONSE_PASS: break; case USE_RESPONSE_FAIL: + ++numberOfFailures; appendTestResultsToFile(testResult, _mismatchWindow.getComparisonImage(), _mismatchWindow.getSSIMResultsImage(testResult._ssimResults), true); break; case USER_RESPONSE_ABORT: @@ -198,7 +201,8 @@ void TestCreator::appendTestResultsToFile(const TestResult& testResult, const QP stream << "TestCreator in folder " << testResult._pathname.left(testResult._pathname.length() - 1) << endl; // remove trailing '/' stream << "Expected image was " << testResult._expectedImageFilename << endl; stream << "Actual image was " << testResult._actualImageFilename << endl; - stream << "Similarity index was " << testResult._error << endl; + stream << "Similarity index was " << testResult._errorGlobal << endl; + stream << "Worst tile was " << testResult._errorLocal << endl; descriptionFile.close(); diff --git a/tools/nitpick/src/TestCreator.h b/tools/nitpick/src/TestCreator.h index 7cd38b42d4..6491d6fe6c 100644 --- a/tools/nitpick/src/TestCreator.h +++ b/tools/nitpick/src/TestCreator.h @@ -121,7 +121,8 @@ private: const QString TEST_RESULTS_FOLDER { "TestResults" }; const QString TEST_RESULTS_FILENAME { "TestResults.txt" }; - const double THRESHOLD{ 0.9999 }; + const double THRESHOLD_GLOBAL{ 0.9999 }; + const double THRESHOLD_LOCAL { 0.7770 }; QDir _imageDirectory; diff --git a/tools/nitpick/src/common.h b/tools/nitpick/src/common.h index eb228ff2b3..17090c46db 100644 --- a/tools/nitpick/src/common.h +++ b/tools/nitpick/src/common.h @@ -18,7 +18,9 @@ public: int width; int height; std::vector results; + double ssim; + double worstTileValue; // Used for scaling double min; @@ -27,15 +29,17 @@ public: class TestResult { public: - TestResult(float error, const QString& pathname, const QString& expectedImageFilename, const QString& actualImageFilename, const SSIMResults& ssimResults) : - _error(error), + TestResult(double errorGlobal, double errorLocal, const QString& pathname, const QString& expectedImageFilename, const QString& actualImageFilename, const SSIMResults& ssimResults) : + _errorGlobal(errorGlobal), + _errorLocal(errorLocal), _pathname(pathname), _expectedImageFilename(expectedImageFilename), _actualImageFilename(actualImageFilename), _ssimResults(ssimResults) {} - double _error; + double _errorGlobal; + double _errorLocal; QString _pathname; QString _expectedImageFilename; diff --git a/tools/nitpick/ui/MismatchWindow.ui b/tools/nitpick/ui/MismatchWindow.ui index 8a174989d4..fa3e21957f 100644 --- a/tools/nitpick/ui/MismatchWindow.ui +++ b/tools/nitpick/ui/MismatchWindow.ui @@ -45,7 +45,7 @@ - 540 + 900 480 800 450 @@ -78,7 +78,7 @@ 60 630 - 480 + 540 28 @@ -145,7 +145,7 @@ - Abort current test + Abort evaluation From 20e1753605afc1d6dcafd8e1aa306f4bf663c869 Mon Sep 17 00:00:00 2001 From: NissimHadar Date: Fri, 8 Mar 2019 09:22:52 -0800 Subject: [PATCH 2/8] Reduced threshold a wee bit after testing on laptop. --- tools/nitpick/src/TestCreator.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/nitpick/src/TestCreator.h b/tools/nitpick/src/TestCreator.h index 6491d6fe6c..c2e7ba14f2 100644 --- a/tools/nitpick/src/TestCreator.h +++ b/tools/nitpick/src/TestCreator.h @@ -121,7 +121,7 @@ private: const QString TEST_RESULTS_FOLDER { "TestResults" }; const QString TEST_RESULTS_FILENAME { "TestResults.txt" }; - const double THRESHOLD_GLOBAL{ 0.9999 }; + const double THRESHOLD_GLOBAL{ 0.9998 }; const double THRESHOLD_LOCAL { 0.7770 }; QDir _imageDirectory; From 19c7c26c6369b5df000f68e794c2e5f5834427b6 Mon Sep 17 00:00:00 2001 From: NissimHadar Date: Fri, 8 Mar 2019 10:04:23 -0800 Subject: [PATCH 3/8] gcc / Mac compilation error. --- tools/nitpick/src/TestCreator.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/nitpick/src/TestCreator.cpp b/tools/nitpick/src/TestCreator.cpp index a79a2b3b09..f87134ce5b 100644 --- a/tools/nitpick/src/TestCreator.cpp +++ b/tools/nitpick/src/TestCreator.cpp @@ -91,6 +91,7 @@ int TestCreator::compareImageLists() { if (isInteractiveMode && (resultImage.width() != expectedImage.width() || resultImage.height() != expectedImage.height())) { QMessageBox::critical(0, "Internal error: " + QString(__FILE__) + ":" + QString::number(__LINE__), "Images are not the same size"); similarityIndex = -100.0; + worstTileValue = 0.0; } else { _imageComparer.compareImages(resultImage, expectedImage); similarityIndex = _imageComparer.getSSIMValue(); From 1e5837f25f5a5a1bffdf36da761040627ce2ceec Mon Sep 17 00:00:00 2001 From: NissimHadar Date: Fri, 8 Mar 2019 11:10:54 -0800 Subject: [PATCH 4/8] Enable Android buttons as needed. --- tools/nitpick/src/TestRunnerMobile.cpp | 3 +-- tools/nitpick/ui/Nitpick.ui | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/tools/nitpick/src/TestRunnerMobile.cpp b/tools/nitpick/src/TestRunnerMobile.cpp index 4d0d18ef3d..d7800f35b4 100644 --- a/tools/nitpick/src/TestRunnerMobile.cpp +++ b/tools/nitpick/src/TestRunnerMobile.cpp @@ -60,6 +60,7 @@ void TestRunnerMobile::setWorkingFolderAndEnableControls() { setWorkingFolder(_workingFolderLabel); _connectDeviceButton->setEnabled(true); + _downloadAPKPushbutton->setEnabled(true); } void TestRunnerMobile::connectDevice() { @@ -154,8 +155,6 @@ void TestRunnerMobile::downloadComplete() { } else { _statusLabel->setText("Installer download complete"); } - - _installAPKPushbutton->setEnabled(true); } void TestRunnerMobile::installAPK() { diff --git a/tools/nitpick/ui/Nitpick.ui b/tools/nitpick/ui/Nitpick.ui index a0f368863d..c85311d86b 100644 --- a/tools/nitpick/ui/Nitpick.ui +++ b/tools/nitpick/ui/Nitpick.ui @@ -46,7 +46,7 @@ - 5 + 0 From dbdf5fdd1f73e1799a9ecd3351ac6dc774ff3ceb Mon Sep 17 00:00:00 2001 From: NissimHadar Date: Fri, 8 Mar 2019 13:53:07 -0800 Subject: [PATCH 5/8] Decreased threshold after additional testing. --- tools/nitpick/src/TestCreator.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/nitpick/src/TestCreator.h b/tools/nitpick/src/TestCreator.h index c2e7ba14f2..f2bd520574 100644 --- a/tools/nitpick/src/TestCreator.h +++ b/tools/nitpick/src/TestCreator.h @@ -122,7 +122,7 @@ private: const QString TEST_RESULTS_FILENAME { "TestResults.txt" }; const double THRESHOLD_GLOBAL{ 0.9998 }; - const double THRESHOLD_LOCAL { 0.7770 }; + const double THRESHOLD_LOCAL { 0.7500 }; QDir _imageDirectory; From 687745dc7e3f3aa44a69db36066840ff0b11ad8f Mon Sep 17 00:00:00 2001 From: NissimHadar Date: Fri, 8 Mar 2019 17:17:10 -0800 Subject: [PATCH 6/8] Remove old recursive scripts that are empty. Don't add unneeded delays. --- tools/nitpick/src/TestCreator.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tools/nitpick/src/TestCreator.cpp b/tools/nitpick/src/TestCreator.cpp index f87134ce5b..587490bb64 100644 --- a/tools/nitpick/src/TestCreator.cpp +++ b/tools/nitpick/src/TestCreator.cpp @@ -824,6 +824,10 @@ void TestCreator::createRecursiveScript(const QString& directory, bool interacti // If 'directories' is empty, this means that this recursive script has no tests to call, so it is redundant if (directories.length() == 0) { + QString testRecursivePathname = directory + "/" + TEST_RECURSIVE_FILENAME; + if (QFile::exists(testRecursivePathname)) { + QFile::remove(testRecursivePathname); + } return; } @@ -856,10 +860,7 @@ void TestCreator::createRecursiveScript(const QString& directory, bool interacti textStream << " nitpick = createNitpick(Script.resolvePath(\".\"));" << endl; textStream << " testsRootPath = nitpick.getTestsRootPath();" << endl << endl; textStream << " nitpick.enableRecursive();" << endl; - textStream << " nitpick.enableAuto();" << endl << endl; - textStream << " if (typeof Test !== 'undefined') {" << endl; - textStream << " Test.wait(10000);" << endl; - textStream << " }" << endl; + textStream << " nitpick.enableAuto();" << endl; textStream << "} else {" << endl; textStream << " depth++" << endl; textStream << "}" << endl << endl; From 7419f9899e4af5b753966ea30b259359698d1d0a Mon Sep 17 00:00:00 2001 From: NissimHadar Date: Sat, 9 Mar 2019 11:54:04 -0800 Subject: [PATCH 7/8] Modified thresholds to reduce false positives. --- tools/nitpick/src/TestCreator.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/nitpick/src/TestCreator.h b/tools/nitpick/src/TestCreator.h index f2bd520574..b4ce56a7d5 100644 --- a/tools/nitpick/src/TestCreator.h +++ b/tools/nitpick/src/TestCreator.h @@ -121,8 +121,8 @@ private: const QString TEST_RESULTS_FOLDER { "TestResults" }; const QString TEST_RESULTS_FILENAME { "TestResults.txt" }; - const double THRESHOLD_GLOBAL{ 0.9998 }; - const double THRESHOLD_LOCAL { 0.7500 }; + const double THRESHOLD_GLOBAL{ 0.9995 }; + const double THRESHOLD_LOCAL { 0.6 }; QDir _imageDirectory; From d4b77d15cc4a84ea2eea91cd3eb13ea7da9b5ac8 Mon Sep 17 00:00:00 2001 From: NissimHadar Date: Mon, 11 Mar 2019 17:11:09 -0700 Subject: [PATCH 8/8] Use correct snapshots folder. Fix bug in APK installer. --- tools/nitpick/src/TestRunnerMobile.cpp | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/tools/nitpick/src/TestRunnerMobile.cpp b/tools/nitpick/src/TestRunnerMobile.cpp index d7800f35b4..62630cc7b3 100644 --- a/tools/nitpick/src/TestRunnerMobile.cpp +++ b/tools/nitpick/src/TestRunnerMobile.cpp @@ -43,7 +43,7 @@ TestRunnerMobile::TestRunnerMobile( _installAPKPushbutton = installAPKPushbutton; _runInterfacePushbutton = runInterfacePushbutton; - folderLineEdit->setText("/sdcard/DCIM/TEST"); + folderLineEdit->setText("/sdcard/snapshots"); modelNames["SM_G955U1"] = "Samsung S8+ unlocked"; modelNames["SM_N960U1"] = "Samsung Note 9 unlocked"; @@ -163,22 +163,16 @@ void TestRunnerMobile::installAPK() { _adbInterface = new AdbInterface(); } - if (_installerFilename.isNull()) { - QString installerPathname = QFileDialog::getOpenFileName(nullptr, "Please select the APK", _workingFolder, - "Available APKs (*.apk)" - ); + QString installerPathname = QFileDialog::getOpenFileName(nullptr, "Please select the APK", _workingFolder, + "Available APKs (*.apk)" + ); - if (installerPathname.isNull()) { - return; - } - - // Remove the path - QStringList parts = installerPathname.split('/'); - _installerFilename = parts[parts.length() - 1]; + if (installerPathname.isNull()) { + return; } _statusLabel->setText("Installing"); - QString command = _adbInterface->getAdbCommand() + " install -r -d " + _workingFolder + "/" + _installerFilename + " >" + _workingFolder + "/installOutput.txt"; + QString command = _adbInterface->getAdbCommand() + " install -r -d " + installerPathname + " >" + _workingFolder + "/installOutput.txt"; appendLog(command); system(command.toStdString().c_str()); _statusLabel->setText("Installation complete");