From 9934537e19c7ce6f9bf926cc8abba023297b0a40 Mon Sep 17 00:00:00 2001 From: Rachel Powers <508861+Ryex@users.noreply.github.com> Date: Thu, 19 Jan 2023 09:46:35 +0200 Subject: feat: add debug printing for Version Signed-off-by: Rachel Powers <508861+Ryex@users.noreply.github.com> --- tests/CMakeLists.txt | 3 +++ tests/Version_test.cpp | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 9f84a9a7..0f716a75 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -53,3 +53,6 @@ ecm_add_test(Packwiz_test.cpp LINK_LIBRARIES Launcher_logic Qt${QT_VERSION_MAJOR ecm_add_test(Index_test.cpp LINK_LIBRARIES Launcher_logic Qt${QT_VERSION_MAJOR}::Test TEST_NAME Index) + +ecm_add_test(Version_test.cpp LINK_LIBRARIES Launcher_logic Qt${QT_VERSION_MAJOR}::Test + TEST_NAME Version) diff --git a/tests/Version_test.cpp b/tests/Version_test.cpp index 734528b7..6836b6fa 100644 --- a/tests/Version_test.cpp +++ b/tests/Version_test.cpp @@ -15,7 +15,6 @@ #include -#include #include class ModUtilsTest : public QObject @@ -74,6 +73,8 @@ private slots: const auto v1 = Version(first); const auto v2 = Version(second); + qDebug() << v1 << "vs" << v2; + QCOMPARE(v1 < v2, lessThan); QCOMPARE(v1 > v2, !lessThan && !equal); QCOMPARE(v1 == v2, equal); -- cgit From 5ae69c079a15fa16945b306e29925e800cb28c87 Mon Sep 17 00:00:00 2001 From: flow Date: Thu, 19 Jan 2023 21:18:39 -0300 Subject: feat(tests): add FlexVer test vector to the Version tests Signed-off-by: flow --- tests/Version_test.cpp | 97 +++++++++++++++++++++++++++++---- tests/testdata/Version/test_vectors.txt | 63 +++++++++++++++++++++ 2 files changed, 149 insertions(+), 11 deletions(-) create mode 100644 tests/testdata/Version/test_vectors.txt (limited to 'tests') diff --git a/tests/Version_test.cpp b/tests/Version_test.cpp index 6836b6fa..bb0a7f5a 100644 --- a/tests/Version_test.cpp +++ b/tests/Version_test.cpp @@ -17,15 +17,20 @@ #include -class ModUtilsTest : public QObject -{ +class VersionTest : public QObject { Q_OBJECT - void setupVersions() + + void addDataColumns() { QTest::addColumn("first"); QTest::addColumn("second"); QTest::addColumn("lessThan"); QTest::addColumn("equal"); + } + + void setupVersions() + { + addDataColumns(); QTest::newRow("equal, explicit") << "1.2.0" << "1.2.0" << false << true; QTest::newRow("equal, implicit 1") << "1.2" << "1.2.0" << false << true; @@ -49,21 +54,91 @@ class ModUtilsTest : public QObject QTest::newRow("greaterThan, two-digit") << "1.42" << "1.41" << false << false; } -private slots: - void initTestCase() + private slots: + void test_versionCompare_data() { - + setupVersions(); } - void cleanupTestCase() + + void test_versionCompare() { + QFETCH(QString, first); + QFETCH(QString, second); + QFETCH(bool, lessThan); + QFETCH(bool, equal); + const auto v1 = Version(first); + const auto v2 = Version(second); + + qDebug() << v1 << "vs" << v2; + + QCOMPARE(v1 < v2, lessThan); + QCOMPARE(v1 > v2, !lessThan && !equal); + QCOMPARE(v1 == v2, equal); } - void test_versionCompare_data() + void test_flexVerTestVector_data() { - setupVersions(); + addDataColumns(); + + QDir test_vector_dir(QFINDTESTDATA("testdata/Version")); + + QFile vector_file{test_vector_dir.absoluteFilePath("test_vectors.txt")}; + + vector_file.open(QFile::OpenModeFlag::ReadOnly); + + int test_number = 0; + const QString test_name_template { "FlexVer test #%1 (%2)" }; + for (auto line = vector_file.readLine(); !vector_file.atEnd(); line = vector_file.readLine()) { + line = line.simplified(); + if (line.startsWith('#') || line.isEmpty()) + continue; + + test_number += 1; + + auto split_line = line.split('<'); + if (split_line.size() == 2) { + QString first{split_line.first().simplified()}; + QString second{split_line.last().simplified()}; + + auto new_test_name = test_name_template.arg(QString::number(test_number), "lessThan").toLatin1().data(); + QTest::newRow(new_test_name) << first << second << true << false; + + continue; + } + + split_line = line.split('='); + if (split_line.size() == 2) { + QString first{split_line.first().simplified()}; + QString second{split_line.last().simplified()}; + + auto new_test_name = test_name_template.arg(QString::number(test_number), "equals").toLatin1().data(); + QTest::newRow(new_test_name) << first << second << false << true; + + continue; + } + + split_line = line.split('>'); + if (split_line.size() == 2) { + QString first{split_line.first().simplified()}; + QString second{split_line.last().simplified()}; + + auto new_test_name = test_name_template.arg(QString::number(test_number), "greaterThan").toLatin1().data(); + QTest::newRow(new_test_name) << first << second << false << false; + + continue; + } + + qCritical() << "Unexpected separator in the test vector: "; + qCritical() << line; + + QVERIFY(0 != 0); + } + + vector_file.close(); } - void test_versionCompare() + + void test_flexVerTestVector() { QFETCH(QString, first); QFETCH(QString, second); @@ -81,6 +156,6 @@ private slots: } }; -QTEST_GUILESS_MAIN(ModUtilsTest) +QTEST_GUILESS_MAIN(VersionTest) #include "Version_test.moc" diff --git a/tests/testdata/Version/test_vectors.txt b/tests/testdata/Version/test_vectors.txt new file mode 100644 index 00000000..e6c6507c --- /dev/null +++ b/tests/testdata/Version/test_vectors.txt @@ -0,0 +1,63 @@ +# Test vector from: +# https://github.com/unascribed/FlexVer/blob/704e12759b6e59220ff888f8bf2ec15b8f8fd969/test/test_vectors.txt +# +# This test file is formatted as " ", seperated by the space character +# Implementations should ignore lines starting with "#" and lines that have a length of 0 + +# Basic numeric ordering (lexical string sort fails these) +10 > 2 +100 > 10 + +# Trivial common numerics +1.0 < 1.1 +1.0 < 1.0.1 +1.1 > 1.0.1 + +# SemVer compatibility +1.5 > 1.5-pre1 +1.5 = 1.5+foobar + +# SemVer incompatibility +1.5 < 1.5-2 +1.5-pre10 > 1.5-pre2 + +# Empty strings + = +1 > + < 1 + +# Check boundary between textual and prerelease +a-a < a + +# Check boundary between textual and appendix +a+a = a + +# Dash is included in prerelease comparison (if stripped it will be a smaller component) +# Note that a-a < a=a regardless since the prerelease splits the component creating a smaller first component; 0 is added to force splitting regardless +a0-a < a0=a + +# Pre-releases must contain only non-digit +1.16.5-10 > 1.16.5 + +# Pre-releases can have multiple dashes (should not be split) +# Reasoning for test data: "p-a!" > "p-a-" (correct); "p-a!" < "p-a t-" (what happens if every dash creates a new component) +-a- > -a! + +# Misc +b1.7.3 > a1.2.6 +b1.2.6 > a1.7.3 +a1.1.2 < a1.1.2_01 +1.16.5-0.00.5 > 1.14.2-1.3.7 +1.0.0 < 1.0.0_01 +1.0.1 > 1.0.0_01 +1.0.0_01 < 1.0.1 +0.17.1-beta.1 < 0.17.1 +0.17.1-beta.1 < 0.17.1-beta.2 +1.4.5_01 = 1.4.5_01+fabric-1.17 +1.4.5_01 = 1.4.5_01+fabric-1.17+ohgod +14w16a < 18w40b +18w40a < 18w40b +1.4.5_01+fabric-1.17 < 18w40b +13w02a < c0.3.0_01 +0.6.0-1.18.x < 0.9.beta-1.18.x + -- cgit From 445f9e5f717bf1ad9b764704b320bbec237a7682 Mon Sep 17 00:00:00 2001 From: flow Date: Fri, 20 Jan 2023 11:11:35 -0300 Subject: feat+fix(Version): make comparsion FlexVer-compatible ... and fixes a minor issue in the parsing. This changes the expected behavior of Versions in one significant way: Now, Versions like 1.2 or 1.5 evaluate to LESS THAN 1.2.0 and 1.5.0 respectively. This makes sense for sorting versions, since one expects the versions without patch release to 'contain' the ones with, so the ones without should be evaluated uniformily with the ones with the patch. Signed-off-by: flow --- launcher/Version.cpp | 92 ++++++++++++++++++++++++++++++++------------------ launcher/Version.h | 50 ++++++++++++++++----------- tests/Version_test.cpp | 16 ++++----- 3 files changed, 97 insertions(+), 61 deletions(-) (limited to 'tests') diff --git a/launcher/Version.cpp b/launcher/Version.cpp index 9307aab3..e4311f31 100644 --- a/launcher/Version.cpp +++ b/launcher/Version.cpp @@ -10,49 +10,63 @@ Version::Version(QString str) : m_string(std::move(str)) parse(); } +#define VERSION_OPERATOR(return_on_different) \ + bool exclude_our_sections = false; \ + bool exclude_their_sections = false; \ + \ + const auto size = qMax(m_sections.size(), other.m_sections.size()); \ + for (int i = 0; i < size; ++i) { \ + Section sec1 = (i >= m_sections.size()) ? Section() : m_sections.at(i); \ + Section sec2 = (i >= other.m_sections.size()) ? Section() : other.m_sections.at(i); \ + \ + { /* Don't include appendixes in the comparison */ \ + if (sec1.isAppendix()) \ + exclude_our_sections = true; \ + if (sec2.isAppendix()) \ + exclude_their_sections = true; \ + \ + if (exclude_our_sections) { \ + sec1 = Section(); \ + if (sec2.m_isNull) \ + break; \ + } \ + \ + if (exclude_their_sections) { \ + sec2 = Section(); \ + if (sec1.m_isNull) \ + break; \ + } \ + } \ + \ + if (sec1 != sec2) \ + return return_on_different; \ + } + bool Version::operator<(const Version& other) const { - const auto size = qMax(m_sections.size(), other.m_sections.size()); - for (int i = 0; i < size; ++i) { - const Section sec1 = - (i >= m_sections.size()) ? Section("") : m_sections.at(i); - const Section sec2 = - (i >= other.m_sections.size()) ? Section("") : other.m_sections.at(i); - - if (sec1 != sec2) - return sec1 < sec2; - } + VERSION_OPERATOR(sec1 < sec2) return false; } bool Version::operator==(const Version& other) const { - const auto size = qMax(m_sections.size(), other.m_sections.size()); - for (int i = 0; i < size; ++i) { - const Section sec1 = - (i >= m_sections.size()) ? Section("") : m_sections.at(i); - const Section sec2 = - (i >= other.m_sections.size()) ? Section("") : other.m_sections.at(i); - - if (sec1 != sec2) - return false; - } + VERSION_OPERATOR(false) return true; } -bool Version::operator!=(const Version &other) const +bool Version::operator!=(const Version& other) const { return !operator==(other); } -bool Version::operator<=(const Version &other) const +bool Version::operator<=(const Version& other) const { return *this < other || *this == other; } -bool Version::operator>(const Version &other) const +bool Version::operator>(const Version& other) const { return !(*this <= other); } -bool Version::operator>=(const Version &other) const +bool Version::operator>=(const Version& other) const { return !(*this < other); } @@ -62,26 +76,38 @@ void Version::parse() m_sections.clear(); QString currentSection; - auto classChange = [](QChar lastChar, QChar currentChar) { - return !lastChar.isNull() && ((!lastChar.isDigit() && currentChar.isDigit()) || (lastChar.isDigit() && !currentChar.isDigit())); + if (m_string.isEmpty()) + return; + + auto classChange = [&](QChar lastChar, QChar currentChar) { + if (lastChar.isNull()) + return false; + if (lastChar.isDigit() != currentChar.isDigit()) + return true; + + const QList s_separators{ '.', '-', '+' }; + if (s_separators.contains(currentChar) && currentSection.at(0) != currentChar) + return true; + + return false; }; - for (int i = 0; i < m_string.size(); ++i) { + currentSection += m_string.at(0); + for (int i = 1; i < m_string.size(); ++i) { const auto& current_char = m_string.at(i); - if ((i > 0 && classChange(m_string.at(i - 1), current_char)) || current_char == '.' || current_char == '-' || current_char == '+') { - if (!currentSection.isEmpty()) { + if (classChange(m_string.at(i - 1), current_char)) { + if (!currentSection.isEmpty()) m_sections.append(Section(currentSection)); - } currentSection = ""; } + currentSection += current_char; } - if (!currentSection.isEmpty()) { + + if (!currentSection.isEmpty()) m_sections.append(Section(currentSection)); - } } - /// qDebug print support for the Version class QDebug operator<<(QDebug debug, const Version& v) { diff --git a/launcher/Version.h b/launcher/Version.h index 23481c29..659f8e54 100644 --- a/launcher/Version.h +++ b/launcher/Version.h @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-3.0-only /* * PolyMC - Minecraft Launcher + * Copyright (C) 2023 flowln * Copyright (C) 2022 Sefa Eyeoglu * * This program is free software: you can redistribute it and/or modify @@ -60,7 +61,7 @@ class Version { private: struct Section { - explicit Section(QString fullString) : m_isNull(true), m_fullString(std::move(fullString)) + explicit Section(QString fullString) : m_fullString(std::move(fullString)) { int cutoff = m_fullString.size(); for (int i = 0; i < m_fullString.size(); i++) { @@ -95,45 +96,54 @@ class Version { explicit Section() = default; - bool m_isNull = false; - int m_numPart = 0; + bool m_isNull = true; + int m_numPart = 0; QString m_stringPart; + QString m_fullString; + [[nodiscard]] inline bool isAppendix() const { return m_stringPart.startsWith('+'); } + [[nodiscard]] inline bool isPreRelease() const { return m_stringPart.startsWith('-') && m_stringPart.length() > 1; } + inline bool operator==(const Section& other) const { if (m_isNull && !other.m_isNull) - return other.m_numPart == 0; - + return false; if (!m_isNull && other.m_isNull) - return m_numPart == 0; - - if (m_isNull || other.m_isNull) - return (m_stringPart == ".") || (other.m_stringPart == "."); + return false; - if (!m_isNull && !other.m_isNull) + if (!m_isNull && !other.m_isNull) { return (m_numPart == other.m_numPart) && (m_stringPart == other.m_stringPart); + } - return m_fullString == other.m_fullString; + return true; } - inline bool operator<(const Section &other) const + inline bool operator<(const Section& other) const { - if (m_isNull && !other.m_isNull) - return other.m_numPart > 0; + static auto unequal_is_less = [](Section const& non_null) -> bool { + if (non_null.m_stringPart.isEmpty()) + return non_null.m_numPart == 0; + return (non_null.m_stringPart != QLatin1Char('.')) && non_null.isPreRelease(); + }; if (!m_isNull && other.m_isNull) - return m_numPart < 0; - - if (m_isNull || other.m_isNull) - return true; + return unequal_is_less(*this); + if (m_isNull && !other.m_isNull) + return !unequal_is_less(other); if (!m_isNull && !other.m_isNull) { - if(m_numPart < other.m_numPart) + if (m_numPart < other.m_numPart) + return true; + if (m_numPart == other.m_numPart && m_stringPart < other.m_stringPart) return true; - if(m_numPart == other.m_numPart && m_stringPart < other.m_stringPart) + + if (!m_stringPart.isEmpty() && other.m_stringPart.isEmpty()) + return false; + if (m_stringPart.isEmpty() && !other.m_stringPart.isEmpty()) return true; + return false; } diff --git a/tests/Version_test.cpp b/tests/Version_test.cpp index bb0a7f5a..afb4c610 100644 --- a/tests/Version_test.cpp +++ b/tests/Version_test.cpp @@ -33,24 +33,24 @@ class VersionTest : public QObject { addDataColumns(); QTest::newRow("equal, explicit") << "1.2.0" << "1.2.0" << false << true; - QTest::newRow("equal, implicit 1") << "1.2" << "1.2.0" << false << true; - QTest::newRow("equal, implicit 2") << "1.2.0" << "1.2" << false << true; QTest::newRow("equal, two-digit") << "1.42" << "1.42" << false << true; QTest::newRow("lessThan, explicit 1") << "1.2.0" << "1.2.1" << true << false; QTest::newRow("lessThan, explicit 2") << "1.2.0" << "1.3.0" << true << false; QTest::newRow("lessThan, explicit 3") << "1.2.0" << "2.2.0" << true << false; - QTest::newRow("lessThan, implicit 1") << "1.2" << "1.2.1" << true << false; - QTest::newRow("lessThan, implicit 2") << "1.2" << "1.3.0" << true << false; - QTest::newRow("lessThan, implicit 3") << "1.2" << "2.2.0" << true << false; + QTest::newRow("lessThan, implicit 1") << "1.2" << "1.2.0" << true << false; + QTest::newRow("lessThan, implicit 2") << "1.2" << "1.2.1" << true << false; + QTest::newRow("lessThan, implicit 3") << "1.2" << "1.3.0" << true << false; + QTest::newRow("lessThan, implicit 4") << "1.2" << "2.2.0" << true << false; QTest::newRow("lessThan, two-digit") << "1.41" << "1.42" << true << false; QTest::newRow("greaterThan, explicit 1") << "1.2.1" << "1.2.0" << false << false; QTest::newRow("greaterThan, explicit 2") << "1.3.0" << "1.2.0" << false << false; QTest::newRow("greaterThan, explicit 3") << "2.2.0" << "1.2.0" << false << false; - QTest::newRow("greaterThan, implicit 1") << "1.2.1" << "1.2" << false << false; - QTest::newRow("greaterThan, implicit 2") << "1.3.0" << "1.2" << false << false; - QTest::newRow("greaterThan, implicit 3") << "2.2.0" << "1.2" << false << false; + QTest::newRow("greaterThan, implicit 1") << "1.2.0" << "1.2" << false << false; + QTest::newRow("greaterThan, implicit 2") << "1.2.1" << "1.2" << false << false; + QTest::newRow("greaterThan, implicit 3") << "1.3.0" << "1.2" << false << false; + QTest::newRow("greaterThan, implicit 4") << "2.2.0" << "1.2" << false << false; QTest::newRow("greaterThan, two-digit") << "1.42" << "1.41" << false << false; } -- cgit