Properly fixed comparison of version + added tests.

src/lib/app/profileupdater.cpp

@@ -64,34 +64,35 @@ void ProfileUpdater::updateProfile(const QString &current, const QString &profil
         return;
     }
 
-//    Updater::Version currentVersion = Updater::parseVersionFromString(current);
-    Updater::Version profileVersion = Updater::parseVersionFromString(profile);
+    Updater::Version prof(profile);
 
-    if (profileVersion == Updater::parseVersionFromString("1.0.0")) {
+    if (prof == Updater::Version("1.0.0")) {
         update100();
         return;
     }
 
-    if (profileVersion == Updater::parseVersionFromString("1.1.0")
-            || profileVersion == Updater::parseVersionFromString("1.1.5")
-            || profileVersion == Updater::parseVersionFromString("1.1.8")) {
+    if (prof == Updater::Version("1.1.0")
+            || prof == Updater::Version("1.1.5")
+            || prof == Updater::Version("1.1.8")) {
         update118();
         return;
     }
 
-    if (profileVersion == Updater::parseVersionFromString("1.2.0")) {
+    if (prof == Updater::Version("1.2.0")) {
         update120();
         return;
     }
 
-    if (profileVersion == Updater::parseVersionFromString("1.3.0")
-            || profileVersion == Updater::parseVersionFromString("1.3.1")) {
+    if (prof == Updater::Version("1.3.0")
+            || prof == Updater::Version("1.3.1")) {
         update130();
         return;
     }
 
-    // 1.3.5 - no changes
-    if (profileVersion == Updater::parseVersionFromString("1.3.5")) {
+    // 1.3.5 - 1.4.1 = no changes
+    if (prof == Updater::Version("1.3.5")
+            || prof == Updater::Version("1.4.0")
+            || prof == Updater::Version("1.4.1")) {
         return;
     }
 

src/lib/other/updater.cpp

@@ -27,42 +27,86 @@
 #include <QNetworkRequest>
 #include <QNetworkReply>
 
-Updater::Updater(QupZilla* mainClass, QObject* parent)
-    : QObject(parent)
-    , p_QupZilla(mainClass)
+Updater::Version::Version(const QString &s)
 {
-    QTimer::singleShot(60 * 1000, this, SLOT(start())); // Start checking after 1 minute
-}
+    isValid = false;
 
-Updater::Version Updater::parseVersionFromString(const QString &string)
-{
-    Version ver;
-    ver.isValid = false;
-
-    QStringList v = string.split(QLatin1Char('.'));
+    QStringList v = s.split(QLatin1Char('.'));
     if (v.count() != 3) {
-        return ver;
+        return;
     }
 
     bool ok;
 
-    ver.majorVersion = v.at(0).toInt(&ok);
+    majorVersion = v.at(0).toInt(&ok);
     if (!ok) {
-        return ver;
+        return;
     }
 
-    ver.minorVersion = v.at(1).toInt(&ok);
+    minorVersion = v.at(1).toInt(&ok);
     if (!ok) {
-        return ver;
+        return;
     }
 
-    ver.revisionNumber = v.at(2).toInt(&ok);
+    revisionNumber = v.at(2).toInt(&ok);
     if (!ok) {
-        return ver;
+        return;
+    }
+
+    isValid = majorVersion >= 0 && minorVersion >= 0 && revisionNumber >= 0;
+}
+
+bool Updater::Version::operator <(const Updater::Version &other) const
+{
+    if (this->majorVersion != other.majorVersion) {
+        return this->majorVersion < other.majorVersion;
+    }
+    if (this->minorVersion != other.minorVersion) {
+        return this->minorVersion < other.minorVersion;
+    }
+    if (this->revisionNumber != other.revisionNumber) {
+        return this->revisionNumber < other.revisionNumber;
+    }
+
+    return false;
+}
+
+bool Updater::Version::operator >(const Updater::Version &other) const
+{
+    if (*this == other) {
+        return false;
+    }
+    return !operator<(other);
+}
+
+bool Updater::Version::operator ==(const Updater::Version &other) const
+{
+    return (this->majorVersion == other.majorVersion &&
+            this->minorVersion == other.minorVersion &&
+            this->revisionNumber == other.revisionNumber);
+}
+
+bool Updater::Version::operator >=(const Updater::Version &other) const
+{
+    if (*this == other) {
+        return true;
+    }
+    return *this > other;
+}
+
+bool Updater::Version::operator <=(const Updater::Version &other) const
+{
+    if (*this == other) {
+        return true;
     }
+    return *this < other;
+}
 
-    ver.isValid = true;
-    return ver;
+Updater::Updater(QupZilla* mainClass, QObject* parent)
+    : QObject(parent)
+    , p_QupZilla(mainClass)
+{
+    QTimer::singleShot(60 * 1000, this, SLOT(start())); // Start checking after 1 minute
 }
 
 void Updater::start()
@@ -88,8 +132,8 @@ void Updater::downCompleted(QNetworkReply* reply)
 
     if (html.startsWith(QLatin1String("Version:"))) {
         html.remove(QLatin1String("Version:"));
-        Version current = parseVersionFromString(QupZilla::VERSION);
-        Version updated = parseVersionFromString(html);
+        Version current(QupZilla::VERSION);
+        Version updated(html);
 
         if (current.isValid && updated.isValid && current < updated) {
             mApp->desktopNotifications()->showNotification(QPixmap(":icons/qupzillaupdate.png"), tr("Update available"), tr("New version of QupZilla is ready to download."));

src/lib/other/updater.h

@@ -40,42 +40,16 @@ public:
         int minorVersion;
         int revisionNumber;
 
-        bool operator<(const Version &other) const {
-            if (this->majorVersion < other.majorVersion)
-                return true;
-            if (this->minorVersion < other.minorVersion)
-                return true;
-            if (this->revisionNumber < other.revisionNumber)
-                return true;
+        Version(const QString &s);
 
-            return false;
-        }
+        bool operator<(const Version &other) const;
+        bool operator>(const Version &other) const;
 
-        bool operator>(const Version &other) const {
-            return !operator<(other);
-        }
-
-        bool operator==(const Version &other) const {
-            return (this->majorVersion == other.majorVersion &&
-                    this->minorVersion == other.minorVersion &&
-                    this->revisionNumber == other.revisionNumber);
-        }
-
-        bool operator>=(const Version &other) const {
-            if (*this == other)
-                return true;
-            return *this > other;
-        }
-
-        bool operator<=(const Version &other) const {
-            if (*this == other)
-                return true;
-            return *this < other;
-        }
+        bool operator==(const Version &other) const;
+        bool operator>=(const Version &other) const;
+        bool operator<=(const Version &other) const;
     };
 
-    static Version parseVersionFromString(const QString &string);
-
 private slots:
     void downCompleted(QNetworkReply* reply);
     void start();

tests/autotests/autotests.pro

@@ -50,7 +50,8 @@ HEADERS += \
     formcompletertest.h \
     cookiestest.h \
     downloadstest.h \
-    adblocktest.h
+    adblocktest.h \
+    updatertest.h
 
 SOURCES += \
     qztoolstest.cpp \
@@ -58,4 +59,5 @@ SOURCES += \
     formcompletertest.cpp \
     cookiestest.cpp \
     downloadstest.cpp \
-    adblocktest.cpp
+    adblocktest.cpp \
+    updatertest.cpp

tests/autotests/main.cpp

@@ -20,6 +20,7 @@
 #include "cookiestest.h"
 #include "downloadstest.h"
 #include "adblocktest.h"
+#include "updatertest.h"
 
 #include <QtTest/QtTest>
 
@@ -43,5 +44,8 @@ int main(int argc, char *argv[])
     AdBlockTest adblockTest;
     QTest::qExec(&adblockTest, argc, argv);
 
+    UpdaterTest updaterTest;
+    QTest::qExec(&updaterTest, argc, argv);
+
     return 0;
 }

tests/autotests/updatertest.cpp

@@ -0,0 +1,93 @@
+/* ============================================================
+* QupZilla - WebKit based browser
+* Copyright (C) 2013  David Rosca <nowrep@gmail.com>
+*
+* This program is free software: you can redistribute it and/or modify
+* it under the terms of the GNU General Public License as published by
+* the Free Software Foundation, either version 3 of the License, or
+* (at your option) any later version.
+*
+* This program is distributed in the hope that it will be useful,
+* but WITHOUT ANY WARRANTY; without even the implied warranty of
+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+* GNU General Public License for more details.
+*
+* You should have received a copy of the GNU General Public License
+* along with this program.  If not, see <http://www.gnu.org/licenses/>.
+* ============================================================ */
+#include "updatertest.h"
+#include "updater.h"
+
+#include <QtTest/QtTest>
+
+void UpdaterTest::parseVersionsTest_data()
+{
+    QTest::addColumn<QString>("versionString");
+    QTest::addColumn<bool>("valid");
+    QTest::addColumn<int>("major");
+    QTest::addColumn<int>("minor");
+    QTest::addColumn<int>("revision");
+
+    QTest::newRow("zeros") << "0.0.0" << true << 0 << 0 << 0;
+    QTest::newRow("zero-1") << "0.0.1" << true << 0 << 0 << 1;
+    QTest::newRow("current") << "1.4.1" << true << 1 << 4 << 1;
+    QTest::newRow("next-bugfix") << "1.4.2" << true << 1 << 4 << 2;
+    QTest::newRow("2digits") << "2.5.15" << true << 2 << 5 << 15;
+    QTest::newRow("3digits") << "123.123.333" << true << 123 << 123 << 333;
+    QTest::newRow("negative") << "-1.4.1" << false << 0 << 0 << 0;
+    QTest::newRow("invalid") << "0.0.0-1" << false << 0 << 0 << 0;
+    QTest::newRow("invalid2") << "invalid1text" << false << 0 << 0 << 0;
+}
+
+void UpdaterTest::parseVersionsTest()
+{
+    QFETCH(QString, versionString);
+    QFETCH(bool, valid);
+    QFETCH(int, major);
+    QFETCH(int, minor);
+    QFETCH(int, revision);
+
+    Updater::Version v(versionString);
+
+    QCOMPARE(v.isValid, valid);
+
+    if (valid) {
+        QCOMPARE(v.majorVersion, major);
+        QCOMPARE(v.minorVersion, minor);
+        QCOMPARE(v.revisionNumber, revision);
+    }
+}
+
+void UpdaterTest::compareVersionsTest_data()
+{
+    QTest::addColumn<QString>("version1");
+    QTest::addColumn<QString>("version2");
+    QTest::addColumn<bool>("less");
+    QTest::addColumn<bool>("more");
+    QTest::addColumn<bool>("equal");
+
+    QTest::newRow("test1") << "0.0.1" << "0.0.2" << true << false << false;
+    QTest::newRow("test2") << "0.1.2" << "0.0.2" << false << true << false;
+    QTest::newRow("test3") << "1.0.1" << "0.0.2" << false << true << false;
+    QTest::newRow("test4") << "1.4.1" << "1.4.2" << true << false << false;
+    QTest::newRow("test5") << "1.5.0" << "1.4.2" << false << true << false;
+    QTest::newRow("test6") << "1.5.0" << "1.5.0" << false << false << true;
+    QTest::newRow("test7") << "1.5.1" << "1.4.2" << false << true << false;
+    QTest::newRow("test8") << "1.4.1" << "1.4.2" << true << false << false;
+}
+
+void UpdaterTest::compareVersionsTest()
+{
+    QFETCH(QString, version1);
+    QFETCH(QString, version2);
+    QFETCH(bool, less);
+    QFETCH(bool, more);
+    QFETCH(bool, equal);
+
+    Updater::Version v1(version1);
+    Updater::Version v2(version2);
+
+    QCOMPARE(v1 < v2, less);
+    QCOMPARE(v1 > v2, more);
+    QCOMPARE(v1 == v2, equal);
+}

tests/autotests/updatertest.h

@@ -0,0 +1,36 @@
+/* ============================================================
+* QupZilla - WebKit based browser
+* Copyright (C) 2013  David Rosca <nowrep@gmail.com>
+*
+* This program is free software: you can redistribute it and/or modify
+* it under the terms of the GNU General Public License as published by
+* the Free Software Foundation, either version 3 of the License, or
+* (at your option) any later version.
+*
+* This program is distributed in the hope that it will be useful,
+* but WITHOUT ANY WARRANTY; without even the implied warranty of
+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+* GNU General Public License for more details.
+*
+* You should have received a copy of the GNU General Public License
+* along with this program.  If not, see <http://www.gnu.org/licenses/>.
+* ============================================================ */
+#ifndef UPDATERTEST_H
+#define UPDATERTEST_H
+
+#include <QObject>
+
+class UpdaterTest : public QObject
+{
+    Q_OBJECT
+
+private slots:
+    void parseVersionsTest_data();
+    void parseVersionsTest();
+
+    void compareVersionsTest_data();
+    void compareVersionsTest();
+
+};
+
+#endif // UPDATERTEST_H