From 8a09bfb2979d1a4845cf2d6c68f07eb7c61e578b Mon Sep 17 00:00:00 2001 From: Paul Colby Date: Sat, 20 Aug 2016 22:47:50 +1000 Subject: [PATCH] Don't use Qt 5.7's QVariant::toString for floats (#69) Qt 5.7 added `QLocale::FloatingPointShortest` (see qt/qtbase@726fed0), and updated `QVariant` to use that (instead of the previous implementation added in Qt 5.5) when converting floats and doubles to string, resulting in slightly different output, and QCOMPARE unit test failures. In this change, we use the Qt 5.5 / 5.6 implementation because its at least as accurate as 5.7+, and implementing a 5.7-compatible version would be a major undertaking (needing to duplicate the third-party double-conversion code Qt borrows from the V8 project). --- src/polar/v2/trainingsession.cpp | 24 +++++++++++++----- test/polar/v2/testtrainingsession.cpp | 36 ++++++++++++++++++--------- 2 files changed, 42 insertions(+), 18 deletions(-) diff --git a/src/polar/v2/trainingsession.cpp b/src/polar/v2/trainingsession.cpp index 5118c5cf..e9638649 100644 --- a/src/polar/v2/trainingsession.cpp +++ b/src/polar/v2/trainingsession.cpp @@ -37,13 +37,25 @@ #include #endif -// As of Qt 5.5 increased the accuracy of QVariant::toString output for -// doubles and floats (see qtproject/qtbase@8153386). Here replicate -// that behaviour for pre-5.5 for a) better accuracy in older Qt versions -// and b) consistent output (eg in unit tests) across all Qt 5.x versions. -#if (QT_VERSION >= QT_VERSION_CHECK(5, 5, 0)) +// Qt 5.5 increased the accuracy of QVariant::toString output for floats and +// doubles (see qtproject/qtbase@8153386), resulting in slightly different +// output, and QCOMPARE unit test failures. +// https://github.com/qt/qtbase/commit/8153386397087ce4f5c8997992edf5c1fd38b8db +// +// Qt 5.7 added QLocale::FloatingPointShortest (see qt/qtbase@726fed0), and +// updated QVariant to use that (instead of the Qt 5.5 change above) when +// converting floats and doubles to string, again resulting in slightly +// different output, and QCOMPARE unit test failures. +// https://github.com/qt/qtbase/commit/726fed0d67013cbfac7921d3d4613ca83406fb0f +// +// So, QVariant floats and doubles convert (and compare) differently between +// Qt 5.[0-4], 5.[5,6], and 5.7+. Here we use the Qt 5.5 / 5.6 implementation +// because its at least as accurate as 5.7+, and implementing a 5.7-compatible +// fallback would be a major undertaking (needing to duplicate the third-party +// double-conversion code Qt borrows from the V8 project). +#if (QT_VERSION >= QT_VERSION_CHECK(5, 5, 0)) && (QT_VERSION < QT_VERSION_CHECK(5, 7, 0)) #define VARIANT_TO_STRING(v) v.toString() -#else // Fallback implementation based closely on Qt 5.5's qvariant.h +#else // Fallback implementation based closely on Qt 5.5's qvariant.cpp #ifndef DBL_MANT_DIG #define DBL_MANT_DIG 53 #endif diff --git a/test/polar/v2/testtrainingsession.cpp b/test/polar/v2/testtrainingsession.cpp index 92528beb..d551150e 100644 --- a/test/polar/v2/testtrainingsession.cpp +++ b/test/polar/v2/testtrainingsession.cpp @@ -66,15 +66,31 @@ void compare(const QDomNodeList &a, const QDomNodeList &b) } } -void fuzzyCompare(const QString &a, const QString &b, bool &compared) +void fuzzyCompare(const QString &a, const QString &b) { - bool aOK, bOK; - const double aDouble = a.toDouble(&aOK); - const double bDouble = b.toDouble(&bOK); - if (aOK && bOK) { - compared = true; - QCOMPARE(aDouble, bDouble); + // Only consider fuzzy comparisons for floating point numbers. + if ((a.contains(QLatin1Char('.'))) || (a.contains(QLatin1Char('.')))) { + if ((a.length() <= 10) && (b.length() <= 10)) { // float precision. + bool aOK, bOK; + const float aFloat = a.toFloat(&aOK); + const float bFloat = b.toFloat(&bOK); + if (aOK && bOK) { + QCOMPARE(aFloat, bFloat); + return; + } + } else { // double precision. + bool aOK, bOK; + const double aDouble = a.toDouble(&aOK); + const double bDouble = b.toDouble(&bOK); + if (aOK && bOK) { + QCOMPARE(aDouble, bDouble); + return; + } + } } + + // Either value was not a floating point number, so compare literal strings. + QCOMPARE(a, b); } void compare(const QDomNode &a, const QDomNode &b) @@ -85,11 +101,7 @@ void compare(const QDomNode &a, const QDomNode &b) QCOMPARE(a.namespaceURI(), b.namespaceURI()); QCOMPARE(a.nodeName(), b.nodeName()); QCOMPARE(a.nodeType(), b.nodeType()); - bool compared = false; - fuzzyCompare(a.nodeValue(), b.nodeValue(), compared); - if (!compared) { - QCOMPARE(a.nodeValue(), b.nodeValue()); - } + fuzzyCompare(a.nodeValue(), b.nodeValue()); QCOMPARE(a.prefix(), b.prefix()); }