From da973c3cafd74fae64a2942a0a6c2e441d76be47 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 21 Feb 2021 00:30:46 +0100 Subject: [PATCH 01/16] Get rid of CppUnit boilerplate in numeric validator unit tests No real changes, just drop CppUnit::TestCase inheritance and the legacy macros and use TEST_CASE_METHOD() instead. --- tests/validators/valnum.cpp | 56 ++++++++++--------------------------- 1 file changed, 15 insertions(+), 41 deletions(-) diff --git a/tests/validators/valnum.cpp b/tests/validators/valnum.cpp index d3a063dab0..3323b3d49a 100644 --- a/tests/validators/valnum.cpp +++ b/tests/validators/valnum.cpp @@ -22,55 +22,29 @@ #include "testableframe.h" #include "wx/uiaction.h" -class NumValidatorTestCase : public CppUnit::TestCase +class NumValidatorTestCase { public: - NumValidatorTestCase() { } + NumValidatorTestCase(); + ~NumValidatorTestCase(); - void setUp() wxOVERRIDE; - void tearDown() wxOVERRIDE; - -private: - CPPUNIT_TEST_SUITE( NumValidatorTestCase ); - CPPUNIT_TEST( TransferInt ); - CPPUNIT_TEST( TransferUnsigned ); - CPPUNIT_TEST( TransferFloat ); - CPPUNIT_TEST( ZeroAsBlank ); - CPPUNIT_TEST( NoTrailingZeroes ); - WXUISIM_TEST( Interactive ); - CPPUNIT_TEST_SUITE_END(); - - void TransferInt(); - void TransferUnsigned(); - void TransferFloat(); - void ZeroAsBlank(); - void NoTrailingZeroes(); -#if wxUSE_UIACTIONSIMULATOR - void Interactive(); -#endif // wxUSE_UIACTIONSIMULATOR - - wxTextCtrl *m_text; +protected: + wxTextCtrl* const m_text; wxDECLARE_NO_COPY_CLASS(NumValidatorTestCase); }; -// register in the unnamed registry so that these tests are run by default -CPPUNIT_TEST_SUITE_REGISTRATION( NumValidatorTestCase ); - -// also include in its own registry so that these tests can be run alone -CPPUNIT_TEST_SUITE_NAMED_REGISTRATION( NumValidatorTestCase, "NumValidatorTestCase" ); - -void NumValidatorTestCase::setUp() +NumValidatorTestCase::NumValidatorTestCase() + : m_text(new wxTextCtrl(wxTheApp->GetTopWindow(), wxID_ANY)) { - m_text = new wxTextCtrl(wxTheApp->GetTopWindow(), wxID_ANY); } -void NumValidatorTestCase::tearDown() +NumValidatorTestCase::~NumValidatorTestCase() { - wxTheApp->GetTopWindow()->DestroyChildren(); + delete m_text; } -void NumValidatorTestCase::TransferInt() +TEST_CASE_METHOD(NumValidatorTestCase, "ValNum::TransferInt", "[valnum]") { int value = 0; wxIntegerValidator valInt(&value); @@ -98,7 +72,7 @@ void NumValidatorTestCase::TransferInt() CPPUNIT_ASSERT( !valInt.TransferFromWindow() ); } -void NumValidatorTestCase::TransferUnsigned() +TEST_CASE_METHOD(NumValidatorTestCase, "ValNum::TransferUnsigned", "[valnum]") { unsigned value = 0; wxIntegerValidator valUnsigned(&value); @@ -129,7 +103,7 @@ void NumValidatorTestCase::TransferUnsigned() CPPUNIT_ASSERT( !valUnsigned.TransferFromWindow() ); } -void NumValidatorTestCase::TransferFloat() +TEST_CASE_METHOD(NumValidatorTestCase, "ValNum::TransferFloat", "[valnum]") { // We need a locale with point as decimal separator. wxLocale loc(wxLANGUAGE_ENGLISH_UK, wxLOCALE_DONT_LOAD_DEFAULT); @@ -161,7 +135,7 @@ void NumValidatorTestCase::TransferFloat() CPPUNIT_ASSERT( !valFloat.TransferFromWindow() ); } -void NumValidatorTestCase::ZeroAsBlank() +TEST_CASE_METHOD(NumValidatorTestCase, "ValNum::ZeroAsBlank", "[valnum]") { long value = 0; m_text->SetValidator( @@ -177,7 +151,7 @@ void NumValidatorTestCase::ZeroAsBlank() CPPUNIT_ASSERT_EQUAL( 0, value ); } -void NumValidatorTestCase::NoTrailingZeroes() +TEST_CASE_METHOD(NumValidatorTestCase, "ValNum::NoTrailingZeroes", "[valnum]") { // We need a locale with point as decimal separator. wxLocale loc(wxLANGUAGE_ENGLISH_UK, wxLOCALE_DONT_LOAD_DEFAULT); @@ -198,7 +172,7 @@ void NumValidatorTestCase::NoTrailingZeroes() #if wxUSE_UIACTIONSIMULATOR -void NumValidatorTestCase::Interactive() +TEST_CASE_METHOD(NumValidatorTestCase, "ValNum::Interactive", "[valnum]") { #ifdef __WXMSW__ // FIXME: This test fails on MSW buildbot slaves although works fine on From d2b2e0a4edded11440952faed0d7658804f47e64 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 21 Feb 2021 00:33:22 +0100 Subject: [PATCH 02/16] Replace CPPUNIT_ASSERT() macros in numeric validator unit tests Simplify code and use CHECK() rather than REQUIRE(), which is what CPPUNIT_ASSERT() is defined as, to continue running the tests even if some of them fail. --- tests/validators/valnum.cpp | 98 ++++++++++++++++++------------------- 1 file changed, 49 insertions(+), 49 deletions(-) diff --git a/tests/validators/valnum.cpp b/tests/validators/valnum.cpp index 3323b3d49a..c6c24eb635 100644 --- a/tests/validators/valnum.cpp +++ b/tests/validators/valnum.cpp @@ -50,26 +50,26 @@ TEST_CASE_METHOD(NumValidatorTestCase, "ValNum::TransferInt", "[valnum]") wxIntegerValidator valInt(&value); valInt.SetWindow(m_text); - CPPUNIT_ASSERT( valInt.TransferToWindow() ); - CPPUNIT_ASSERT_EQUAL( "0", m_text->GetValue() ); + CHECK( valInt.TransferToWindow() ); + CHECK( m_text->GetValue() == "0" ); value = 17; - CPPUNIT_ASSERT( valInt.TransferToWindow() ); - CPPUNIT_ASSERT_EQUAL( "17", m_text->GetValue() ); + CHECK( valInt.TransferToWindow() ); + CHECK( m_text->GetValue() == "17" ); m_text->ChangeValue("foobar"); - CPPUNIT_ASSERT( !valInt.TransferFromWindow() ); + CHECK( !valInt.TransferFromWindow() ); m_text->ChangeValue("-234"); - CPPUNIT_ASSERT( valInt.TransferFromWindow() ); - CPPUNIT_ASSERT_EQUAL( -234, value ); + CHECK( valInt.TransferFromWindow() ); + CHECK( value == -234 ); m_text->ChangeValue("9223372036854775808"); // == LLONG_MAX + 1 - CPPUNIT_ASSERT( !valInt.TransferFromWindow() ); + CHECK( !valInt.TransferFromWindow() ); m_text->Clear(); - CPPUNIT_ASSERT( !valInt.TransferFromWindow() ); + CHECK( !valInt.TransferFromWindow() ); } TEST_CASE_METHOD(NumValidatorTestCase, "ValNum::TransferUnsigned", "[valnum]") @@ -78,29 +78,29 @@ TEST_CASE_METHOD(NumValidatorTestCase, "ValNum::TransferUnsigned", "[valnum]") wxIntegerValidator valUnsigned(&value); valUnsigned.SetWindow(m_text); - CPPUNIT_ASSERT( valUnsigned.TransferToWindow() ); - CPPUNIT_ASSERT_EQUAL( "0", m_text->GetValue() ); + CHECK( valUnsigned.TransferToWindow() ); + CHECK( m_text->GetValue() == "0" ); value = 17; - CPPUNIT_ASSERT( valUnsigned.TransferToWindow() ); - CPPUNIT_ASSERT_EQUAL( "17", m_text->GetValue() ); + CHECK( valUnsigned.TransferToWindow() ); + CHECK( m_text->GetValue() == "17" ); m_text->ChangeValue("foobar"); - CPPUNIT_ASSERT( !valUnsigned.TransferFromWindow() ); + CHECK( !valUnsigned.TransferFromWindow() ); m_text->ChangeValue("-234"); - CPPUNIT_ASSERT( !valUnsigned.TransferFromWindow() ); + CHECK( !valUnsigned.TransferFromWindow() ); m_text->ChangeValue("234"); - CPPUNIT_ASSERT( valUnsigned.TransferFromWindow() ); - CPPUNIT_ASSERT_EQUAL( 234, value ); + CHECK( valUnsigned.TransferFromWindow() ); + CHECK( value == 234 ); m_text->ChangeValue("18446744073709551616"); // == ULLONG_MAX + 1 - CPPUNIT_ASSERT( !valUnsigned.TransferFromWindow() ); + CHECK( !valUnsigned.TransferFromWindow() ); m_text->Clear(); - CPPUNIT_ASSERT( !valUnsigned.TransferFromWindow() ); + CHECK( !valUnsigned.TransferFromWindow() ); } TEST_CASE_METHOD(NumValidatorTestCase, "ValNum::TransferFloat", "[valnum]") @@ -112,27 +112,27 @@ TEST_CASE_METHOD(NumValidatorTestCase, "ValNum::TransferFloat", "[valnum]") wxFloatingPointValidator valFloat(3, &value); valFloat.SetWindow(m_text); - CPPUNIT_ASSERT( valFloat.TransferToWindow() ); - CPPUNIT_ASSERT_EQUAL( "0.000", m_text->GetValue() ); + CHECK( valFloat.TransferToWindow() ); + CHECK( m_text->GetValue() == "0.000" ); value = 1.234f; - CPPUNIT_ASSERT( valFloat.TransferToWindow() ); - CPPUNIT_ASSERT_EQUAL( "1.234", m_text->GetValue() ); + CHECK( valFloat.TransferToWindow() ); + CHECK( m_text->GetValue() == "1.234" ); value = 1.2345678f; - CPPUNIT_ASSERT( valFloat.TransferToWindow() ); - CPPUNIT_ASSERT_EQUAL( "1.235", m_text->GetValue() ); + CHECK( valFloat.TransferToWindow() ); + CHECK( m_text->GetValue() == "1.235" ); m_text->ChangeValue("foobar"); - CPPUNIT_ASSERT( !valFloat.TransferFromWindow() ); + CHECK( !valFloat.TransferFromWindow() ); m_text->ChangeValue("-234.567"); - CPPUNIT_ASSERT( valFloat.TransferFromWindow() ); - CPPUNIT_ASSERT_EQUAL( -234.567f, value ); + CHECK( valFloat.TransferFromWindow() ); + CHECK( value == -234.567f ); m_text->Clear(); - CPPUNIT_ASSERT( !valFloat.TransferFromWindow() ); + CHECK( !valFloat.TransferFromWindow() ); } TEST_CASE_METHOD(NumValidatorTestCase, "ValNum::ZeroAsBlank", "[valnum]") @@ -143,12 +143,12 @@ TEST_CASE_METHOD(NumValidatorTestCase, "ValNum::ZeroAsBlank", "[valnum]") wxValidator * const val = m_text->GetValidator(); - CPPUNIT_ASSERT( val->TransferToWindow() ); - CPPUNIT_ASSERT_EQUAL( "", m_text->GetValue() ); + CHECK( val->TransferToWindow() ); + CHECK( m_text->GetValue() == "" ); value++; - CPPUNIT_ASSERT( val->TransferFromWindow() ); - CPPUNIT_ASSERT_EQUAL( 0, value ); + CHECK( val->TransferFromWindow() ); + CHECK( value == 0 ); } TEST_CASE_METHOD(NumValidatorTestCase, "ValNum::NoTrailingZeroes", "[valnum]") @@ -162,12 +162,12 @@ TEST_CASE_METHOD(NumValidatorTestCase, "ValNum::NoTrailingZeroes", "[valnum]") wxValidator * const val = m_text->GetValidator(); - CPPUNIT_ASSERT( val->TransferToWindow() ); - CPPUNIT_ASSERT_EQUAL( "1.2", m_text->GetValue() ); + CHECK( val->TransferToWindow() ); + CHECK( m_text->GetValue() == "1.2" ); value = 1.234; - CPPUNIT_ASSERT( val->TransferToWindow() ); - CPPUNIT_ASSERT_EQUAL( "1.234", m_text->GetValue() ); + CHECK( val->TransferToWindow() ); + CHECK( m_text->GetValue() == "1.234" ); } #if wxUSE_UIACTIONSIMULATOR @@ -202,12 +202,12 @@ TEST_CASE_METHOD(NumValidatorTestCase, "ValNum::Interactive", "[valnum]") m_text->SetFocus(); sim.Char('-'); wxYield(); - CPPUNIT_ASSERT_EQUAL( "", m_text->GetValue() ); + CHECK( m_text->GetValue() == "" ); // Neither is entering '.' or any non-digit character. sim.Text(".a+/"); wxYield(); - CPPUNIT_ASSERT_EQUAL( "", m_text->GetValue() ); + CHECK( m_text->GetValue() == "" ); // Entering digits should work though and after leaving the control the // contents should be normalized. @@ -216,43 +216,43 @@ TEST_CASE_METHOD(NumValidatorTestCase, "ValNum::Interactive", "[valnum]") text2->SetFocus(); wxYield(); if ( loc.IsOk() ) - CPPUNIT_ASSERT_EQUAL( "1,234,567", m_text->GetValue() ); + CHECK( m_text->GetValue() == "1,234,567" ); else - CPPUNIT_ASSERT_EQUAL( "1234567", m_text->GetValue() ); + CHECK( m_text->GetValue() == "1234567" ); // Entering both '-' and '.' in this control should work but only in the // correct order. sim.Char('-'); wxYield(); - CPPUNIT_ASSERT_EQUAL( "-", text2->GetValue() ); + CHECK( text2->GetValue() == "-" ); text2->SetInsertionPoint(0); sim.Char('.'); wxYield(); - CPPUNIT_ASSERT_EQUAL( "-", text2->GetValue() ); + CHECK( text2->GetValue() == "-" ); text2->SetInsertionPointEnd(); sim.Char('.'); wxYield(); - CPPUNIT_ASSERT_EQUAL( "-.", text2->GetValue() ); + CHECK( text2->GetValue() == "-." ); // Adding up to three digits after the point should work. sim.Text("987"); wxYield(); - CPPUNIT_ASSERT_EQUAL( "-.987", text2->GetValue() ); + CHECK( text2->GetValue() == "-.987" ); // But no more. sim.Text("654"); wxYield(); - CPPUNIT_ASSERT_EQUAL( "-.987", text2->GetValue() ); + CHECK( text2->GetValue() == "-.987" ); // We can remove one digit and another one though. sim.Char(WXK_BACK); sim.Char(WXK_BACK); sim.Char('6'); wxYield(); - CPPUNIT_ASSERT_EQUAL( "-.96", text2->GetValue() ); + CHECK( text2->GetValue() == "-.96" ); // Also test the range constraint. @@ -260,11 +260,11 @@ TEST_CASE_METHOD(NumValidatorTestCase, "ValNum::Interactive", "[valnum]") sim.Char('9'); wxYield(); - CPPUNIT_ASSERT_EQUAL( "9", text2->GetValue() ); + CHECK( text2->GetValue() == "9" ); sim.Char('9'); wxYield(); - CPPUNIT_ASSERT_EQUAL( "9", text2->GetValue() ); + CHECK( text2->GetValue() == "9" ); } #endif // wxUSE_UIACTIONSIMULATOR From 709b259bbb0953de20709f003c01f2348a97cb30 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 21 Feb 2021 14:55:16 +0100 Subject: [PATCH 03/16] Don't define copy ctor in numeric validator classes unnecessarily There is just no need to do it manually, when the compiler can do the job perfectly well itself. No real changes, but this will simplify the upcoming refactoring. --- include/wx/valnum.h | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/include/wx/valnum.h b/include/wx/valnum.h index ba684c1ab3..94174a9ad7 100644 --- a/include/wx/valnum.h +++ b/include/wx/valnum.h @@ -294,12 +294,7 @@ protected: "This style doesn't make sense for integers." ); } - wxIntegerValidatorBase(const wxIntegerValidatorBase& other) - : wxNumValidatorBase(other) - { - m_min = other.m_min; - m_max = other.m_max; - } + // Default copy ctor is ok. // Provide methods for wxNumValidator use. wxString ToString(LongestValueType value) const; @@ -395,15 +390,7 @@ protected: m_factor = 1.0; } - wxFloatingPointValidatorBase(const wxFloatingPointValidatorBase& other) - : wxNumValidatorBase(other) - { - m_precision = other.m_precision; - m_factor = other.m_factor; - - m_min = other.m_min; - m_max = other.m_max; - } + // Default copy ctor is ok. // Provide methods for wxNumValidator use. wxString ToString(LongestValueType value) const; From 7371131f1e9adbecfc7f9554bb4da0c13477ff73 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 21 Feb 2021 15:09:03 +0100 Subject: [PATCH 04/16] Fix confusion around '-' handling in numeric validators code Actual implementation of IsCharOk() didn't correspond to the comments in it or near the function declaration which stated that it's never called with ch='-' as argument -- it was called with it and called IsMinusOk() right before a comment saying that it doesn't need to do it, which was very confusing. Fix this by making the behaviour really correspond to the comments and handling '-' at the base class level. This required introducing a new pure virtual CanBeNegative() function, but it's going to be useful for other things later too. Still keep IsMinusOk() helper, but make it private now because it doesn't need to be called from the derived class IsCharOk() any longer. --- include/wx/valnum.h | 21 ++++++++++++++++----- src/common/valnum.cpp | 37 +++++++++++++++++-------------------- 2 files changed, 33 insertions(+), 25 deletions(-) diff --git a/include/wx/valnum.h b/include/wx/valnum.h index 94174a9ad7..c7858dec13 100644 --- a/include/wx/valnum.h +++ b/include/wx/valnum.h @@ -79,11 +79,6 @@ protected: // bits of our style to the corresponding wxNumberFormatter::Style values. int GetFormatFlags() const; - // Return true if pressing a '-' key is acceptable for the current control - // contents and insertion point. This is meant to be called from the - // derived class IsCharOk() implementation. - bool IsMinusOk(const wxString& val, int pos) const; - // Return the string which would result from inserting the given character // at the specified position. wxString GetValueAfterInsertingChar(wxString val, int pos, wxChar ch) const @@ -92,6 +87,11 @@ protected: return val; } + // Return true if this control allows negative numbers in it. + // + // If it doesn't, we don't allow entering "-" at all. + virtual bool CanBeNegative() const = 0; + private: // Check whether the specified character can be inserted in the control at // the given position in the string representing the current controls @@ -114,6 +114,11 @@ private: // Determine the current insertion point and text in the associated control. void GetCurrentValueAndInsertionPoint(wxString& val, int& pos) const; + // Return true if pressing a '-' key is acceptable for the current control + // contents and insertion point. This is used by OnChar() to handle '-' and + // relies on CanBeNegative() implementation in the derived class. + bool IsMinusOk(const wxString& val, int pos) const; + // Combination of wxVAL_NUM_XXX values. int m_style; @@ -346,6 +351,9 @@ public: virtual wxObject *Clone() const wxOVERRIDE { return new wxIntegerValidator(*this); } +protected: + virtual bool CanBeNegative() const wxOVERRIDE { return DoGetMin() < 0; } + private: wxDECLARE_NO_ASSIGN_CLASS(wxIntegerValidator); }; @@ -458,6 +466,9 @@ public: return new wxFloatingPointValidator(*this); } +protected: + virtual bool CanBeNegative() const wxOVERRIDE { return DoGetMin() < 0; } + private: typedef typename Base::LongestValueType LongestValueType; diff --git a/src/common/valnum.cpp b/src/common/valnum.cpp index 33344b185b..8704e72fce 100644 --- a/src/common/valnum.cpp +++ b/src/common/valnum.cpp @@ -117,6 +117,10 @@ wxNumValidatorBase::GetCurrentValueAndInsertionPoint(wxString& val, bool wxNumValidatorBase::IsMinusOk(const wxString& val, int pos) const { + // We need to know if we accept negative numbers at all. + if ( !CanBeNegative() ) + return false; + // Minus is only ever accepted in the beginning of the string. if ( pos != 0 ) return false; @@ -125,6 +129,15 @@ bool wxNumValidatorBase::IsMinusOk(const wxString& val, int pos) const if ( !val.empty() && val[0] == '-' ) return false; + // Notice that entering '-' can make our value invalid, for example if + // we're limited to -5..15 range and the current value is 12, then the + // new value would be (invalid) -12. We consider it better to let the + // user do this because perhaps he is going to press Delete key next to + // make it -2 and forcing him to delete 1 first would be unnatural. + // + // TODO: It would be nice to indicate that the current control contents + // is invalid (if it's indeed going to be the case) once + // wxValidator supports doing this non-intrusively. return true; } @@ -165,7 +178,10 @@ void wxNumValidatorBase::OnChar(wxKeyEvent& event) int pos; GetCurrentValueAndInsertionPoint(val, pos); - if ( !IsCharOk(val, pos, ch) ) + // Minus is a special case because we can deal with it directly here, for + // all the rest call the derived class virtual function. + const bool ok = ch == '-' ? IsMinusOk(val, pos) : IsCharOk(val, pos, ch); + if ( !ok ) { if ( !wxValidator::IsSilent() ) wxBell(); @@ -226,21 +242,6 @@ wxIntegerValidatorBase::FromString(const wxString& s, LongestValueType *value) bool wxIntegerValidatorBase::IsCharOk(const wxString& val, int pos, wxChar ch) const { - // We may accept minus sign if we can represent negative numbers at all. - if ( ch == '-' ) - { - // Notice that entering '-' can make our value invalid, for example if - // we're limited to -5..15 range and the current value is 12, then the - // new value would be (invalid) -12. We consider it better to let the - // user do this because perhaps he is going to press Delete key next to - // make it -2 and forcing him to delete 1 first would be unnatural. - // - // TODO: It would be nice to indicate that the current control contents - // is invalid (if it's indeed going to be the case) once - // wxValidator supports doing this non-intrusively. - return m_min < 0 && IsMinusOk(val, pos); - } - // We only accept digits here (remember that '-' is taken care of by the // base class already). if ( ch < '0' || ch > '9' ) @@ -292,10 +293,6 @@ wxFloatingPointValidatorBase::IsCharOk(const wxString& val, int pos, wxChar ch) const { - // We may accept minus sign if we can represent negative numbers at all. - if ( ch == '-' ) - return m_min < 0 && IsMinusOk(val, pos); - const wxChar separator = wxNumberFormatter::GetDecimalSeparator(); if ( ch == separator ) { From d3f051e328d0c427cfb42bc0a54ea75c4b824507 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 21 Feb 2021 16:43:04 +0100 Subject: [PATCH 05/16] Document wxNumberFormatter overloads using long long These overloads were added back in f2a5052baa (Add support for long long to wxNumberFormatter., 2011-01-19) but never documented. --- interface/wx/numformatter.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/interface/wx/numformatter.h b/interface/wx/numformatter.h index db678d08f5..c6e0eaffe2 100644 --- a/interface/wx/numformatter.h +++ b/interface/wx/numformatter.h @@ -70,7 +70,10 @@ public: Combination of values from the Style enumeration (except for Style_NoTrailingZeroes which can't be used with this overload). */ + //@{ static wxString ToString(long val, int flags = Style_WithThousandsSep); + static wxString ToString(long long val, int flags = Style_WithThousandsSep); + //@} /** Returns string representation of a floating point number. @@ -98,6 +101,7 @@ public: */ //@{ static bool FromString(wxString s, long *val); + static bool FromString(wxString s, long long *val); static bool FromString(wxString s, double *val); //@} From 5ebce6549477e35d21b07699baa901982c2e978a Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 21 Feb 2021 16:52:06 +0100 Subject: [PATCH 06/16] Get rid of CppUnit boilerplate in wxNumberFormatter unit tests No real changes, just drop CppUnit::TestCase inheritance and the legacy macros and use TEST_CASE_METHOD() instead. --- tests/strings/numformatter.cpp | 82 +++++++++------------------------- 1 file changed, 21 insertions(+), 61 deletions(-) diff --git a/tests/strings/numformatter.cpp b/tests/strings/numformatter.cpp index 6416a2ddd1..4ce8e3c449 100644 --- a/tests/strings/numformatter.cpp +++ b/tests/strings/numformatter.cpp @@ -20,72 +20,32 @@ // test class // ---------------------------------------------------------------------------- -class NumFormatterTestCase : public CppUnit::TestCase +class NumFormatterTestCase { public: - NumFormatterTestCase() { m_locale = NULL; } - - virtual void setUp() wxOVERRIDE - { + NumFormatterTestCase() : // We need to use a locale with known decimal point and which uses the // thousands separator for the tests to make sense. - m_locale = new wxLocale(wxLANGUAGE_ENGLISH_UK, - wxLOCALE_DONT_LOAD_DEFAULT); - if ( !m_locale->IsOk() ) - tearDown(); + m_locale(wxLANGUAGE_ENGLISH_UK, wxLOCALE_DONT_LOAD_DEFAULT) + { } - virtual void tearDown() wxOVERRIDE - { - delete m_locale; - m_locale = NULL; - } +protected: + bool CanRunTest() const { return m_locale.IsOk(); } private: - CPPUNIT_TEST_SUITE( NumFormatterTestCase ); - CPPUNIT_TEST( LongToString ); -#ifdef wxHAS_LONG_LONG_T_DIFFERENT_FROM_LONG - CPPUNIT_TEST( LongLongToString ); -#endif // wxHAS_LONG_LONG_T_DIFFERENT_FROM_LONG - CPPUNIT_TEST( DoubleToString ); - CPPUNIT_TEST( NoTrailingZeroes ); - CPPUNIT_TEST( LongFromString ); -#ifdef wxHAS_LONG_LONG_T_DIFFERENT_FROM_LONG - CPPUNIT_TEST( LongLongFromString ); -#endif // wxHAS_LONG_LONG_T_DIFFERENT_FROM_LONG - CPPUNIT_TEST( DoubleFromString ); - CPPUNIT_TEST_SUITE_END(); - - void LongToString(); -#ifdef wxHAS_LONG_LONG_T_DIFFERENT_FROM_LONG - void LongLongToString(); -#endif // wxHAS_LONG_LONG_T_DIFFERENT_FROM_LONG - void DoubleToString(); - void NoTrailingZeroes(); - void LongFromString(); -#ifdef wxHAS_LONG_LONG_T_DIFFERENT_FROM_LONG - void LongLongFromString(); -#endif // wxHAS_LONG_LONG_T_DIFFERENT_FROM_LONG - void DoubleFromString(); - - wxLocale *m_locale; + wxLocale m_locale; wxDECLARE_NO_COPY_CLASS(NumFormatterTestCase); }; -// register in the unnamed registry so that these tests are run by default -CPPUNIT_TEST_SUITE_REGISTRATION( NumFormatterTestCase ); - -// also include in its own registry so that these tests can be run alone -CPPUNIT_TEST_SUITE_NAMED_REGISTRATION( NumFormatterTestCase, "NumFormatterTestCase" ); - // ---------------------------------------------------------------------------- // tests themselves // ---------------------------------------------------------------------------- -void NumFormatterTestCase::LongToString() +TEST_CASE_METHOD(NumFormatterTestCase, "NumFormatter::LongToString", "[numformatter]") { - if ( !m_locale ) + if ( !CanRunTest() ) return; CPPUNIT_ASSERT_EQUAL( "1", wxNumberFormatter::ToString( 1L)); @@ -109,9 +69,9 @@ void NumFormatterTestCase::LongToString() #ifdef wxHAS_LONG_LONG_T_DIFFERENT_FROM_LONG -void NumFormatterTestCase::LongLongToString() +TEST_CASE_METHOD(NumFormatterTestCase, "NumFormatter::LongLongToString", "[numformatter]") { - if ( !m_locale ) + if ( !CanRunTest() ) return; CPPUNIT_ASSERT_EQUAL( "1", wxNumberFormatter::ToString(wxLL( 1))); @@ -127,9 +87,9 @@ void NumFormatterTestCase::LongLongToString() #endif // wxHAS_LONG_LONG_T_DIFFERENT_FROM_LONG -void NumFormatterTestCase::DoubleToString() +TEST_CASE_METHOD(NumFormatterTestCase, "NumFormatter::DoubleToString", "[numformatter]") { - if ( !m_locale ) + if ( !CanRunTest() ) return; CPPUNIT_ASSERT_EQUAL("1.0", wxNumberFormatter::ToString(1., 1)); @@ -154,14 +114,14 @@ void NumFormatterTestCase::DoubleToString() wxNumberFormatter::ToString(-0.02, 1, wxNumberFormatter::Style_None)); } -void NumFormatterTestCase::NoTrailingZeroes() +TEST_CASE_METHOD(NumFormatterTestCase, "NumFormatter::NoTrailingZeroes", "[numformatter]") { WX_ASSERT_FAILS_WITH_ASSERT ( wxNumberFormatter::ToString(123L, wxNumberFormatter::Style_NoTrailingZeroes) ); - if ( !m_locale ) + if ( !CanRunTest() ) return; CPPUNIT_ASSERT_EQUAL @@ -231,9 +191,9 @@ void NumFormatterTestCase::NoTrailingZeroes() ); } -void NumFormatterTestCase::LongFromString() +TEST_CASE_METHOD(NumFormatterTestCase, "NumFormatter::LongFromString", "[numformatter]") { - if ( !m_locale ) + if ( !CanRunTest() ) return; WX_ASSERT_FAILS_WITH_ASSERT @@ -267,9 +227,9 @@ void NumFormatterTestCase::LongFromString() #ifdef wxHAS_LONG_LONG_T_DIFFERENT_FROM_LONG -void NumFormatterTestCase::LongLongFromString() +TEST_CASE_METHOD(NumFormatterTestCase, "NumFormatter::LongLongFromString", "[numformatter]") { - if ( !m_locale ) + if ( !CanRunTest() ) return; WX_ASSERT_FAILS_WITH_ASSERT @@ -303,9 +263,9 @@ void NumFormatterTestCase::LongLongFromString() #endif // wxHAS_LONG_LONG_T_DIFFERENT_FROM_LONG -void NumFormatterTestCase::DoubleFromString() +TEST_CASE_METHOD(NumFormatterTestCase, "NumFormatter::DoubleFromString", "[numformatter]") { - if ( !m_locale ) + if ( !CanRunTest() ) return; WX_ASSERT_FAILS_WITH_ASSERT From a015270709bed84ea894b9d02c61ec1cee9da9f6 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 21 Feb 2021 00:33:22 +0100 Subject: [PATCH 07/16] Replace CPPUNIT_ASSERT() macros in wxNumberFormatter unit tests Just use CHECK() rather than CPPUNIT_ASSERT_EQUAL() etc. --- tests/strings/numformatter.cpp | 263 ++++++++++++++------------------- 1 file changed, 112 insertions(+), 151 deletions(-) diff --git a/tests/strings/numformatter.cpp b/tests/strings/numformatter.cpp index 4ce8e3c449..867e977a26 100644 --- a/tests/strings/numformatter.cpp +++ b/tests/strings/numformatter.cpp @@ -48,23 +48,23 @@ TEST_CASE_METHOD(NumFormatterTestCase, "NumFormatter::LongToString", "[numformat if ( !CanRunTest() ) return; - CPPUNIT_ASSERT_EQUAL( "1", wxNumberFormatter::ToString( 1L)); - CPPUNIT_ASSERT_EQUAL( "-1", wxNumberFormatter::ToString( -1L)); - CPPUNIT_ASSERT_EQUAL( "12", wxNumberFormatter::ToString( 12L)); - CPPUNIT_ASSERT_EQUAL( "-12", wxNumberFormatter::ToString( -12L)); - CPPUNIT_ASSERT_EQUAL( "123", wxNumberFormatter::ToString( 123L)); - CPPUNIT_ASSERT_EQUAL( "-123", wxNumberFormatter::ToString( -123L)); - CPPUNIT_ASSERT_EQUAL( "1,234", wxNumberFormatter::ToString( 1234L)); - CPPUNIT_ASSERT_EQUAL( "-1,234", wxNumberFormatter::ToString( -1234L)); - CPPUNIT_ASSERT_EQUAL( "12,345", wxNumberFormatter::ToString( 12345L)); - CPPUNIT_ASSERT_EQUAL( "-12,345", wxNumberFormatter::ToString( -12345L)); - CPPUNIT_ASSERT_EQUAL( "123,456", wxNumberFormatter::ToString( 123456L)); - CPPUNIT_ASSERT_EQUAL( "-123,456", wxNumberFormatter::ToString( -123456L)); - CPPUNIT_ASSERT_EQUAL( "1,234,567", wxNumberFormatter::ToString( 1234567L)); - CPPUNIT_ASSERT_EQUAL( "-1,234,567", wxNumberFormatter::ToString( -1234567L)); - CPPUNIT_ASSERT_EQUAL( "12,345,678", wxNumberFormatter::ToString( 12345678L)); - CPPUNIT_ASSERT_EQUAL("-12,345,678", wxNumberFormatter::ToString( -12345678L)); - CPPUNIT_ASSERT_EQUAL("123,456,789", wxNumberFormatter::ToString( 123456789L)); + CHECK( wxNumberFormatter::ToString( 1L) == "1" ); + CHECK( wxNumberFormatter::ToString( -1L) == "-1" ); + CHECK( wxNumberFormatter::ToString( 12L) == "12" ); + CHECK( wxNumberFormatter::ToString( -12L) == "-12" ); + CHECK( wxNumberFormatter::ToString( 123L) == "123" ); + CHECK( wxNumberFormatter::ToString( -123L) == "-123" ); + CHECK( wxNumberFormatter::ToString( 1234L) == "1,234" ); + CHECK( wxNumberFormatter::ToString( -1234L) == "-1,234" ); + CHECK( wxNumberFormatter::ToString( 12345L) == "12,345" ); + CHECK( wxNumberFormatter::ToString( -12345L) == "-12,345" ); + CHECK( wxNumberFormatter::ToString( 123456L) == "123,456" ); + CHECK( wxNumberFormatter::ToString( -123456L) == "-123,456" ); + CHECK( wxNumberFormatter::ToString( 1234567L) == "1,234,567" ); + CHECK( wxNumberFormatter::ToString( -1234567L) == "-1,234,567" ); + CHECK( wxNumberFormatter::ToString( 12345678L) == "12,345,678" ); + CHECK( wxNumberFormatter::ToString( -12345678L) == "-12,345,678" ); + CHECK( wxNumberFormatter::ToString( 123456789L) == "123,456,789" ); } #ifdef wxHAS_LONG_LONG_T_DIFFERENT_FROM_LONG @@ -74,15 +74,15 @@ TEST_CASE_METHOD(NumFormatterTestCase, "NumFormatter::LongLongToString", "[numfo if ( !CanRunTest() ) return; - CPPUNIT_ASSERT_EQUAL( "1", wxNumberFormatter::ToString(wxLL( 1))); - CPPUNIT_ASSERT_EQUAL( "12", wxNumberFormatter::ToString(wxLL( 12))); - CPPUNIT_ASSERT_EQUAL( "123", wxNumberFormatter::ToString(wxLL( 123))); - CPPUNIT_ASSERT_EQUAL( "1,234", wxNumberFormatter::ToString(wxLL( 1234))); - CPPUNIT_ASSERT_EQUAL( "12,345", wxNumberFormatter::ToString(wxLL( 12345))); - CPPUNIT_ASSERT_EQUAL( "123,456", wxNumberFormatter::ToString(wxLL( 123456))); - CPPUNIT_ASSERT_EQUAL( "1,234,567", wxNumberFormatter::ToString(wxLL( 1234567))); - CPPUNIT_ASSERT_EQUAL( "12,345,678", wxNumberFormatter::ToString(wxLL( 12345678))); - CPPUNIT_ASSERT_EQUAL("123,456,789", wxNumberFormatter::ToString(wxLL( 123456789))); + CHECK( wxNumberFormatter::ToString(wxLL( 1)) == "1" ); + CHECK( wxNumberFormatter::ToString(wxLL( 12)) == "12" ); + CHECK( wxNumberFormatter::ToString(wxLL( 123)) == "123" ); + CHECK( wxNumberFormatter::ToString(wxLL( 1234)) == "1,234" ); + CHECK( wxNumberFormatter::ToString(wxLL( 12345)) == "12,345" ); + CHECK( wxNumberFormatter::ToString(wxLL( 123456)) == "123,456" ); + CHECK( wxNumberFormatter::ToString(wxLL( 1234567)) == "1,234,567" ); + CHECK( wxNumberFormatter::ToString(wxLL( 12345678)) == "12,345,678" ); + CHECK( wxNumberFormatter::ToString(wxLL( 123456789)) == "123,456,789" ); } #endif // wxHAS_LONG_LONG_T_DIFFERENT_FROM_LONG @@ -92,26 +92,22 @@ TEST_CASE_METHOD(NumFormatterTestCase, "NumFormatter::DoubleToString", "[numform if ( !CanRunTest() ) return; - CPPUNIT_ASSERT_EQUAL("1.0", wxNumberFormatter::ToString(1., 1)); - CPPUNIT_ASSERT_EQUAL("0.123456", wxNumberFormatter::ToString(0.123456, 6)); - CPPUNIT_ASSERT_EQUAL("1.234567", wxNumberFormatter::ToString(1.234567, 6)); - CPPUNIT_ASSERT_EQUAL("12.34567", wxNumberFormatter::ToString(12.34567, 5)); - CPPUNIT_ASSERT_EQUAL("123.4567", wxNumberFormatter::ToString(123.4567, 4)); - CPPUNIT_ASSERT_EQUAL("1,234.56", wxNumberFormatter::ToString(1234.56, 2)); - CPPUNIT_ASSERT_EQUAL("12,345.6", wxNumberFormatter::ToString(12345.6, 1)); - CPPUNIT_ASSERT_EQUAL("12,345.6", wxNumberFormatter::ToString(12345.6, 1)); - CPPUNIT_ASSERT_EQUAL("123,456,789.0", - wxNumberFormatter::ToString(123456789., 1)); - CPPUNIT_ASSERT_EQUAL("123,456,789.012", - wxNumberFormatter::ToString(123456789.012, 3)); - CPPUNIT_ASSERT_EQUAL("12,345", - wxNumberFormatter::ToString(12345.012, -1)); - CPPUNIT_ASSERT_EQUAL("-123.1230", - wxNumberFormatter::ToString(-123.123, 4, wxNumberFormatter::Style_None)); - CPPUNIT_ASSERT_EQUAL("0.0", - wxNumberFormatter::ToString(0.02, 1, wxNumberFormatter::Style_None)); - CPPUNIT_ASSERT_EQUAL("-0.0", - wxNumberFormatter::ToString(-0.02, 1, wxNumberFormatter::Style_None)); + CHECK( wxNumberFormatter::ToString(1., 1) == "1.0" ); + CHECK( wxNumberFormatter::ToString(0.123456, 6) == "0.123456" ); + CHECK( wxNumberFormatter::ToString(1.234567, 6) == "1.234567" ); + CHECK( wxNumberFormatter::ToString(12.34567, 5) == "12.34567" ); + CHECK( wxNumberFormatter::ToString(123.4567, 4) == "123.4567" ); + CHECK( wxNumberFormatter::ToString(1234.56, 2) == "1,234.56" ); + CHECK( wxNumberFormatter::ToString(12345.6, 1) == "12,345.6" ); + CHECK( wxNumberFormatter::ToString(123456789.012, 3) == "123,456,789.012" ); + CHECK( wxNumberFormatter::ToString(12345.012, -1) == "12,345" ); + + CHECK( wxNumberFormatter::ToString(-123.123, 4, wxNumberFormatter::Style_None) + == "-123.1230" ); + CHECK( wxNumberFormatter::ToString( 0.02, 1, wxNumberFormatter::Style_None) + == "0.0" ); + CHECK( wxNumberFormatter::ToString(-0.02, 1, wxNumberFormatter::Style_None) + == "-0.0" ); } TEST_CASE_METHOD(NumFormatterTestCase, "NumFormatter::NoTrailingZeroes", "[numformatter]") @@ -124,71 +120,36 @@ TEST_CASE_METHOD(NumFormatterTestCase, "NumFormatter::NoTrailingZeroes", "[numfo if ( !CanRunTest() ) return; - CPPUNIT_ASSERT_EQUAL - ( - "123.000", - wxNumberFormatter::ToString(123., 3) - ); + CHECK( wxNumberFormatter::ToString(123., 3) == "123.000" ); - CPPUNIT_ASSERT_EQUAL - ( - "123", - wxNumberFormatter::ToString(123., 3, wxNumberFormatter::Style_NoTrailingZeroes) - ); + CHECK( wxNumberFormatter::ToString(123., 3, wxNumberFormatter::Style_NoTrailingZeroes) + == "123" ); - CPPUNIT_ASSERT_EQUAL - ( - "123", - wxNumberFormatter::ToString(123., 9, wxNumberFormatter::Style_NoTrailingZeroes) - ); + CHECK( wxNumberFormatter::ToString(123., 9, wxNumberFormatter::Style_NoTrailingZeroes) + == "123" ); - CPPUNIT_ASSERT_EQUAL - ( - "123.456", - wxNumberFormatter::ToString(123.456, 3, wxNumberFormatter::Style_NoTrailingZeroes) - ); + CHECK( wxNumberFormatter::ToString(123.456, 3, wxNumberFormatter::Style_NoTrailingZeroes) + == "123.456" ); - CPPUNIT_ASSERT_EQUAL - ( - "123.456000000", - wxNumberFormatter::ToString(123.456, 9) - ); + CHECK( wxNumberFormatter::ToString(123.456, 9) == "123.456000000" ); - CPPUNIT_ASSERT_EQUAL - ( - "123.456", - wxNumberFormatter::ToString(123.456, 9, wxNumberFormatter::Style_NoTrailingZeroes) - ); + CHECK( wxNumberFormatter::ToString(123.456, 9, wxNumberFormatter::Style_NoTrailingZeroes) + == "123.456" ); - CPPUNIT_ASSERT_EQUAL - ( - "123.12", - wxNumberFormatter::ToString(123.123, 2, wxNumberFormatter::Style_NoTrailingZeroes) - ); + CHECK( wxNumberFormatter::ToString(123.123, 2, wxNumberFormatter::Style_NoTrailingZeroes) + == "123.12" ); - CPPUNIT_ASSERT_EQUAL - ( - "123", - wxNumberFormatter::ToString(123.123, 0, wxNumberFormatter::Style_NoTrailingZeroes) - ); + CHECK( wxNumberFormatter::ToString(123.123, 0, wxNumberFormatter::Style_NoTrailingZeroes) + == "123" ); - CPPUNIT_ASSERT_EQUAL - ( - "0", - wxNumberFormatter::ToString(-0.000123, 3, wxNumberFormatter::Style_NoTrailingZeroes) - ); + CHECK( wxNumberFormatter::ToString(-0.000123, 3, wxNumberFormatter::Style_NoTrailingZeroes) + == "0" ); - CPPUNIT_ASSERT_EQUAL - ( - "123", - wxNumberFormatter::ToString(123., -1, wxNumberFormatter::Style_NoTrailingZeroes) - ); + CHECK( wxNumberFormatter::ToString(123., -1, wxNumberFormatter::Style_NoTrailingZeroes) + == "123" ); - CPPUNIT_ASSERT_EQUAL - ( - "1e-120", - wxNumberFormatter::ToString(1e-120, -1, wxNumberFormatter::Style_NoTrailingZeroes) - ); + CHECK( wxNumberFormatter::ToString(1e-120, -1, wxNumberFormatter::Style_NoTrailingZeroes) + == "1e-120" ); } TEST_CASE_METHOD(NumFormatterTestCase, "NumFormatter::LongFromString", "[numformatter]") @@ -202,27 +163,27 @@ TEST_CASE_METHOD(NumFormatterTestCase, "NumFormatter::LongFromString", "[numform ); long l; - CPPUNIT_ASSERT( !wxNumberFormatter::FromString("", &l) ); - CPPUNIT_ASSERT( !wxNumberFormatter::FromString("foo", &l) ); - CPPUNIT_ASSERT( !wxNumberFormatter::FromString("1.234", &l) ); + CHECK( !wxNumberFormatter::FromString("", &l) ); + CHECK( !wxNumberFormatter::FromString("foo", &l) ); + CHECK( !wxNumberFormatter::FromString("1.234", &l) ); - CPPUNIT_ASSERT( wxNumberFormatter::FromString("123", &l) ); - CPPUNIT_ASSERT_EQUAL( 123, l ); + CHECK( wxNumberFormatter::FromString("123", &l) ); + CHECK( l == 123 ); - CPPUNIT_ASSERT( wxNumberFormatter::FromString("1234", &l) ); - CPPUNIT_ASSERT_EQUAL( 1234, l ); + CHECK( wxNumberFormatter::FromString("1234", &l) ); + CHECK( l == 1234 ); - CPPUNIT_ASSERT( wxNumberFormatter::FromString("1,234", &l) ); - CPPUNIT_ASSERT_EQUAL( 1234, l ); + CHECK( wxNumberFormatter::FromString("1,234", &l) ); + CHECK( l == 1234 ); - CPPUNIT_ASSERT( wxNumberFormatter::FromString("12,345", &l) ); - CPPUNIT_ASSERT_EQUAL( 12345, l ); + CHECK( wxNumberFormatter::FromString("12,345", &l) ); + CHECK( l == 12345 ); - CPPUNIT_ASSERT( wxNumberFormatter::FromString("123,456", &l) ); - CPPUNIT_ASSERT_EQUAL( 123456, l ); + CHECK( wxNumberFormatter::FromString("123,456", &l) ); + CHECK( l == 123456 ); - CPPUNIT_ASSERT( wxNumberFormatter::FromString("1,234,567", &l) ); - CPPUNIT_ASSERT_EQUAL( 1234567, l ); + CHECK( wxNumberFormatter::FromString("1,234,567", &l) ); + CHECK( l == 1234567 ); } #ifdef wxHAS_LONG_LONG_T_DIFFERENT_FROM_LONG @@ -238,27 +199,27 @@ TEST_CASE_METHOD(NumFormatterTestCase, "NumFormatter::LongLongFromString", "[num ); wxLongLong_t l; - CPPUNIT_ASSERT( !wxNumberFormatter::FromString("", &l) ); - CPPUNIT_ASSERT( !wxNumberFormatter::FromString("foo", &l) ); - CPPUNIT_ASSERT( !wxNumberFormatter::FromString("1.234", &l) ); + CHECK( !wxNumberFormatter::FromString("", &l) ); + CHECK( !wxNumberFormatter::FromString("foo", &l) ); + CHECK( !wxNumberFormatter::FromString("1.234", &l) ); - CPPUNIT_ASSERT( wxNumberFormatter::FromString("123", &l) ); - CPPUNIT_ASSERT_EQUAL( 123, l ); + CHECK( wxNumberFormatter::FromString("123", &l) ); + CHECK( l == 123 ); - CPPUNIT_ASSERT( wxNumberFormatter::FromString("1234", &l) ); - CPPUNIT_ASSERT_EQUAL( 1234, l ); + CHECK( wxNumberFormatter::FromString("1234", &l) ); + CHECK( l == 1234 ); - CPPUNIT_ASSERT( wxNumberFormatter::FromString("1,234", &l) ); - CPPUNIT_ASSERT_EQUAL( 1234, l ); + CHECK( wxNumberFormatter::FromString("1,234", &l) ); + CHECK( l == 1234 ); - CPPUNIT_ASSERT( wxNumberFormatter::FromString("12,345", &l) ); - CPPUNIT_ASSERT_EQUAL( 12345, l ); + CHECK( wxNumberFormatter::FromString("12,345", &l) ); + CHECK( l == 12345 ); - CPPUNIT_ASSERT( wxNumberFormatter::FromString("123,456", &l) ); - CPPUNIT_ASSERT_EQUAL( 123456, l ); + CHECK( wxNumberFormatter::FromString("123,456", &l) ); + CHECK( l == 123456 ); - CPPUNIT_ASSERT( wxNumberFormatter::FromString("1,234,567", &l) ); - CPPUNIT_ASSERT_EQUAL( 1234567, l ); + CHECK( wxNumberFormatter::FromString("1,234,567", &l) ); + CHECK( l == 1234567 ); } #endif // wxHAS_LONG_LONG_T_DIFFERENT_FROM_LONG @@ -274,33 +235,33 @@ TEST_CASE_METHOD(NumFormatterTestCase, "NumFormatter::DoubleFromString", "[numfo ); double d; - CPPUNIT_ASSERT( !wxNumberFormatter::FromString("", &d) ); - CPPUNIT_ASSERT( !wxNumberFormatter::FromString("bar", &d) ); + CHECK( !wxNumberFormatter::FromString("", &d) ); + CHECK( !wxNumberFormatter::FromString("bar", &d) ); - CPPUNIT_ASSERT( wxNumberFormatter::FromString("123", &d) ); - CPPUNIT_ASSERT_EQUAL( 123., d ); + CHECK( wxNumberFormatter::FromString("123", &d) ); + CHECK( d == 123. ); - CPPUNIT_ASSERT( wxNumberFormatter::FromString("123.456789012", &d) ); - CPPUNIT_ASSERT_EQUAL( 123.456789012, d ); + CHECK( wxNumberFormatter::FromString("123.456789012", &d) ); + CHECK( d == 123.456789012 ); - CPPUNIT_ASSERT( wxNumberFormatter::FromString("1,234.56789012", &d) ); - CPPUNIT_ASSERT_EQUAL( 1234.56789012, d ); + CHECK( wxNumberFormatter::FromString("1,234.56789012", &d) ); + CHECK( d == 1234.56789012 ); - CPPUNIT_ASSERT( wxNumberFormatter::FromString("12,345.6789012", &d) ); - CPPUNIT_ASSERT_EQUAL( 12345.6789012, d ); + CHECK( wxNumberFormatter::FromString("12,345.6789012", &d) ); + CHECK( d == 12345.6789012 ); - CPPUNIT_ASSERT( wxNumberFormatter::FromString("123,456.789012", &d) ); - CPPUNIT_ASSERT_EQUAL( 123456.789012, d ); + CHECK( wxNumberFormatter::FromString("123,456.789012", &d) ); + CHECK( d == 123456.789012 ); - CPPUNIT_ASSERT( wxNumberFormatter::FromString("1,234,567.89012", &d) ); - CPPUNIT_ASSERT_EQUAL( 1234567.89012, d ); + CHECK( wxNumberFormatter::FromString("1,234,567.89012", &d) ); + CHECK( d == 1234567.89012 ); - CPPUNIT_ASSERT( wxNumberFormatter::FromString("12,345,678.9012", &d) ); - CPPUNIT_ASSERT_EQUAL( 12345678.9012, d ); + CHECK( wxNumberFormatter::FromString("12,345,678.9012", &d) ); + CHECK( d == 12345678.9012 ); - CPPUNIT_ASSERT( wxNumberFormatter::FromString("123,456,789.012", &d) ); - CPPUNIT_ASSERT_EQUAL( 123456789.012, d ); + CHECK( wxNumberFormatter::FromString("123,456,789.012", &d) ); + CHECK( d == 123456789.012 ); - CPPUNIT_ASSERT( wxNumberFormatter::FromString("123456789.012", &d) ); - CPPUNIT_ASSERT_EQUAL( 123456789.012, d ); + CHECK( wxNumberFormatter::FromString("123456789.012", &d) ); + CHECK( d == 123456789.012 ); } From 959d955a80fc3f4dba68dc175599cae89481b0d2 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 21 Feb 2021 17:10:31 +0100 Subject: [PATCH 08/16] Make wxNumberFormatter tests more concise and readable Define a couple of helper functions to avoid over long lines or having to break them and also align the tests vertically to allow scanning them more easily. No real changes. --- tests/strings/numformatter.cpp | 94 ++++++++++++++++++---------------- 1 file changed, 49 insertions(+), 45 deletions(-) diff --git a/tests/strings/numformatter.cpp b/tests/strings/numformatter.cpp index 867e977a26..d93f6c3244 100644 --- a/tests/strings/numformatter.cpp +++ b/tests/strings/numformatter.cpp @@ -20,6 +20,9 @@ // test class // ---------------------------------------------------------------------------- +namespace +{ + class NumFormatterTestCase { public: @@ -39,6 +42,29 @@ private: wxDECLARE_NO_COPY_CLASS(NumFormatterTestCase); }; +// A couple of helpers to avoid writing over long expressions. +wxString ToStringWithoutTrailingZeroes(double val, int precision) +{ + return wxNumberFormatter::ToString + ( + val, + precision, + wxNumberFormatter::Style_NoTrailingZeroes + ); +} + +wxString ToStringWithTrailingZeroes(double val, int precision) +{ + return wxNumberFormatter::ToString + ( + val, + precision, + wxNumberFormatter::Style_None + ); +} + +} // anonymous namespace + // ---------------------------------------------------------------------------- // tests themselves // ---------------------------------------------------------------------------- @@ -92,22 +118,19 @@ TEST_CASE_METHOD(NumFormatterTestCase, "NumFormatter::DoubleToString", "[numform if ( !CanRunTest() ) return; - CHECK( wxNumberFormatter::ToString(1., 1) == "1.0" ); - CHECK( wxNumberFormatter::ToString(0.123456, 6) == "0.123456" ); - CHECK( wxNumberFormatter::ToString(1.234567, 6) == "1.234567" ); - CHECK( wxNumberFormatter::ToString(12.34567, 5) == "12.34567" ); - CHECK( wxNumberFormatter::ToString(123.4567, 4) == "123.4567" ); - CHECK( wxNumberFormatter::ToString(1234.56, 2) == "1,234.56" ); - CHECK( wxNumberFormatter::ToString(12345.6, 1) == "12,345.6" ); - CHECK( wxNumberFormatter::ToString(123456789.012, 3) == "123,456,789.012" ); - CHECK( wxNumberFormatter::ToString(12345.012, -1) == "12,345" ); + CHECK( wxNumberFormatter::ToString( 1., 1) == "1.0" ); + CHECK( wxNumberFormatter::ToString( 0.123456, 6) == "0.123456" ); + CHECK( wxNumberFormatter::ToString( 1.234567, 6) == "1.234567" ); + CHECK( wxNumberFormatter::ToString( 12.34567, 5) == "12.34567" ); + CHECK( wxNumberFormatter::ToString( 123.4567, 4) == "123.4567" ); + CHECK( wxNumberFormatter::ToString( 1234.56, 2) == "1,234.56" ); + CHECK( wxNumberFormatter::ToString( 12345.6, 1) == "12,345.6" ); + CHECK( wxNumberFormatter::ToString(123456789.012, 3) == "123,456,789.012" ); + CHECK( wxNumberFormatter::ToString( 12345.012, -1) == "12,345" ); - CHECK( wxNumberFormatter::ToString(-123.123, 4, wxNumberFormatter::Style_None) - == "-123.1230" ); - CHECK( wxNumberFormatter::ToString( 0.02, 1, wxNumberFormatter::Style_None) - == "0.0" ); - CHECK( wxNumberFormatter::ToString(-0.02, 1, wxNumberFormatter::Style_None) - == "-0.0" ); + CHECK( ToStringWithTrailingZeroes(-123.123, 4) == "-123.1230" ); + CHECK( ToStringWithTrailingZeroes( 0.02, 1) == "0.0" ); + CHECK( ToStringWithTrailingZeroes( -0.02, 1) == "-0.0" ); } TEST_CASE_METHOD(NumFormatterTestCase, "NumFormatter::NoTrailingZeroes", "[numformatter]") @@ -120,36 +143,17 @@ TEST_CASE_METHOD(NumFormatterTestCase, "NumFormatter::NoTrailingZeroes", "[numfo if ( !CanRunTest() ) return; - CHECK( wxNumberFormatter::ToString(123., 3) == "123.000" ); - - CHECK( wxNumberFormatter::ToString(123., 3, wxNumberFormatter::Style_NoTrailingZeroes) - == "123" ); - - CHECK( wxNumberFormatter::ToString(123., 9, wxNumberFormatter::Style_NoTrailingZeroes) - == "123" ); - - CHECK( wxNumberFormatter::ToString(123.456, 3, wxNumberFormatter::Style_NoTrailingZeroes) - == "123.456" ); - - CHECK( wxNumberFormatter::ToString(123.456, 9) == "123.456000000" ); - - CHECK( wxNumberFormatter::ToString(123.456, 9, wxNumberFormatter::Style_NoTrailingZeroes) - == "123.456" ); - - CHECK( wxNumberFormatter::ToString(123.123, 2, wxNumberFormatter::Style_NoTrailingZeroes) - == "123.12" ); - - CHECK( wxNumberFormatter::ToString(123.123, 0, wxNumberFormatter::Style_NoTrailingZeroes) - == "123" ); - - CHECK( wxNumberFormatter::ToString(-0.000123, 3, wxNumberFormatter::Style_NoTrailingZeroes) - == "0" ); - - CHECK( wxNumberFormatter::ToString(123., -1, wxNumberFormatter::Style_NoTrailingZeroes) - == "123" ); - - CHECK( wxNumberFormatter::ToString(1e-120, -1, wxNumberFormatter::Style_NoTrailingZeroes) - == "1e-120" ); + CHECK( ToStringWithTrailingZeroes ( 123., 3) == "123.000" ); + CHECK( ToStringWithoutTrailingZeroes( 123., 3) == "123" ); + CHECK( ToStringWithoutTrailingZeroes( 123., 9) == "123" ); + CHECK( ToStringWithoutTrailingZeroes( 123.456, 3) == "123.456" ); + CHECK( ToStringWithTrailingZeroes ( 123.456, 9) == "123.456000000" ); + CHECK( ToStringWithoutTrailingZeroes( 123.456, 9) == "123.456" ); + CHECK( ToStringWithoutTrailingZeroes( 123.123, 2) == "123.12" ); + CHECK( ToStringWithoutTrailingZeroes( 123.123, 0) == "123" ); + CHECK( ToStringWithoutTrailingZeroes( -0.000123, 3) == "0" ); + CHECK( ToStringWithoutTrailingZeroes( 123., -1) == "123" ); + CHECK( ToStringWithoutTrailingZeroes( 1e-120, -1) == "1e-120" ); } TEST_CASE_METHOD(NumFormatterTestCase, "NumFormatter::LongFromString", "[numformatter]") From eb64202ad420deca7eb91d9d2d83a7a46583da52 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 21 Feb 2021 17:28:02 +0100 Subject: [PATCH 09/16] Add support for unsigned long long to wxNumberFormatter This is necessary in order to deal with the numbers greater than wxINT64_MAX that can't be represented in just long long. It also allows to implement the intuitive handling of minus sign for the unsigned numbers, i.e. not to accept it in FromString(), unlike the standard functions which do (and parse -1 as 0xffff...fff). Also extend the tests to check for more boundary cases. --- include/wx/numformatter.h | 3 ++ interface/wx/numformatter.h | 8 +++++ src/common/numformatter.cpp | 27 ++++++++++++++++ tests/strings/numformatter.cpp | 57 ++++++++++++++++++++++++++++++++++ 4 files changed, 95 insertions(+) diff --git a/include/wx/numformatter.h b/include/wx/numformatter.h index 13b47b210b..24ab702ab7 100644 --- a/include/wx/numformatter.h +++ b/include/wx/numformatter.h @@ -34,6 +34,8 @@ public: static wxString ToString(wxLongLong_t val, int style = Style_WithThousandsSep); #endif // wxHAS_LONG_LONG_T_DIFFERENT_FROM_LONG + static wxString ToString(wxULongLong_t val, + int style = Style_WithThousandsSep); static wxString ToString(double val, int precision, int style = Style_WithThousandsSep); @@ -46,6 +48,7 @@ public: #ifdef wxHAS_LONG_LONG_T_DIFFERENT_FROM_LONG static bool FromString(wxString s, wxLongLong_t *val); #endif // wxHAS_LONG_LONG_T_DIFFERENT_FROM_LONG + static bool FromString(wxString s, wxULongLong_t *val); static bool FromString(wxString s, double *val); diff --git a/interface/wx/numformatter.h b/interface/wx/numformatter.h index c6e0eaffe2..1479bae1c4 100644 --- a/interface/wx/numformatter.h +++ b/interface/wx/numformatter.h @@ -73,6 +73,7 @@ public: //@{ static wxString ToString(long val, int flags = Style_WithThousandsSep); static wxString ToString(long long val, int flags = Style_WithThousandsSep); + static wxString ToString(unsigned long long val, int flags = Style_WithThousandsSep); //@} /** @@ -97,11 +98,18 @@ public: success they return @true and store the result at the location pointed to by @a val (which can't be @NULL), otherwise @false is returned. + Note that the overload taking unsigned long long value is only + available since wxWidgets 3.1.5. Also, unlike wxString::ToULongLong() + and the standard functions such as @c strtoul(), this overload does + @em not accept, i.e. returns @false, for the strings starting with the + minus sign. + @see wxString::ToLong(), wxString::ToDouble() */ //@{ static bool FromString(wxString s, long *val); static bool FromString(wxString s, long long *val); + static bool FromString(wxString s, unsigned long long *val); static bool FromString(wxString s, double *val); //@} diff --git a/src/common/numformatter.cpp b/src/common/numformatter.cpp index 1bf8497712..e57ab02335 100644 --- a/src/common/numformatter.cpp +++ b/src/common/numformatter.cpp @@ -198,6 +198,12 @@ wxString wxNumberFormatter::ToString(wxLongLong_t val, int style) #endif // wxHAS_LONG_LONG_T_DIFFERENT_FROM_LONG +wxString wxNumberFormatter::ToString(wxULongLong_t val, int style) +{ + return PostProcessIntString(wxString::Format("%" wxLongLongFmtSpec "u", val), + style); +} + wxString wxNumberFormatter::ToString(double val, int precision, int style) { wxString s = wxString::FromDouble(val,precision); @@ -300,6 +306,27 @@ bool wxNumberFormatter::FromString(wxString s, wxLongLong_t *val) #endif // wxHAS_LONG_LONG_T_DIFFERENT_FROM_LONG +bool wxNumberFormatter::FromString(wxString s, wxULongLong_t *val) +{ + RemoveThousandsSeparators(s); + + // wxString::ToULongLong() does accept minus sign for unsigned integers, + // consistently with the standard functions behaviour, e.g. strtoul() does + // the same thing, but here we really want to accept the "true" unsigned + // numbers only, so check for leading minus, possibly preceded by some + // whitespace. + for ( wxString::const_iterator it = s.begin(); it != s.end(); ++it ) + { + if ( *it == '-' ) + return false; + + if ( *it != ' ' && *it != '\t' ) + break; + } + + return s.ToULongLong(val); +} + bool wxNumberFormatter::FromString(wxString s, double *val) { RemoveThousandsSeparators(s); diff --git a/tests/strings/numformatter.cpp b/tests/strings/numformatter.cpp index d93f6c3244..b9445190dc 100644 --- a/tests/strings/numformatter.cpp +++ b/tests/strings/numformatter.cpp @@ -206,6 +206,10 @@ TEST_CASE_METHOD(NumFormatterTestCase, "NumFormatter::LongLongFromString", "[num CHECK( !wxNumberFormatter::FromString("", &l) ); CHECK( !wxNumberFormatter::FromString("foo", &l) ); CHECK( !wxNumberFormatter::FromString("1.234", &l) ); + CHECK( !wxNumberFormatter::FromString("-", &l) ); + + CHECK( wxNumberFormatter::FromString("0", &l) ); + CHECK( l == 0 ); CHECK( wxNumberFormatter::FromString("123", &l) ); CHECK( l == 123 ); @@ -224,10 +228,63 @@ TEST_CASE_METHOD(NumFormatterTestCase, "NumFormatter::LongLongFromString", "[num CHECK( wxNumberFormatter::FromString("1,234,567", &l) ); CHECK( l == 1234567 ); + + CHECK( wxNumberFormatter::FromString("-123", &l) ); + CHECK( l == -123 ); + + CHECK( wxNumberFormatter::FromString("9223372036854775807", &l) ); + CHECK( l == wxINT64_MAX ); + + CHECK( !wxNumberFormatter::FromString("9223372036854775808", &l) ); } #endif // wxHAS_LONG_LONG_T_DIFFERENT_FROM_LONG +TEST_CASE_METHOD(NumFormatterTestCase, "NumFormatter::ULongLongFromString", "[numformatter]") +{ + if ( !CanRunTest() ) + return; + + wxULongLong_t u; + CHECK( !wxNumberFormatter::FromString("", &u) ); + CHECK( !wxNumberFormatter::FromString("bar", &u) ); + CHECK( !wxNumberFormatter::FromString("1.234", &u) ); + CHECK( !wxNumberFormatter::FromString("-2", &u) ); + CHECK( !wxNumberFormatter::FromString("-", &u) ); + + CHECK( wxNumberFormatter::FromString("0", &u) ); + CHECK( u == 0 ); + + CHECK( wxNumberFormatter::FromString("123", &u) ); + CHECK( u == 123 ); + + CHECK( wxNumberFormatter::FromString("1234", &u) ); + CHECK( u == 1234 ); + + CHECK( wxNumberFormatter::FromString("1,234", &u) ); + CHECK( u == 1234 ); + + CHECK( wxNumberFormatter::FromString("12,345", &u) ); + CHECK( u == 12345 ); + + CHECK( wxNumberFormatter::FromString("123,456", &u) ); + CHECK( u == 123456 ); + + CHECK( wxNumberFormatter::FromString("1,234,567", &u) ); + CHECK( u == 1234567 ); + + CHECK( wxNumberFormatter::FromString("9223372036854775807", &u) ); + CHECK( u == static_cast(wxINT64_MAX) ); + + CHECK( wxNumberFormatter::FromString("9223372036854775808", &u) ); + CHECK( u == static_cast(wxINT64_MAX) + 1 ); + + CHECK( wxNumberFormatter::FromString("18446744073709551615", &u) ); + CHECK( u == wxUINT64_MAX ); + + CHECK( !wxNumberFormatter::FromString("18446744073709551616", &u) ); +} + TEST_CASE_METHOD(NumFormatterTestCase, "NumFormatter::DoubleFromString", "[numformatter]") { if ( !CanRunTest() ) From f1e6af089adaa0b30defd9ec42a60510880f1469 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 21 Feb 2021 19:05:13 +0100 Subject: [PATCH 10/16] Fix parsing unsigned numbers in wxIntegerValidatorBase Use wxNumberFormatter::FromString() overload taking wxULongLong_t to allow parsing numbers greater than LLONG_MAX. --- include/wx/valnum.h | 2 +- src/common/valnum.cpp | 24 ++++++++++++++++++++++-- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/include/wx/valnum.h b/include/wx/valnum.h index c7858dec13..76386debf4 100644 --- a/include/wx/valnum.h +++ b/include/wx/valnum.h @@ -303,7 +303,7 @@ protected: // Provide methods for wxNumValidator use. wxString ToString(LongestValueType value) const; - static bool FromString(const wxString& s, LongestValueType *value); + bool FromString(const wxString& s, LongestValueType *value) const; void DoSetMin(LongestValueType min) { m_min = min; } LongestValueType DoGetMin() const { return m_min; } diff --git a/src/common/valnum.cpp b/src/common/valnum.cpp index 8704e72fce..b4adef753b 100644 --- a/src/common/valnum.cpp +++ b/src/common/valnum.cpp @@ -234,9 +234,29 @@ wxString wxIntegerValidatorBase::ToString(LongestValueType value) const } bool -wxIntegerValidatorBase::FromString(const wxString& s, LongestValueType *value) +wxIntegerValidatorBase::FromString(const wxString& s, + LongestValueType *value) const { - return wxNumberFormatter::FromString(s, value); + if ( CanBeNegative() ) + { + return wxNumberFormatter::FromString(s, value); + } + else + { + // Parse as unsigned to ensure we don't accept minus sign here. +#ifdef wxULongLong_t + wxULongLong_t uvalue; +#else + unsigned long uvalue; +#endif + if ( !wxNumberFormatter::FromString(s, &uvalue) ) + return false; + + // This cast is lossless. + *value = static_cast(uvalue); + + return true; + } } bool From cddc65750509ba66925c1421800306e0d929b6c9 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 21 Feb 2021 19:06:14 +0100 Subject: [PATCH 11/16] Use min/max values of correct type in numeric validators Use the actual type of the value, not LongestValueType, for storing m_min and m_max as this is necessary for the comparisons between the value and them to work correctly for unsigned types. Also check for precision loss when converting from the bigger LongestValueType to the actual type. Add new unit tests, that failed before, but pass now. --- include/wx/valnum.h | 80 +++++++++++++++++++------------------ tests/validators/valnum.cpp | 29 ++++++++++++++ 2 files changed, 71 insertions(+), 38 deletions(-) diff --git a/include/wx/valnum.h b/include/wx/valnum.h index 76386debf4..5305796edd 100644 --- a/include/wx/valnum.h +++ b/include/wx/valnum.h @@ -159,22 +159,22 @@ public: void SetMin(ValueType min) { - this->DoSetMin(static_cast(min)); + m_min = min; } ValueType GetMin() const { - return static_cast(this->DoGetMin()); + return m_min; } void SetMax(ValueType max) { - this->DoSetMax(static_cast(max)); + m_max = max; } ValueType GetMax() const { - return static_cast(this->DoGetMax()); + return m_max; } void SetRange(ValueType min, ValueType max) @@ -243,6 +243,9 @@ protected: : wxString(); } + virtual bool CanBeNegative() const wxOVERRIDE { return m_min < 0; } + + // This member is protected because it can be useful to the derived classes // in their Transfer{From,To}Window() implementations. ValueType * const m_value; @@ -266,6 +269,8 @@ private: return s; } + // Minimal and maximal values accepted (inclusive). + ValueType m_min, m_max; wxDECLARE_NO_ASSIGN_CLASS(wxNumValidator); }; @@ -305,23 +310,12 @@ protected: wxString ToString(LongestValueType value) const; bool FromString(const wxString& s, LongestValueType *value) const; - void DoSetMin(LongestValueType min) { m_min = min; } - LongestValueType DoGetMin() const { return m_min; } - void DoSetMax(LongestValueType max) { m_max = max; } - LongestValueType DoGetMax() const { return m_max; } - - bool IsInRange(LongestValueType value) const - { - return m_min <= value && value <= m_max; - } + virtual bool IsInRange(LongestValueType value) const = 0; // Implement wxNumValidatorBase pure virtual method. virtual bool IsCharOk(const wxString& val, int pos, wxChar ch) const wxOVERRIDE; private: - // Minimal and maximal values accepted (inclusive). - LongestValueType m_min, m_max; - wxDECLARE_NO_ASSIGN_CLASS(wxIntegerValidatorBase); }; @@ -337,6 +331,8 @@ public: typedef wxPrivate::wxNumValidator Base; + typedef + wxIntegerValidatorBase::LongestValueType LongestValueType; // Ctor for an integer validator. // @@ -345,14 +341,29 @@ public: wxIntegerValidator(ValueType *value = NULL, int style = wxNUM_VAL_DEFAULT) : Base(value, style) { - this->DoSetMin(std::numeric_limits::min()); - this->DoSetMax(std::numeric_limits::max()); + this->SetMin(std::numeric_limits::min()); + this->SetMax(std::numeric_limits::max()); } virtual wxObject *Clone() const wxOVERRIDE { return new wxIntegerValidator(*this); } -protected: - virtual bool CanBeNegative() const wxOVERRIDE { return DoGetMin() < 0; } + virtual bool IsInRange(LongestValueType value) const wxOVERRIDE + { + // LongestValueType is used as a container for the values of any type + // which can be used in type-independent wxIntegerValidatorBase code, + // but we need to use the correct type for comparisons, notably for + // comparing unsigned values correctly, so cast to this type and check + // that we don't lose precision while doing it. + const ValueType valueT = static_cast(value); + if ( static_cast(valueT) != value ) + { + // The conversion wasn't lossless, so the value must not be exactly + // representable in this type and so is definitely not in range. + return false; + } + + return this->GetMin() <= valueT && valueT <= this->GetMax(); + } private: wxDECLARE_NO_ASSIGN_CLASS(wxIntegerValidator); @@ -404,15 +415,7 @@ protected: wxString ToString(LongestValueType value) const; bool FromString(const wxString& s, LongestValueType *value) const; - void DoSetMin(LongestValueType min) { m_min = min; } - LongestValueType DoGetMin() const { return m_min; } - void DoSetMax(LongestValueType max) { m_max = max; } - LongestValueType DoGetMax() const { return m_max; } - - bool IsInRange(LongestValueType value) const - { - return m_min <= value && value <= m_max; - } + virtual bool IsInRange(LongestValueType value) const = 0; // Implement wxNumValidatorBase pure virtual method. virtual bool IsCharOk(const wxString& val, int pos, wxChar ch) const wxOVERRIDE; @@ -424,9 +427,6 @@ private: // Factor applied for the displayed the value. double m_factor; - // Minimal and maximal values accepted (inclusive). - LongestValueType m_min, m_max; - wxDECLARE_NO_ASSIGN_CLASS(wxFloatingPointValidatorBase); }; @@ -439,6 +439,8 @@ class wxFloatingPointValidator public: typedef T ValueType; typedef wxPrivate::wxNumValidator Base; + typedef wxFloatingPointValidatorBase::LongestValueType LongestValueType; + // Ctor using implicit (maximal) precision for this type. wxFloatingPointValidator(ValueType *value = NULL, @@ -466,19 +468,21 @@ public: return new wxFloatingPointValidator(*this); } -protected: - virtual bool CanBeNegative() const wxOVERRIDE { return DoGetMin() < 0; } + virtual bool IsInRange(LongestValueType value) const wxOVERRIDE + { + const ValueType valueT = static_cast(value); + + return this->GetMin() <= valueT && valueT <= this->GetMax(); + } private: - typedef typename Base::LongestValueType LongestValueType; - void DoSetMinMax() { // NB: Do not use min(), it's not the smallest representable value for // the floating point types but rather the smallest representable // positive value. - this->DoSetMin(static_cast(-std::numeric_limits::max())); - this->DoSetMax(static_cast( std::numeric_limits::max())); + this->SetMin(-std::numeric_limits::max()); + this->SetMax( std::numeric_limits::max()); } }; diff --git a/tests/validators/valnum.cpp b/tests/validators/valnum.cpp index c6c24eb635..8af177f3c9 100644 --- a/tests/validators/valnum.cpp +++ b/tests/validators/valnum.cpp @@ -96,6 +96,13 @@ TEST_CASE_METHOD(NumValidatorTestCase, "ValNum::TransferUnsigned", "[valnum]") CHECK( valUnsigned.TransferFromWindow() ); CHECK( value == 234 ); + m_text->ChangeValue("4294967295"); // == ULONG_MAX in 32 bits + CHECK( valUnsigned.TransferFromWindow() ); + CHECK( value == wxUINT32_MAX ); + + m_text->ChangeValue("4294967296"); // == ULONG_MAX + 1 + CHECK( !valUnsigned.TransferFromWindow() ); + m_text->ChangeValue("18446744073709551616"); // == ULLONG_MAX + 1 CHECK( !valUnsigned.TransferFromWindow() ); @@ -103,6 +110,28 @@ TEST_CASE_METHOD(NumValidatorTestCase, "ValNum::TransferUnsigned", "[valnum]") CHECK( !valUnsigned.TransferFromWindow() ); } +TEST_CASE_METHOD(NumValidatorTestCase, "ValNum::TransferULL", "[valnum]") +{ + unsigned long long value = 0; + wxIntegerValidator valULL(&value); + valULL.SetWindow(m_text); + + m_text->ChangeValue("9223372036854775807"); // == LLONG_MAX + CHECK( valULL.TransferFromWindow() ); + CHECK( value == static_cast(wxINT64_MAX) ); + + m_text->ChangeValue("9223372036854775808"); // == LLONG_MAX + 1 + CHECK( valULL.TransferFromWindow() ); + CHECK( value == static_cast(wxINT64_MAX) + 1 ); + + m_text->ChangeValue("18446744073709551615"); // == ULLONG_MAX + CHECK( valULL.TransferFromWindow() ); + CHECK( value == wxUINT64_MAX ); + + m_text->ChangeValue("18446744073709551616"); // == ULLONG_MAX + 1 + CHECK( !valULL.TransferFromWindow() ); +} + TEST_CASE_METHOD(NumValidatorTestCase, "ValNum::TransferFloat", "[valnum]") { // We need a locale with point as decimal separator. From a91cb5c344e0f4698c097fe12cf47b4ee08b703d Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 22 Feb 2021 11:55:32 +0100 Subject: [PATCH 12/16] Use CHECK_FALSE in wxNumberFormatter unit tests Just replace CHECK(!condition) with CHECK_FALSE(condition), this should result in slightly more clear error messages. Also make tests for long and long long more consistent. --- tests/strings/numformatter.cpp | 41 +++++++++++++++++++++------------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/tests/strings/numformatter.cpp b/tests/strings/numformatter.cpp index b9445190dc..77c65c05e8 100644 --- a/tests/strings/numformatter.cpp +++ b/tests/strings/numformatter.cpp @@ -167,9 +167,13 @@ TEST_CASE_METHOD(NumFormatterTestCase, "NumFormatter::LongFromString", "[numform ); long l; - CHECK( !wxNumberFormatter::FromString("", &l) ); - CHECK( !wxNumberFormatter::FromString("foo", &l) ); - CHECK( !wxNumberFormatter::FromString("1.234", &l) ); + CHECK_FALSE( wxNumberFormatter::FromString("", &l) ); + CHECK_FALSE( wxNumberFormatter::FromString("foo", &l) ); + CHECK_FALSE( wxNumberFormatter::FromString("1.234", &l) ); + CHECK_FALSE( wxNumberFormatter::FromString("-", &l) ); + + CHECK( wxNumberFormatter::FromString("0", &l) ); + CHECK( l == 0 ); CHECK( wxNumberFormatter::FromString("123", &l) ); CHECK( l == 123 ); @@ -188,6 +192,11 @@ TEST_CASE_METHOD(NumFormatterTestCase, "NumFormatter::LongFromString", "[numform CHECK( wxNumberFormatter::FromString("1,234,567", &l) ); CHECK( l == 1234567 ); + + CHECK( wxNumberFormatter::FromString("-123", &l) ); + CHECK( l == -123 ); + + CHECK_FALSE( wxNumberFormatter::FromString("9223372036854775808", &l) ); } #ifdef wxHAS_LONG_LONG_T_DIFFERENT_FROM_LONG @@ -203,10 +212,10 @@ TEST_CASE_METHOD(NumFormatterTestCase, "NumFormatter::LongLongFromString", "[num ); wxLongLong_t l; - CHECK( !wxNumberFormatter::FromString("", &l) ); - CHECK( !wxNumberFormatter::FromString("foo", &l) ); - CHECK( !wxNumberFormatter::FromString("1.234", &l) ); - CHECK( !wxNumberFormatter::FromString("-", &l) ); + CHECK_FALSE( wxNumberFormatter::FromString("", &l) ); + CHECK_FALSE( wxNumberFormatter::FromString("foo", &l) ); + CHECK_FALSE( wxNumberFormatter::FromString("1.234", &l) ); + CHECK_FALSE( wxNumberFormatter::FromString("-", &l) ); CHECK( wxNumberFormatter::FromString("0", &l) ); CHECK( l == 0 ); @@ -235,7 +244,7 @@ TEST_CASE_METHOD(NumFormatterTestCase, "NumFormatter::LongLongFromString", "[num CHECK( wxNumberFormatter::FromString("9223372036854775807", &l) ); CHECK( l == wxINT64_MAX ); - CHECK( !wxNumberFormatter::FromString("9223372036854775808", &l) ); + CHECK_FALSE( wxNumberFormatter::FromString("9223372036854775808", &l) ); } #endif // wxHAS_LONG_LONG_T_DIFFERENT_FROM_LONG @@ -246,11 +255,11 @@ TEST_CASE_METHOD(NumFormatterTestCase, "NumFormatter::ULongLongFromString", "[nu return; wxULongLong_t u; - CHECK( !wxNumberFormatter::FromString("", &u) ); - CHECK( !wxNumberFormatter::FromString("bar", &u) ); - CHECK( !wxNumberFormatter::FromString("1.234", &u) ); - CHECK( !wxNumberFormatter::FromString("-2", &u) ); - CHECK( !wxNumberFormatter::FromString("-", &u) ); + CHECK_FALSE( wxNumberFormatter::FromString("", &u) ); + CHECK_FALSE( wxNumberFormatter::FromString("bar", &u) ); + CHECK_FALSE( wxNumberFormatter::FromString("1.234", &u) ); + CHECK_FALSE( wxNumberFormatter::FromString("-2", &u) ); + CHECK_FALSE( wxNumberFormatter::FromString("-", &u) ); CHECK( wxNumberFormatter::FromString("0", &u) ); CHECK( u == 0 ); @@ -282,7 +291,7 @@ TEST_CASE_METHOD(NumFormatterTestCase, "NumFormatter::ULongLongFromString", "[nu CHECK( wxNumberFormatter::FromString("18446744073709551615", &u) ); CHECK( u == wxUINT64_MAX ); - CHECK( !wxNumberFormatter::FromString("18446744073709551616", &u) ); + CHECK_FALSE( wxNumberFormatter::FromString("18446744073709551616", &u) ); } TEST_CASE_METHOD(NumFormatterTestCase, "NumFormatter::DoubleFromString", "[numformatter]") @@ -296,8 +305,8 @@ TEST_CASE_METHOD(NumFormatterTestCase, "NumFormatter::DoubleFromString", "[numfo ); double d; - CHECK( !wxNumberFormatter::FromString("", &d) ); - CHECK( !wxNumberFormatter::FromString("bar", &d) ); + CHECK_FALSE( wxNumberFormatter::FromString("", &d) ); + CHECK_FALSE( wxNumberFormatter::FromString("bar", &d) ); CHECK( wxNumberFormatter::FromString("123", &d) ); CHECK( d == 123. ); From 94289a46b1b1ae88c5525b8740144c65d32c3439 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 22 Feb 2021 12:03:35 +0100 Subject: [PATCH 13/16] Relax check for converting "-" to long long in the unit tests This somehow succeeds when using gcc 4.8 under Ubuntu 14.04 or MinGW 5.3, so don't fail the tests in this case, but still warn about it because it seems quite unexpected. --- tests/strings/numformatter.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/strings/numformatter.cpp b/tests/strings/numformatter.cpp index 77c65c05e8..ba0e86ac69 100644 --- a/tests/strings/numformatter.cpp +++ b/tests/strings/numformatter.cpp @@ -215,7 +215,13 @@ TEST_CASE_METHOD(NumFormatterTestCase, "NumFormatter::LongLongFromString", "[num CHECK_FALSE( wxNumberFormatter::FromString("", &l) ); CHECK_FALSE( wxNumberFormatter::FromString("foo", &l) ); CHECK_FALSE( wxNumberFormatter::FromString("1.234", &l) ); - CHECK_FALSE( wxNumberFormatter::FromString("-", &l) ); + + // This somehow succeeds with gcc 4.8.4 under Ubuntu and MinGW 5.3, so + // don't use CHECK() for it. + if ( wxNumberFormatter::FromString("-", &l) ) + { + WARN("Converting \"-\" to long long unexpectedly succeeded, result: " << l); + } CHECK( wxNumberFormatter::FromString("0", &l) ); CHECK( l == 0 ); From 483856f3d327c5faaf3775475584183e52c25537 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 22 Feb 2021 12:47:58 +0100 Subject: [PATCH 14/16] Add wxIntegerValidatorBase::ULongestValueType typedef Allow just writing ULongestValueType instead of writing #ifdefs in several places. --- include/wx/valnum.h | 2 ++ src/common/valnum.cpp | 6 +----- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/include/wx/valnum.h b/include/wx/valnum.h index 5305796edd..6c94f53613 100644 --- a/include/wx/valnum.h +++ b/include/wx/valnum.h @@ -293,8 +293,10 @@ protected: // on it. #ifdef wxLongLong_t typedef wxLongLong_t LongestValueType; + typedef wxULongLong_t ULongestValueType; #else typedef long LongestValueType; + typedef unsigned long ULongestValueType; #endif wxIntegerValidatorBase(int style) diff --git a/src/common/valnum.cpp b/src/common/valnum.cpp index b4adef753b..b0735393b0 100644 --- a/src/common/valnum.cpp +++ b/src/common/valnum.cpp @@ -244,11 +244,7 @@ wxIntegerValidatorBase::FromString(const wxString& s, else { // Parse as unsigned to ensure we don't accept minus sign here. -#ifdef wxULongLong_t - wxULongLong_t uvalue; -#else - unsigned long uvalue; -#endif + ULongestValueType uvalue; if ( !wxNumberFormatter::FromString(s, &uvalue) ) return false; From bdaca389045f5d67ad36b7626f6f7c584bfac0a6 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 22 Feb 2021 12:52:25 +0100 Subject: [PATCH 15/16] Fix transferring unsigned values to the window too In addition to not accepting "-1" as a valid unsigned value, we also must not format unsigned values as signed ones, i.e. we need to use wxNumberFormatter::ToString() overload taking unsigned for them. Do it in wxIntegerValidatorBase::ToString(), used by TransferToWindow(), and add more tests for the latter. Also reorganize the tests in sections and use REQUIRE() for the checks that should prevent the rest of the section from running if they fail. --- src/common/valnum.cpp | 10 ++++++++- tests/validators/valnum.cpp | 45 ++++++++++++++++++++++++++++--------- 2 files changed, 43 insertions(+), 12 deletions(-) diff --git a/src/common/valnum.cpp b/src/common/valnum.cpp index b0735393b0..563701aadb 100644 --- a/src/common/valnum.cpp +++ b/src/common/valnum.cpp @@ -230,7 +230,15 @@ void wxNumValidatorBase::OnKillFocus(wxFocusEvent& event) wxString wxIntegerValidatorBase::ToString(LongestValueType value) const { - return wxNumberFormatter::ToString(value, GetFormatFlags()); + if ( CanBeNegative() ) + { + return wxNumberFormatter::ToString(value, GetFormatFlags()); + } + else + { + ULongestValueType uvalue = static_cast(value); + return wxNumberFormatter::ToString(uvalue, GetFormatFlags()); + } } bool diff --git a/tests/validators/valnum.cpp b/tests/validators/valnum.cpp index 8af177f3c9..b4fb2ae277 100644 --- a/tests/validators/valnum.cpp +++ b/tests/validators/valnum.cpp @@ -99,6 +99,8 @@ TEST_CASE_METHOD(NumValidatorTestCase, "ValNum::TransferUnsigned", "[valnum]") m_text->ChangeValue("4294967295"); // == ULONG_MAX in 32 bits CHECK( valUnsigned.TransferFromWindow() ); CHECK( value == wxUINT32_MAX ); + CHECK( valUnsigned.TransferToWindow() ); + CHECK( m_text->GetValue() == "4294967295" ); m_text->ChangeValue("4294967296"); // == ULONG_MAX + 1 CHECK( !valUnsigned.TransferFromWindow() ); @@ -116,20 +118,41 @@ TEST_CASE_METHOD(NumValidatorTestCase, "ValNum::TransferULL", "[valnum]") wxIntegerValidator valULL(&value); valULL.SetWindow(m_text); - m_text->ChangeValue("9223372036854775807"); // == LLONG_MAX - CHECK( valULL.TransferFromWindow() ); - CHECK( value == static_cast(wxINT64_MAX) ); + SECTION("LLONG_MAX") + { + m_text->ChangeValue("9223372036854775807"); // == LLONG_MAX + REQUIRE( valULL.TransferFromWindow() ); + CHECK( value == static_cast(wxINT64_MAX) ); - m_text->ChangeValue("9223372036854775808"); // == LLONG_MAX + 1 - CHECK( valULL.TransferFromWindow() ); - CHECK( value == static_cast(wxINT64_MAX) + 1 ); + REQUIRE( valULL.TransferToWindow() ); + CHECK( m_text->GetValue() == "9223372036854775807" ); + } - m_text->ChangeValue("18446744073709551615"); // == ULLONG_MAX - CHECK( valULL.TransferFromWindow() ); - CHECK( value == wxUINT64_MAX ); + SECTION("LLONG_MAX+1") + { + m_text->ChangeValue("9223372036854775808"); // == LLONG_MAX + 1 + REQUIRE( valULL.TransferFromWindow() ); + CHECK( value == static_cast(wxINT64_MAX) + 1 ); - m_text->ChangeValue("18446744073709551616"); // == ULLONG_MAX + 1 - CHECK( !valULL.TransferFromWindow() ); + REQUIRE( valULL.TransferToWindow() ); + CHECK( m_text->GetValue() == "9223372036854775808" ); + } + + SECTION("ULLONG_MAX") + { + m_text->ChangeValue("18446744073709551615"); // == ULLONG_MAX + REQUIRE( valULL.TransferFromWindow() ); + CHECK( value == wxUINT64_MAX ); + + REQUIRE( valULL.TransferToWindow() ); + CHECK( m_text->GetValue() == "18446744073709551615" ); + } + + SECTION("ULLONG_MAX+1") + { + m_text->ChangeValue("18446744073709551616"); // == ULLONG_MAX + 1 + CHECK( !valULL.TransferFromWindow() ); + } } TEST_CASE_METHOD(NumValidatorTestCase, "ValNum::TransferFloat", "[valnum]") From 72a24b402b0075629ad3c19abb1d17665b25e5e1 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 22 Feb 2021 14:26:23 +0100 Subject: [PATCH 16/16] Fix deleting of wxTextCtrl in wxNumValidator test case This was broken in da973c3caf (Get rid of CppUnit boilerplate in numeric validator unit tests, 2021-02-21) which replaced DestroyChildren() in the old CppUnit test case dtor with just "delete m_text", as this didn't take care of "text2" created in one of the tests. Using DestroyChildren() still seems overly side, so ensure that "text2" is destroyed explicitly. --- tests/validators/valnum.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/validators/valnum.cpp b/tests/validators/valnum.cpp index b4fb2ae277..b91129359c 100644 --- a/tests/validators/valnum.cpp +++ b/tests/validators/valnum.cpp @@ -20,6 +20,8 @@ #include "asserthelper.h" #include "testableframe.h" + +#include "wx/scopeguard.h" #include "wx/uiaction.h" class NumValidatorTestCase @@ -243,6 +245,7 @@ TEST_CASE_METHOD(NumValidatorTestCase, "ValNum::Interactive", "[valnum]") // Create a sibling text control to be able to switch focus and thus // trigger the control validation/normalization. wxTextCtrl * const text2 = new wxTextCtrl(m_text->GetParent(), wxID_ANY); + wxON_BLOCK_EXIT_OBJ0( *text2, wxWindow::Destroy ); text2->Move(10, 80); // Just to see it better while debugging... wxFloatingPointValidator valFloat(3); valFloat.SetRange(-10., 10.);