Merge pull request #14903 from NissimHadar/21129-improveNitpickUsability

Case 21129: improve nitpick usability

tools/nitpick/README.md

@@ -18,7 +18,7 @@ Nitpick has 5 functions, separated into separate tabs:
 ### Windows
 1.  (First time) download and install Python 3 from https://hifi-qa.s3.amazonaws.com/nitpick/Windows/python-3.7.0-amd64.exe (also located at https://www.python.org/downloads/)
     1. Click the "add python to path" checkbox on the python installer  
-    1. After installation - create an environment variable called PYTHON_PATH and set it to the folder containing the Python executable.
+    1. After installation - add the path to python.exe to the Windows PATH environment variable.
 1.  (First time) download and install AWS CLI from https://hifi-qa.s3.amazonaws.com/nitpick/Windows/AWSCLI64PY3.msi (also available at https://aws.amazon.com/cli/
     1.  Open a new command prompt and run  
         `aws configure`
@@ -31,7 +31,7 @@ Nitpick has 5 functions, separated into separate tabs:
 1.  (First time) Download adb (Android Debug Bridge) from *https://dl.google.com/android/repository/platform-tools-latest-windows.zip*
     1.  Copy the downloaded file to (for example) **C:\adb** and extract in place.  
         Verify you see *adb.exe* in **C:\adb\platform-tools\\**.
-    1.  Create an environment variable named ADB_PATH and set its value to the installation location (e.g. **C:\adb**)
+    1.  After installation - add the path to adb.exe to the Windows PATH environment variable (note that it is in *adb\platform-tools*).
 ### Mac
 1.  (first time) Install brew
     In a terminal:  

tools/nitpick/src/AdbInterface.cpp

@@ -0,0 +1,38 @@
+//
+//  AdbInterface.cpp
+//
+//  Created by Nissim Hadar on Feb 11, 2019.
+//  Copyright 2013 High Fidelity, Inc.
+//
+//  Distributed under the Apache License, Version 2.0.
+//  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
+//
+#include "AdbInterface.h"
+#include <PathUtils.h>
+
+#include <QFile>
+#include <QMessageBox>
+
+QString AdbInterface::getAdbCommand() {
+#ifdef Q_OS_WIN
+    if (_adbCommand.isNull()) {
+        QString adbPath = PathUtils::getPathToExecutable("adb.exe");
+        if (!adbPath.isNull()) {
+            _adbCommand = adbPath + _adbExe;
+        } else {
+            QMessageBox::critical(0, "python.exe not found",
+                "Please verify that pyton.exe is in the PATH");
+            exit(-1);
+        }
+    }
+#elif defined Q_OS_MAC
+    _adbCommand = "/usr/local/bin/adb";
+    if (!QFile::exists(_adbCommand)) {
+        QMessageBox::critical(0, "adb not found",
+            "adb not found at " + _adbCommand);
+        exit(-1);
+    }
+#endif
+
+    return _adbCommand;
+}

tools/nitpick/src/AdbInterface.h

@@ -0,0 +1,30 @@
+//
+//  AdbInterface.h
+//
+//  Created by Nissim Hadar on Feb 11, 2019.
+//  Copyright 2013 High Fidelity, Inc.
+//
+//  Distributed under the Apache License, Version 2.0.
+//  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
+//
+#ifndef hifi_AdbInterface_h
+#define hifi_AdbInterface_h
+
+#include <QString>
+
+class AdbInterface {
+public:
+    QString getAdbCommand();
+
+private:
+#ifdef Q_OS_WIN
+    const QString _adbExe{ "adb.exe" };
+#else
+    // Both Mac and Linux use "python"
+    const QString _adbExe{ "adb" };
+#endif
+
+    QString _adbCommand;
+};
+
+#endif // hifi_AdbInterface_h

tools/nitpick/src/Nitpick.cpp

