From 885ef5819e2e2567be5c91ffecfaff4f4f2b5461 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 11 Jul 2020 18:52:49 +0200 Subject: [PATCH 1/3] Remove CppUnit boilerplate from wxVector unit test case No real changes. --- tests/vectors/vectors.cpp | 182 +++++++++++++++----------------------- 1 file changed, 72 insertions(+), 110 deletions(-) diff --git a/tests/vectors/vectors.cpp b/tests/vectors/vectors.cpp index bf0fffa752..5c393afbb8 100644 --- a/tests/vectors/vectors.cpp +++ b/tests/vectors/vectors.cpp @@ -58,7 +58,7 @@ class SelfPointingObject public: SelfPointingObject() { m_self = this; } SelfPointingObject(const SelfPointingObject&) { m_self = this; } - ~SelfPointingObject() { CPPUNIT_ASSERT( this == m_self ); } + ~SelfPointingObject() { CHECK( this == m_self ); } // the assignment operator should not modify our "this" pointer so // implement it just to prevent the default version from doing it @@ -72,100 +72,63 @@ private: // test class // ---------------------------------------------------------------------------- -class VectorsTestCase : public CppUnit::TestCase -{ -public: - VectorsTestCase() {} - -private: - CPPUNIT_TEST_SUITE( VectorsTestCase ); - CPPUNIT_TEST( PushPopTest ); - CPPUNIT_TEST( Insert ); - CPPUNIT_TEST( Erase ); - CPPUNIT_TEST( Iterators ); - CPPUNIT_TEST( Objects ); - CPPUNIT_TEST( NonPODs ); - CPPUNIT_TEST( Resize ); - CPPUNIT_TEST( Swap ); - CPPUNIT_TEST( Sort ); - CPPUNIT_TEST_SUITE_END(); - - void PushPopTest(); - void Insert(); - void Erase(); - void Iterators(); - void Objects(); - void NonPODs(); - void Resize(); - void Swap(); - void Sort(); - - wxDECLARE_NO_COPY_CLASS(VectorsTestCase); -}; - -// register in the unnamed registry so that these tests are run by default -CPPUNIT_TEST_SUITE_REGISTRATION( VectorsTestCase ); - -// also include in its own registry so that these tests can be run alone -CPPUNIT_TEST_SUITE_NAMED_REGISTRATION( VectorsTestCase, "VectorsTestCase" ); - -void VectorsTestCase::PushPopTest() +TEST_CASE("wxVector::Push/Pop", "[vector][push_back][pop_back]") { wxVector v; - CPPUNIT_ASSERT( v.size() == 0 ); + CHECK( v.size() == 0 ); v.push_back(1); - CPPUNIT_ASSERT( v.size() == 1 ); + CHECK( v.size() == 1 ); v.push_back(2); - CPPUNIT_ASSERT( v.size() == 2 ); + CHECK( v.size() == 2 ); v.push_back(42); - CPPUNIT_ASSERT( v.size() == 3 ); + CHECK( v.size() == 3 ); - CPPUNIT_ASSERT( v[0] == 1 ); - CPPUNIT_ASSERT( v[1] == 2 ); - CPPUNIT_ASSERT( v[2] == 42 ); + CHECK( v[0] == 1 ); + CHECK( v[1] == 2 ); + CHECK( v[2] == 42 ); v.pop_back(); - CPPUNIT_ASSERT( v.size() == 2 ); - CPPUNIT_ASSERT( v[0] == 1 ); - CPPUNIT_ASSERT( v[1] == 2 ); + CHECK( v.size() == 2 ); + CHECK( v[0] == 1 ); + CHECK( v[1] == 2 ); v.pop_back(); - CPPUNIT_ASSERT( v.size() == 1 ); - CPPUNIT_ASSERT( v[0] == 1 ); + CHECK( v.size() == 1 ); + CHECK( v[0] == 1 ); v.pop_back(); - CPPUNIT_ASSERT( v.size() == 0 ); - CPPUNIT_ASSERT( v.empty() ); + CHECK( v.size() == 0 ); + CHECK( v.empty() ); wxVector vEmpty; } -void VectorsTestCase::Insert() +TEST_CASE("wxVector::Insert", "[vector][insert]") { wxVector v; v.insert(v.end(), 'a'); - CPPUNIT_ASSERT( v.size() == 1 ); - CPPUNIT_ASSERT( v[0] == 'a' ); + CHECK( v.size() == 1 ); + CHECK( v[0] == 'a' ); v.insert(v.end(), 'b'); - CPPUNIT_ASSERT( v.size() == 2 ); - CPPUNIT_ASSERT( v[0] == 'a' ); - CPPUNIT_ASSERT( v[1] == 'b' ); + CHECK( v.size() == 2 ); + CHECK( v[0] == 'a' ); + CHECK( v[1] == 'b' ); v.insert(v.begin(), '0'); - CPPUNIT_ASSERT( v.size() == 3 ); - CPPUNIT_ASSERT( v[0] == '0' ); - CPPUNIT_ASSERT( v[1] == 'a' ); - CPPUNIT_ASSERT( v[2] == 'b' ); + CHECK( v.size() == 3 ); + CHECK( v[0] == '0' ); + CHECK( v[1] == 'a' ); + CHECK( v[2] == 'b' ); v.insert(v.begin() + 2, 'X'); - CPPUNIT_ASSERT( v.size() == 4 ); - CPPUNIT_ASSERT( v[0] == '0' ); - CPPUNIT_ASSERT( v[1] == 'a' ); - CPPUNIT_ASSERT( v[2] == 'X' ); - CPPUNIT_ASSERT( v[3] == 'b' ); + CHECK( v.size() == 4 ); + CHECK( v[0] == '0' ); + CHECK( v[1] == 'a' ); + CHECK( v[2] == 'X' ); + CHECK( v[3] == 'b' ); v.insert(v.begin() + 3, 3, 'Z'); REQUIRE( v.size() == 7 ); @@ -178,7 +141,7 @@ void VectorsTestCase::Insert() CHECK( v[6] == 'b' ); } -void VectorsTestCase::Erase() +TEST_CASE("wxVector::Erase", "[vector][erase]") { wxVector v; @@ -186,27 +149,27 @@ void VectorsTestCase::Erase() v.push_back(2); v.push_back(3); v.push_back(4); - CPPUNIT_ASSERT( v.size() == 4 ); + CHECK( v.size() == 4 ); v.erase(v.begin(), v.end()-1); - CPPUNIT_ASSERT( v.size() == 1 ); - CPPUNIT_ASSERT( v[0] == 4 ); + CHECK( v.size() == 1 ); + CHECK( v[0] == 4 ); v.clear(); v.push_back(1); v.push_back(2); v.push_back(3); v.push_back(4); - CPPUNIT_ASSERT( v.size() == 4 ); + CHECK( v.size() == 4 ); v.erase(v.begin()); - CPPUNIT_ASSERT( v.size() == 3 ); - CPPUNIT_ASSERT( v[0] == 2 ); - CPPUNIT_ASSERT( v[1] == 3 ); - CPPUNIT_ASSERT( v[2] == 4 ); + CHECK( v.size() == 3 ); + CHECK( v[0] == 2 ); + CHECK( v[1] == 3 ); + CHECK( v[2] == 4 ); } -void VectorsTestCase::Iterators() +TEST_CASE("wxVector::Iterators", "[vector][iterator]") { wxVector v; v.push_back(1); @@ -217,11 +180,11 @@ void VectorsTestCase::Iterators() int value = 1; for ( wxVector::iterator i = v.begin(); i != v.end(); ++i, ++value ) { - CPPUNIT_ASSERT_EQUAL( value, *i ); + CHECK( *i == value ); } } -void VectorsTestCase::Objects() +TEST_CASE("wxVector::Objects", "[vector]") { wxVector v; v.push_back(CountedObject(1)); @@ -229,14 +192,14 @@ void VectorsTestCase::Objects() v.push_back(CountedObject(3)); v.erase(v.begin()); - CPPUNIT_ASSERT_EQUAL( 2, v.size() ); - CPPUNIT_ASSERT_EQUAL( 2, CountedObject::GetCount() ); + CHECK( v.size() == 2 ); + CHECK( CountedObject::GetCount() == 2 ); v.clear(); - CPPUNIT_ASSERT_EQUAL( 0, CountedObject::GetCount() ); + CHECK( CountedObject::GetCount() == 0 ); } -void VectorsTestCase::NonPODs() +TEST_CASE("wxVector::NonPODs", "[vector]") { wxVector v; v.push_back(SelfPointingObject()); @@ -259,53 +222,52 @@ void VectorsTestCase::NonPODs() vs.clear(); } -void VectorsTestCase::Resize() +TEST_CASE("wxVector::Resize", "[vector][resize]") { wxVector v; v.resize(3); - CPPUNIT_ASSERT_EQUAL( 3, v.size() ); - CPPUNIT_ASSERT_EQUAL( 3, CountedObject::GetCount() ); - CPPUNIT_ASSERT_EQUAL( 0, v[0].GetValue() ); - CPPUNIT_ASSERT_EQUAL( 0, v[1].GetValue() ); - CPPUNIT_ASSERT_EQUAL( 0, v[2].GetValue() ); + CHECK( v.size() == 3 ); + CHECK( CountedObject::GetCount() == 3 ); + CHECK( v[0].GetValue() == 0 ); + CHECK( v[1].GetValue() == 0 ); + CHECK( v[2].GetValue() == 0 ); v.resize(1); - CPPUNIT_ASSERT_EQUAL( 1, v.size() ); - CPPUNIT_ASSERT_EQUAL( 1, CountedObject::GetCount() ); + CHECK( v.size() == 1 ); + CHECK( CountedObject::GetCount() == 1 ); v.resize(4, CountedObject(17)); - CPPUNIT_ASSERT_EQUAL( 4, v.size() ); - CPPUNIT_ASSERT_EQUAL( 4, CountedObject::GetCount() ); - CPPUNIT_ASSERT_EQUAL( 0, v[0].GetValue() ); - CPPUNIT_ASSERT_EQUAL( 17, v[1].GetValue() ); - CPPUNIT_ASSERT_EQUAL( 17, v[2].GetValue() ); - CPPUNIT_ASSERT_EQUAL( 17, v[3].GetValue() ); + CHECK( v.size() == 4 ); + CHECK( CountedObject::GetCount() == 4 ); + CHECK( v[0].GetValue() == 0 ); + CHECK( v[1].GetValue() == 17 ); + CHECK( v[2].GetValue() == 17 ); + CHECK( v[3].GetValue() == 17 ); } -void VectorsTestCase::Swap() +TEST_CASE("wxVector::Swap", "[vector][swap]") { wxVector v1, v2; v1.push_back(17); v1.swap(v2); - CPPUNIT_ASSERT( v1.empty() ); - CPPUNIT_ASSERT_EQUAL( 1, v2.size() ); - CPPUNIT_ASSERT_EQUAL( 17, v2[0] ); + CHECK( v1.empty() ); + CHECK( v2.size() == 1 ); + CHECK( v2[0] == 17 ); v1.push_back(9); v2.swap(v1); - CPPUNIT_ASSERT_EQUAL( 1, v1.size() ); - CPPUNIT_ASSERT_EQUAL( 17, v1[0] ); - CPPUNIT_ASSERT_EQUAL( 1, v2.size() ); - CPPUNIT_ASSERT_EQUAL( 9, v2[0] ); + CHECK( v1.size() == 1 ); + CHECK( v1[0] == 17 ); + CHECK( v2.size() == 1 ); + CHECK( v2[0] == 9 ); v2.clear(); v1.swap(v2); - CPPUNIT_ASSERT( v1.empty() ); + CHECK( v1.empty() ); } - -void VectorsTestCase::Sort() +TEST_CASE("wxVector::Sort", "[vector][sort]") { size_t idx; wxVector v; @@ -325,7 +287,7 @@ void VectorsTestCase::Sort() for (idx=1; idx Date: Sat, 11 Jul 2020 19:00:38 +0200 Subject: [PATCH 2/3] Fix off-by-one bug in wxVector::reverse_iterator::base() This is embarrassing, but the iterator returned by this method seems to have always been wrong, ever since it was added back in 946954d3bf (Added reverse iterator to wxVector, 2008-09-16). Moreover, it was also broken in its const_reverse_iterator counterpart where it was copy-and-pasted in f7ef20685f (Add wxVector<>::const_reverse_iterator, 2013-05-08). Finally fix this to return the correct value and add at least a simple unit test check that this method behaves as expected. --- include/wx/vector.h | 4 ++-- tests/vectors/vectors.cpp | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/include/wx/vector.h b/include/wx/vector.h index e05920f985..771b25bc0a 100644 --- a/include/wx/vector.h +++ b/include/wx/vector.h @@ -198,7 +198,7 @@ public: reference operator*() const { return *m_ptr; } pointer operator->() const { return m_ptr; } - iterator base() const { return m_ptr; } + iterator base() const { return m_ptr + 1; } reverse_iterator& operator++() { --m_ptr; return *this; } @@ -261,7 +261,7 @@ public: const_reference operator*() const { return *m_ptr; } const_pointer operator->() const { return m_ptr; } - const_iterator base() const { return m_ptr; } + const_iterator base() const { return m_ptr + 1; } const_reverse_iterator& operator++() { --m_ptr; return *this; } diff --git a/tests/vectors/vectors.cpp b/tests/vectors/vectors.cpp index 5c393afbb8..056b9303e6 100644 --- a/tests/vectors/vectors.cpp +++ b/tests/vectors/vectors.cpp @@ -334,6 +334,10 @@ TEST_CASE("wxVector::reverse_iterator", "[vector][reverse_iterator]") CHECK( ri < re ); CHECK( ri <= re ); + CHECK( rb.base() == v.end() ); + CHECK( re.base() == v.begin() ); + CHECK( *ri.base() == 9 ); + #if wxUSE_STD_CONTAINERS_COMPATIBLY std::vector stdvec(rb, re); REQUIRE( stdvec.size() == 10 ); From 6f7dc148010ea7a532f05f3068d057e836bce471 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 11 Jul 2020 19:33:35 +0200 Subject: [PATCH 3/3] Fix wrong iterator in wxInfoBar::RemoveButton() in wxGTK The iterator passed to erase() was off by 1, but this worked correctly in the default build using wxVector, and not std::vector, because its base() implementation was off by 1 too. Now that wxVector bug is fixed (see the parent) commit, fix the code using it too, which makes it work both in the default and STL builds. Closes #18765. --- src/gtk/infobar.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gtk/infobar.cpp b/src/gtk/infobar.cpp index c3d4d4018d..349c4e4ea5 100644 --- a/src/gtk/infobar.cpp +++ b/src/gtk/infobar.cpp @@ -323,7 +323,7 @@ void wxInfoBar::RemoveButton(wxWindowID btnid) if (i->id == btnid) { gtk_widget_destroy(i->button); - buttons.erase(i.base()); + buttons.erase(i.base() - 1); // see comment in GTKAddButton() InvalidateBestSize();