From 7c2031620d9993b98074cd8c975dcec94f17a3fa Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 29 Aug 2022 18:32:52 +0200 Subject: [PATCH 1/3] Remove CppUnit boilerplate from wxRadioBox unit test Define a simple fixture instead of inheriting from CppUnit::TestCase. No real changes, just make the code shorter, less redundant and make it simpler to run individual tests. --- tests/controls/radioboxtest.cpp | 61 ++++++++------------------------- 1 file changed, 15 insertions(+), 46 deletions(-) diff --git a/tests/controls/radioboxtest.cpp b/tests/controls/radioboxtest.cpp index 5ecefb64eb..4f5175e715 100644 --- a/tests/controls/radioboxtest.cpp +++ b/tests/controls/radioboxtest.cpp @@ -18,49 +18,18 @@ #include "wx/tooltip.h" -class RadioBoxTestCase : public CppUnit::TestCase +class RadioBoxTestCase { -public: - RadioBoxTestCase() { } - - void setUp() wxOVERRIDE; - void tearDown() wxOVERRIDE; - -private: - CPPUNIT_TEST_SUITE( RadioBoxTestCase ); - CPPUNIT_TEST( FindString ); - CPPUNIT_TEST( RowColCount ); - CPPUNIT_TEST( Enable ); - CPPUNIT_TEST( Show ); - CPPUNIT_TEST( HelpText ); - CPPUNIT_TEST( ToolTip ); - CPPUNIT_TEST( Selection ); - CPPUNIT_TEST( Count ); - CPPUNIT_TEST( SetString ); - CPPUNIT_TEST_SUITE_END(); - - void FindString(); - void RowColCount(); - void Enable(); - void Show(); - void HelpText(); - void ToolTip(); - void Selection(); - void Count(); - void SetString(); +protected: + RadioBoxTestCase(); + ~RadioBoxTestCase(); wxRadioBox* m_radio; wxDECLARE_NO_COPY_CLASS(RadioBoxTestCase); }; -// register in the unnamed registry so that these tests are run by default -CPPUNIT_TEST_SUITE_REGISTRATION( RadioBoxTestCase ); - -// also include in its own registry so that these tests can be run alone -CPPUNIT_TEST_SUITE_NAMED_REGISTRATION( RadioBoxTestCase, "RadioBoxTestCase" ); - -void RadioBoxTestCase::setUp() +RadioBoxTestCase::RadioBoxTestCase() { wxArrayString choices; choices.push_back("item 0"); @@ -71,12 +40,12 @@ void RadioBoxTestCase::setUp() wxDefaultPosition, wxDefaultSize, choices); } -void RadioBoxTestCase::tearDown() +RadioBoxTestCase::~RadioBoxTestCase() { wxTheApp->GetTopWindow()->DestroyChildren(); } -void RadioBoxTestCase::FindString() +TEST_CASE_METHOD(RadioBoxTestCase, "RadioBox::FindString", "[radiobox][find]") { CPPUNIT_ASSERT_EQUAL(wxNOT_FOUND, m_radio->FindString("not here")); CPPUNIT_ASSERT_EQUAL(1, m_radio->FindString("item 1")); @@ -84,7 +53,7 @@ void RadioBoxTestCase::FindString() CPPUNIT_ASSERT_EQUAL(wxNOT_FOUND, m_radio->FindString("ITEM 2", true)); } -void RadioBoxTestCase::RowColCount() +TEST_CASE_METHOD(RadioBoxTestCase, "RadioBox::RowColCount", "[radiobox]") { #ifndef __WXGTK__ wxArrayString choices; @@ -107,7 +76,7 @@ void RadioBoxTestCase::RowColCount() #endif } -void RadioBoxTestCase::Enable() +TEST_CASE_METHOD(RadioBoxTestCase, "RadioBox::Enable", "[radiobox][enable]") { #ifndef __WXOSX__ m_radio->Enable(false); @@ -134,7 +103,7 @@ void RadioBoxTestCase::Enable() #endif } -void RadioBoxTestCase::Show() +TEST_CASE_METHOD(RadioBoxTestCase, "RadioBox::Show", "[radiobox][show]") { m_radio->Show(false); @@ -159,7 +128,7 @@ void RadioBoxTestCase::Show() CPPUNIT_ASSERT(m_radio->IsItemShown(2)); } -void RadioBoxTestCase::HelpText() +TEST_CASE_METHOD(RadioBoxTestCase, "RadioBox::HelpText", "[radiobox][help]") { CPPUNIT_ASSERT_EQUAL(wxEmptyString, m_radio->GetItemHelpText(0)); @@ -172,7 +141,7 @@ void RadioBoxTestCase::HelpText() CPPUNIT_ASSERT_EQUAL(wxEmptyString, m_radio->GetItemHelpText(1)); } -void RadioBoxTestCase::ToolTip() +TEST_CASE_METHOD(RadioBoxTestCase, "RadioBox::ToolTip", "[radiobox][tooltip]") { #if defined (__WXMSW__) || defined(__WXGTK__) //GetItemToolTip returns null if there is no tooltip set @@ -189,7 +158,7 @@ void RadioBoxTestCase::ToolTip() #endif } -void RadioBoxTestCase::Selection() +TEST_CASE_METHOD(RadioBoxTestCase, "RadioBox::Selection", "[radiobox][selection]") { //Until other item containers the first item is selected by default CPPUNIT_ASSERT_EQUAL(0, m_radio->GetSelection()); @@ -206,7 +175,7 @@ void RadioBoxTestCase::Selection() CPPUNIT_ASSERT_EQUAL("item 2", m_radio->GetStringSelection()); } -void RadioBoxTestCase::Count() +TEST_CASE_METHOD(RadioBoxTestCase, "RadioBox::Count", "[radiobox]") { //A trivial test for the item count as items can neither //be added or removed @@ -214,7 +183,7 @@ void RadioBoxTestCase::Count() CPPUNIT_ASSERT(!m_radio->IsEmpty()); } -void RadioBoxTestCase::SetString() +TEST_CASE_METHOD(RadioBoxTestCase, "RadioBox::SetString", "[radiobox]") { m_radio->SetString(0, "new item 0"); m_radio->SetString(2, ""); From 5a6f2796d5cef15467e2084bb51ad62372d72776 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 29 Aug 2022 18:34:16 +0200 Subject: [PATCH 2/3] Use CATCH macros instead of CPPUNIT_ASSERT() in wxRadioBox test No real changes, just get rid of the last vestiges of CppUnit in this file. --- tests/controls/radioboxtest.cpp | 88 ++++++++++++++++----------------- 1 file changed, 44 insertions(+), 44 deletions(-) diff --git a/tests/controls/radioboxtest.cpp b/tests/controls/radioboxtest.cpp index 4f5175e715..1f78e180b3 100644 --- a/tests/controls/radioboxtest.cpp +++ b/tests/controls/radioboxtest.cpp @@ -47,10 +47,10 @@ RadioBoxTestCase::~RadioBoxTestCase() TEST_CASE_METHOD(RadioBoxTestCase, "RadioBox::FindString", "[radiobox][find]") { - CPPUNIT_ASSERT_EQUAL(wxNOT_FOUND, m_radio->FindString("not here")); - CPPUNIT_ASSERT_EQUAL(1, m_radio->FindString("item 1")); - CPPUNIT_ASSERT_EQUAL(2, m_radio->FindString("ITEM 2")); - CPPUNIT_ASSERT_EQUAL(wxNOT_FOUND, m_radio->FindString("ITEM 2", true)); + CHECK( m_radio->FindString("not here") == wxNOT_FOUND ); + CHECK( m_radio->FindString("item 1") == 1 ); + CHECK( m_radio->FindString("ITEM 2") == 2 ); + CHECK( m_radio->FindString("ITEM 2", true) == wxNOT_FOUND ); } TEST_CASE_METHOD(RadioBoxTestCase, "RadioBox::RowColCount", "[radiobox]") @@ -64,15 +64,15 @@ TEST_CASE_METHOD(RadioBoxTestCase, "RadioBox::RowColCount", "[radiobox]") m_radio = new wxRadioBox(wxTheApp->GetTopWindow(), wxID_ANY, "RadioBox", wxDefaultPosition, wxDefaultSize, choices, 2); - CPPUNIT_ASSERT_EQUAL(2, m_radio->GetColumnCount()); - CPPUNIT_ASSERT_EQUAL(2, m_radio->GetRowCount()); + CHECK( m_radio->GetColumnCount() == 2 ); + CHECK( m_radio->GetRowCount() == 2 ); m_radio = new wxRadioBox(wxTheApp->GetTopWindow(), wxID_ANY, "RadioBox", wxDefaultPosition, wxDefaultSize, choices, 1, wxRA_SPECIFY_ROWS); - CPPUNIT_ASSERT_EQUAL(3, m_radio->GetColumnCount()); - CPPUNIT_ASSERT_EQUAL(1, m_radio->GetRowCount()); + CHECK( m_radio->GetColumnCount() == 3 ); + CHECK( m_radio->GetRowCount() == 1 ); #endif } @@ -81,25 +81,25 @@ TEST_CASE_METHOD(RadioBoxTestCase, "RadioBox::Enable", "[radiobox][enable]") #ifndef __WXOSX__ m_radio->Enable(false); - CPPUNIT_ASSERT(!m_radio->IsItemEnabled(0)); + CHECK(!m_radio->IsItemEnabled(0)); m_radio->Enable(1, true); - CPPUNIT_ASSERT(!m_radio->IsItemEnabled(0)); - CPPUNIT_ASSERT(m_radio->IsItemEnabled(1)); - CPPUNIT_ASSERT(!m_radio->IsItemEnabled(2)); + CHECK(!m_radio->IsItemEnabled(0)); + CHECK(m_radio->IsItemEnabled(1)); + CHECK(!m_radio->IsItemEnabled(2)); m_radio->Enable(true); - CPPUNIT_ASSERT(m_radio->IsItemEnabled(0)); - CPPUNIT_ASSERT(m_radio->IsItemEnabled(1)); - CPPUNIT_ASSERT(m_radio->IsItemEnabled(2)); + CHECK(m_radio->IsItemEnabled(0)); + CHECK(m_radio->IsItemEnabled(1)); + CHECK(m_radio->IsItemEnabled(2)); m_radio->Enable(0, false); - CPPUNIT_ASSERT(!m_radio->IsItemEnabled(0)); - CPPUNIT_ASSERT(m_radio->IsItemEnabled(1)); - CPPUNIT_ASSERT(m_radio->IsItemEnabled(2)); + CHECK(!m_radio->IsItemEnabled(0)); + CHECK(m_radio->IsItemEnabled(1)); + CHECK(m_radio->IsItemEnabled(2)); #endif } @@ -107,80 +107,80 @@ TEST_CASE_METHOD(RadioBoxTestCase, "RadioBox::Show", "[radiobox][show]") { m_radio->Show(false); - CPPUNIT_ASSERT(!m_radio->IsItemShown(0)); + CHECK(!m_radio->IsItemShown(0)); m_radio->Show(1, true); - CPPUNIT_ASSERT(!m_radio->IsItemShown(0)); - CPPUNIT_ASSERT(m_radio->IsItemShown(1)); - CPPUNIT_ASSERT(!m_radio->IsItemShown(2)); + CHECK(!m_radio->IsItemShown(0)); + CHECK(m_radio->IsItemShown(1)); + CHECK(!m_radio->IsItemShown(2)); m_radio->Show(true); - CPPUNIT_ASSERT(m_radio->IsItemShown(0)); - CPPUNIT_ASSERT(m_radio->IsItemShown(1)); - CPPUNIT_ASSERT(m_radio->IsItemShown(2)); + CHECK(m_radio->IsItemShown(0)); + CHECK(m_radio->IsItemShown(1)); + CHECK(m_radio->IsItemShown(2)); m_radio->Show(0, false); - CPPUNIT_ASSERT(!m_radio->IsItemShown(0)); - CPPUNIT_ASSERT(m_radio->IsItemShown(1)); - CPPUNIT_ASSERT(m_radio->IsItemShown(2)); + CHECK(!m_radio->IsItemShown(0)); + CHECK(m_radio->IsItemShown(1)); + CHECK(m_radio->IsItemShown(2)); } TEST_CASE_METHOD(RadioBoxTestCase, "RadioBox::HelpText", "[radiobox][help]") { - CPPUNIT_ASSERT_EQUAL(wxEmptyString, m_radio->GetItemHelpText(0)); + CHECK( m_radio->GetItemHelpText(0) == wxEmptyString ); m_radio->SetItemHelpText(1, "Item 1 help"); - CPPUNIT_ASSERT_EQUAL("Item 1 help", m_radio->GetItemHelpText(1)); + CHECK( m_radio->GetItemHelpText(1) == "Item 1 help" ); m_radio->SetItemHelpText(1, ""); - CPPUNIT_ASSERT_EQUAL(wxEmptyString, m_radio->GetItemHelpText(1)); + CHECK( m_radio->GetItemHelpText(1) == wxEmptyString ); } TEST_CASE_METHOD(RadioBoxTestCase, "RadioBox::ToolTip", "[radiobox][tooltip]") { #if defined (__WXMSW__) || defined(__WXGTK__) //GetItemToolTip returns null if there is no tooltip set - CPPUNIT_ASSERT(!m_radio->GetItemToolTip(0)); + CHECK(!m_radio->GetItemToolTip(0)); m_radio->SetItemToolTip(1, "Item 1 help"); - CPPUNIT_ASSERT_EQUAL("Item 1 help", m_radio->GetItemToolTip(1)->GetTip()); + CHECK( m_radio->GetItemToolTip(1)->GetTip() == "Item 1 help" ); m_radio->SetItemToolTip(1, ""); //However if we set a blank tip this does count as a tooltip - CPPUNIT_ASSERT(!m_radio->GetItemToolTip(1)); + CHECK(!m_radio->GetItemToolTip(1)); #endif } TEST_CASE_METHOD(RadioBoxTestCase, "RadioBox::Selection", "[radiobox][selection]") { //Until other item containers the first item is selected by default - CPPUNIT_ASSERT_EQUAL(0, m_radio->GetSelection()); - CPPUNIT_ASSERT_EQUAL("item 0", m_radio->GetStringSelection()); + CHECK( m_radio->GetSelection() == 0 ); + CHECK( m_radio->GetStringSelection() == "item 0" ); m_radio->SetSelection(1); - CPPUNIT_ASSERT_EQUAL(1, m_radio->GetSelection()); - CPPUNIT_ASSERT_EQUAL("item 1", m_radio->GetStringSelection()); + CHECK( m_radio->GetSelection() == 1 ); + CHECK( m_radio->GetStringSelection() == "item 1" ); m_radio->SetStringSelection("item 2"); - CPPUNIT_ASSERT_EQUAL(2, m_radio->GetSelection()); - CPPUNIT_ASSERT_EQUAL("item 2", m_radio->GetStringSelection()); + CHECK( m_radio->GetSelection() == 2 ); + CHECK( m_radio->GetStringSelection() == "item 2" ); } TEST_CASE_METHOD(RadioBoxTestCase, "RadioBox::Count", "[radiobox]") { //A trivial test for the item count as items can neither //be added or removed - CPPUNIT_ASSERT_EQUAL(3, m_radio->GetCount()); - CPPUNIT_ASSERT(!m_radio->IsEmpty()); + CHECK( m_radio->GetCount() == 3 ); + CHECK(!m_radio->IsEmpty()); } TEST_CASE_METHOD(RadioBoxTestCase, "RadioBox::SetString", "[radiobox]") @@ -188,8 +188,8 @@ TEST_CASE_METHOD(RadioBoxTestCase, "RadioBox::SetString", "[radiobox]") m_radio->SetString(0, "new item 0"); m_radio->SetString(2, ""); - CPPUNIT_ASSERT_EQUAL("new item 0", m_radio->GetString(0)); - CPPUNIT_ASSERT_EQUAL("", m_radio->GetString(2)); + CHECK( m_radio->GetString(0) == "new item 0" ); + CHECK( m_radio->GetString(2) == "" ); } #endif // wxUSE_RADIOBOX From b0c417db36b7daca3b9b6731588f5b7b3f19ca6a Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 29 Aug 2022 18:39:09 +0200 Subject: [PATCH 3/3] Fix crash when using a wxRadioBox without items in wxOSX wxRadioBox::m_radioButtonCycle is never null -- except when it is, when there are no items at all in the radio box. Handle this case without crashing too by checking for the pointer being valid before using it. Also add a unit test checking that this works correctly. See #22751. --- src/osx/radiobox_osx.cpp | 14 ++++++++++---- tests/controls/radioboxtest.cpp | 15 +++++++++++++++ 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/osx/radiobox_osx.cpp b/src/osx/radiobox_osx.cpp index 2a7b33c827..626b0f5fec 100644 --- a/src/osx/radiobox_osx.cpp +++ b/src/osx/radiobox_osx.cpp @@ -59,18 +59,21 @@ wxRadioBox::~wxRadioBox() wxRadioButton *next, *current; - current = m_radioButtonCycle->NextInCycle(); + current = m_radioButtonCycle; if (current != NULL) { - while (current != m_radioButtonCycle) + // We need to start deleting the buttons from the second one because + // deleting the first one would change the pointers stored in them. + for (current = current->NextInCycle();;) { next = current->NextInCycle(); delete current; + if (next == m_radioButtonCycle) + break; + current = next; } - - delete current; } } @@ -339,6 +342,9 @@ void wxRadioBox::Command( wxCommandEvent& event ) // void wxRadioBox::SetFocus() { + if (!m_radioButtonCycle) + return; + wxRadioButton *current; current = m_radioButtonCycle; diff --git a/tests/controls/radioboxtest.cpp b/tests/controls/radioboxtest.cpp index 1f78e180b3..507ab2e3b1 100644 --- a/tests/controls/radioboxtest.cpp +++ b/tests/controls/radioboxtest.cpp @@ -16,6 +16,7 @@ #include "wx/radiobox.h" #endif // WX_PRECOMP +#include "wx/scopedptr.h" #include "wx/tooltip.h" class RadioBoxTestCase @@ -192,4 +193,18 @@ TEST_CASE_METHOD(RadioBoxTestCase, "RadioBox::SetString", "[radiobox]") CHECK( m_radio->GetString(2) == "" ); } +TEST_CASE("RadioBox::NoItems", "[radiobox]") +{ + wxScopedPtr + radio(new wxRadioBox(wxTheApp->GetTopWindow(), wxID_ANY, "Empty", + wxDefaultPosition, wxDefaultSize, + 0, NULL, + 1, wxRA_SPECIFY_COLS)); + + CHECK( radio->GetCount() == 0 ); + CHECK( radio->IsEmpty() ); + + CHECK_NOTHROW( radio->SetFocus() ); +} + #endif // wxUSE_RADIOBOX