@@ -40,7 +40,7 @@ Nitpick::Nitpick(QWidget* parent) : QMainWindow(parent) {
  
     _ui.plainTextEdit->setReadOnly(true);
 
-   setWindowTitle("Nitpick - v2.1.0");
+   setWindowTitle("Nitpick - v2.1.1");
 }
 
 Nitpick::~Nitpick() {

tools/nitpick/src/Nitpick.h

@@ -21,8 +21,6 @@
 #include "TestRunnerDesktop.h"
 #include "TestRunnerMobile.h"
 
-#include "AWSInterface.h"
-
 class Nitpick : public QMainWindow {
     Q_OBJECT
 
@@ -112,8 +110,6 @@ private:
     TestRunnerDesktop* _testRunnerDesktop{ nullptr };
     TestRunnerMobile* _testRunnerMobile{ nullptr };
 
-    AWSInterface _awsInterface;
-
     std::vector<Downloader*> _downloaders;
 
     // local storage for parameters - folder to store downloaded files in, and a list of their names

tools/nitpick/src/PathUtils.cpp

@@ -0,0 +1,31 @@
+//
+//  PathUtils.h
+//
+//  Created by Nissim Hadar on 11 Feb 2019.
+//  Copyright 2013 High Fidelity, Inc.
+//
+//  Distributed under the Apache License, Version 2.0.
+//  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
+//
+#include "PathUtils.h"
+
+#include <QFile>
+#include <QProcess>
+#include <QStringList>
+
+QString PathUtils::getPathToExecutable(const QString& executableName) {
+    QString path = QProcessEnvironment::systemEnvironment().value("PATH");
+
+    QStringList pathLocations = path.replace('\\', '/').split(';');
+
+    foreach (QString pathLocation, pathLocations) {
+        if (pathLocation[pathLocation.length() - 1] != '/') {
+            pathLocation += '/';
+        }
+        if (QFile::exists(pathLocation + executableName)) {
+            return pathLocation;
+        }
+    }
+
+    return QString();
+}

tools/nitpick/src/PathUtils.h

@@ -0,0 +1,20 @@
+//
+//  PathUtils.h
+//
+//  Created by Nissim Hadar on 11 Feb 2019.
+//  Copyright 2013 High Fidelity, Inc.
+//
+//  Distributed under the Apache License, Version 2.0.
+//  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
+//
+#ifndef hifi_PathUtils_h
+#define hifi_PathUtils_h
+
+#include <QString>
+
+class PathUtils {
+public:
+    static QString getPathToExecutable(const QString& executableName);
+};
+
+#endif

tools/nitpick/src/PythonInterface.cpp

@@ -8,36 +8,31 @@
 //  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
 //
 #include "PythonInterface.h"
+#include <PathUtils.h>
 
 #include <QFile>
 #include <QMessageBox>
-#include <QProcess>
 
-PythonInterface::PythonInterface() {
+QString PythonInterface::getPythonCommand() {
 #ifdef Q_OS_WIN
-    if (QProcessEnvironment::systemEnvironment().contains("PYTHON_PATH")) {
-        QString pythonPath = QProcessEnvironment::systemEnvironment().value("PYTHON_PATH");
-        if (!QFile::exists(pythonPath + "/" + _pythonExe)) {
-            QMessageBox::critical(0, _pythonExe, QString("Python executable not found in ") + pythonPath);
+    if (_pythonCommand.isNull()) {
+        QString pythonPath = PathUtils::getPathToExecutable("python.exe");
+        if (!pythonPath.isNull()) {
+            _pythonCommand = pythonPath + _pythonExe;
+        } else {
+            QMessageBox::critical(0, "python.exe not found",
+                "Please verify that pyton.exe is in the PATH");
             exit(-1);
         }
-        
-        _pythonCommand = pythonPath + "/" + _pythonExe;
-    } else {
-        QMessageBox::critical(0, "PYTHON_PATH not defined",
-                              "Please set PYTHON_PATH to directory containing the Python executable");
-        exit(-1);
     }
 #elif defined Q_OS_MAC
     _pythonCommand = "/usr/local/bin/python3";
     if (!QFile::exists(_pythonCommand)) {
         QMessageBox::critical(0, "python not found",
-                              "python3 not found at " + _pythonCommand);
+            "python3 not found at " + _pythonCommand);
         exit(-1);
     }
 #endif
-}
 
-QString PythonInterface::getPythonCommand() {
     return _pythonCommand;
 }

tools/nitpick/src/PythonInterface.h

@@ -14,8 +14,6 @@
 
 class PythonInterface {
 public:
-    PythonInterface();
-
     QString getPythonCommand();
 
 private:

tools/nitpick/src/Test.cpp

@@ -24,7 +24,7 @@ extern Nitpick* nitpick;
 
 #include <math.h>
 
-Test::Test(QProgressBar* progressBar, QCheckBox* checkBoxInteractiveMode) {
+Test::Test(QProgressBar* progressBar, QCheckBox* checkBoxInteractiveMode) : _awsInterface(NULL) {
     _progressBar = progressBar;
     _checkBoxInteractiveMode = checkBoxInteractiveMode;
 
@@ -966,11 +966,15 @@ void Test::createTestRailTestCases() {
         return;
     }
 
+    if (!_testRailInterface) {
+        _testRailInterface = new TestRailInterface;
+    }
+
     if (_testRailCreateMode == PYTHON) {
-        _testRailInterface.createTestSuitePython(_testDirectory, outputDirectory, nitpick->getSelectedUser(),
+        _testRailInterface->createTestSuitePython(_testDirectory, outputDirectory, nitpick->getSelectedUser(),
                                               nitpick->getSelectedBranch());
     } else {
-        _testRailInterface.createTestSuiteXML(_testDirectory, outputDirectory, nitpick->getSelectedUser(),
+        _testRailInterface->createTestSuiteXML(_testDirectory, outputDirectory, nitpick->getSelectedUser(),
                                            nitpick->getSelectedBranch());
     }
 }
@@ -983,7 +987,12 @@ void Test::createTestRailRun() {
         return;
     }
 
-    _testRailInterface.createTestRailRun(outputDirectory);
+
+    if (!_testRailInterface) {
+        _testRailInterface = new TestRailInterface;
+    }
+
+    _testRailInterface->createTestRailRun(outputDirectory);
 }
 
 void Test::updateTestRailRunResult() {
@@ -999,7 +1008,12 @@ void Test::updateTestRailRunResult() {
         return;
     }
 
-    _testRailInterface.updateTestRailRunResults(testResults, tempDirectory);
+
+    if (!_testRailInterface) {
+        _testRailInterface = new TestRailInterface;
+    }
+
+    _testRailInterface->updateTestRailRunResults(testResults, tempDirectory);
 }
 
 QStringList Test::createListOfAll_imagesInDirectory(const QString& imageFormat, const QString& pathToImageDirectory) {
@@ -1088,5 +1102,9 @@ void Test::createWebPage(QCheckBox* updateAWSCheckBox, QLineEdit* urlLineEdit) {
         return;
     }
 
-    _awsInterface.createWebPageFromResults(testResults, workingDirectory, updateAWSCheckBox, urlLineEdit);
+    if (!_awsInterface) {
+        _awsInterface = new AWSInterface;
+    }
+
+    _awsInterface->createWebPageFromResults(testResults, workingDirectory, updateAWSCheckBox, urlLineEdit);
 }

tools/nitpick/src/Test.h

@@ -159,10 +159,10 @@ private:
 
     bool _exitWhenComplete{ false };
 
-    TestRailInterface _testRailInterface;
+    TestRailInterface* _testRailInterface;
     TestRailCreateMode _testRailCreateMode { PYTHON };
 
-    AWSInterface _awsInterface;
+    AWSInterface* _awsInterface;
 };
 
 #endif // hifi_test_h

tools/nitpick/src/TestRailInterface.cpp

@@ -20,7 +20,7 @@
 #include <QMessageBox>
 #include <QTextStream>
 
-TestRailInterface::TestRailInterface() {
+TestRailInterface::TestRailInterface() : _pythonInterface(NULL) {
     _testRailTestCasesSelectorWindow.setURL("https://highfidelity.testrail.net");
     _testRailTestCasesSelectorWindow.setUser("@highfidelity.io");
 

tools/nitpick/src/TestRunnerMobile.cpp

@@ -30,7 +30,7 @@ TestRunnerMobile::TestRunnerMobile(
     QLabel* statusLabel,
 
     QObject* parent
-) : QObject(parent) 
+) : QObject(parent), _adbInterface(NULL)
 {
     _workingFolderLabel = workingFolderLabel;
     _connectDeviceButton = connectDeviceButton;
@@ -47,30 +47,6 @@ TestRunnerMobile::TestRunnerMobile(
     folderLineEdit->setText("/sdcard/DCIM/TEST");
 
     modelNames["SM_G955U1"] = "Samsung S8+ unlocked";
-
-    // Find ADB (Android Debugging Bridge)
-#ifdef Q_OS_WIN
-    if (QProcessEnvironment::systemEnvironment().contains("ADB_PATH")) {
-        QString adbExePath = QProcessEnvironment::systemEnvironment().value("ADB_PATH") + "/platform-tools";
-        if (!QFile::exists(adbExePath + "/" + _adbExe)) {
-            QMessageBox::critical(0, _adbExe, QString("ADB executable not found in ") + adbExePath);
-            exit(-1);
-        }
-
-        _adbCommand = adbExePath + "/" + _adbExe;
-    } else {
-        QMessageBox::critical(0, "ADB_PATH not defined",
-            "Please set ADB_PATH to directory containing the `adb` executable");
-        exit(-1);
-    }
-#elif defined Q_OS_MAC
-    _adbCommand = "/usr/local/bin/adb";
-    if (!QFile::exists(_adbCommand)) {
-        QMessageBox::critical(0, "adb not found",
-            "python3 not found at " + _adbCommand);
-        exit(-1);
-    }
-#endif
 }
 
 TestRunnerMobile::~TestRunnerMobile() {
@@ -84,8 +60,12 @@ void TestRunnerMobile::setWorkingFolderAndEnableControls() {
 
 void TestRunnerMobile::connectDevice() {
 #if defined Q_OS_WIN || defined Q_OS_MAC
+    if (!_adbInterface) {
+        _adbInterface = new AdbInterface();
+    }
+    
     QString devicesFullFilename{ _workingFolder + "/devices.txt" };
-    QString command = _adbCommand + " devices -l > " + devicesFullFilename;
+    QString command = _adbInterface->getAdbCommand() + " devices -l > " + devicesFullFilename;
     system(command.toStdString().c_str());
 
     if (!QFile::exists(devicesFullFilename)) {
@@ -100,12 +80,13 @@ void TestRunnerMobile::connectDevice() {
     QString line2 = devicesFile.readLine();
 
     const QString DEVICE{ "device" };
-    if (line2.contains(DEVICE)) {
-        // Make sure only 1 device
+    if (line2.contains("unauthorized")) {
+        QMessageBox::critical(0, "Unauthorized device detected", "Please allow USB debugging on device");
+    } else if (line2.contains(DEVICE)) {
+            // Make sure only 1 device
         QString line3 = devicesFile.readLine();
         if (line3.contains(DEVICE)) {
             QMessageBox::critical(0, "Too many devices detected", "Tests will run only if a single device is attached");
-
         } else {
             // Line looks like this: 988a1b47335239434b     device product:dream2qlteue model:SM_G955U1 device:dream2qlteue transport_id:2
             QStringList tokens = line2.split(QRegExp("[\r\n\t ]+"));
@@ -169,8 +150,12 @@ void TestRunnerMobile::downloadComplete() {
 
 void TestRunnerMobile::installAPK() {
 #if defined Q_OS_WIN || defined Q_OS_MAC
+    if (!_adbInterface) {
+        _adbInterface = new AdbInterface();
+    }
+
     _statusLabel->setText("Installing");
-    QString command = _adbCommand + " install -r -d " + _workingFolder + "/" + _installerFilename + " >" + _workingFolder  + "/installOutput.txt";
+    QString command = _adbInterface->getAdbCommand() + " install -r -d " + _workingFolder + "/" + _installerFilename + " >" + _workingFolder  + "/installOutput.txt";
     system(command.toStdString().c_str());
     _statusLabel->setText("Installation complete");
     _runInterfacePushbutton->setEnabled(true);
@@ -179,8 +164,12 @@ void TestRunnerMobile::installAPK() {
 
 void TestRunnerMobile::runInterface() {
 #if defined Q_OS_WIN || defined Q_OS_MAC
+    if (!_adbInterface) {
+        _adbInterface = new AdbInterface();
+    }
+
     _statusLabel->setText("Starting Interface");
-    QString command = _adbCommand + " shell monkey -p io.highfidelity.hifiinterface -v 1";
+    QString command = _adbInterface->getAdbCommand() + " shell monkey -p io.highfidelity.hifiinterface -v 1";
     system(command.toStdString().c_str());
     _statusLabel->setText("Interface started");
 #endif
@@ -188,8 +177,12 @@ void TestRunnerMobile::runInterface() {
 
 void TestRunnerMobile::pullFolder() {
 #if defined Q_OS_WIN || defined Q_OS_MAC
+    if (!_adbInterface) {
+        _adbInterface = new AdbInterface();
+    }
+
     _statusLabel->setText("Pulling folder");
-    QString command = _adbCommand + " pull " + _folderLineEdit->text() + " " + _workingFolder + _installerFilename;
+    QString command = _adbInterface->getAdbCommand() + " pull " + _folderLineEdit->text() + " " + _workingFolder + _installerFilename;
     system(command.toStdString().c_str());
     _statusLabel->setText("Pull complete");
 #endif

tools/nitpick/src/TestRunnerMobile.h

@@ -17,6 +17,7 @@
 #include <QPushButton>
 
 #include "TestRunner.h"
+#include "AdbInterface.h"
 
 class TestRunnerMobile : public QObject, public TestRunner {
     Q_OBJECT
@@ -70,5 +71,7 @@ private:
     QString _adbCommand;
 
     std::map<QString, QString> modelNames;
+
+    AdbInterface* _adbInterface;
 };
 #endif

tools/nitpick/ui/Nitpick.ui

@@ -686,8 +686,8 @@
      <widget class="QLabel" name="workingFolderLabel_4">
       <property name="geometry">
        <rect>
-        <x>290</x>
-        <y>20</y>
+        <x>300</x>
+        <y>60</y>
         <width>41</width>
         <height>31</height>
        </rect>
@@ -699,8 +699,8 @@
      <widget class="QLabel" name="statusLabelOnMobile">
       <property name="geometry">
        <rect>
-        <x>340</x>
-        <y>20</y>
+        <x>350</x>
+        <y>60</y>
         <width>271</width>
         <height>31</height>
        </rect>