From de6bc5f0629d59102559404a94bb282910506a89 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 10 Jul 2020 19:31:33 +0200 Subject: [PATCH 1/3] Remove useless CppUnit test case class from wxColour unit test No real changes, just get rid of the unnecessary legacy boilerplate. --- tests/graphics/colour.cpp | 29 +++-------------------------- 1 file changed, 3 insertions(+), 26 deletions(-) diff --git a/tests/graphics/colour.cpp b/tests/graphics/colour.cpp index bde3b17803..aad5278258 100644 --- a/tests/graphics/colour.cpp +++ b/tests/graphics/colour.cpp @@ -37,33 +37,10 @@ CPPUNIT_ASSERT_EQUAL( a, (int)c.Alpha() ) // ---------------------------------------------------------------------------- -// test class +// tests // ---------------------------------------------------------------------------- -class ColourTestCase : public CppUnit::TestCase -{ -public: - ColourTestCase() { } - -private: - CPPUNIT_TEST_SUITE( ColourTestCase ); - CPPUNIT_TEST( GetSetRGB ); - CPPUNIT_TEST( FromString ); - CPPUNIT_TEST_SUITE_END(); - - void GetSetRGB(); - void FromString(); - - wxDECLARE_NO_COPY_CLASS(ColourTestCase); -}; - -// register in the unnamed registry so that these tests are run by default -CPPUNIT_TEST_SUITE_REGISTRATION( ColourTestCase ); - -// also include in its own registry so that these tests can be run alone -CPPUNIT_TEST_SUITE_NAMED_REGISTRATION( ColourTestCase, "ColourTestCase" ); - -void ColourTestCase::GetSetRGB() +TEST_CASE("wxColour::GetSetRGB", "[colour][rgb]") { wxColour c; c.SetRGB(0x123456); @@ -96,7 +73,7 @@ void ColourTestCase::GetSetRGB() #endif // __WXX11__ } -void ColourTestCase::FromString() +TEST_CASE("wxColour::FromString", "[colour][string]") { ASSERT_EQUAL_RGB( wxColour("rgb(11, 22, 33)"), 11, 22, 33 ); // wxX11 doesn't support alpha at all currently. From 5d14346325d2ca093b76c8b21d6f5a14e3c3eb39 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 11 Jul 2020 14:24:06 +0200 Subject: [PATCH 2/3] Replace CppUnit assertion macros with Catch ones Also define wxColour-specific matchers to allow comparing RGB(A) channels to the expected values, replacing the old ASSERT_EQUAL_RGB(A) macros. No real changes. --- tests/graphics/colour.cpp | 119 +++++++++++++++++++++++++++----------- 1 file changed, 86 insertions(+), 33 deletions(-) diff --git a/tests/graphics/colour.cpp b/tests/graphics/colour.cpp index aad5278258..ec26cfefa2 100644 --- a/tests/graphics/colour.cpp +++ b/tests/graphics/colour.cpp @@ -20,21 +20,74 @@ #include "asserthelper.h" // ---------------------------------------------------------------------------- -// helpers +// helpers for checking wxColour RGB[A] values // ---------------------------------------------------------------------------- -// Check the colour components, with and without alpha. -// -// NB: These are macros and not functions to have correct line numbers in case -// of failure. -#define ASSERT_EQUAL_RGB(c, r, g, b) \ - CPPUNIT_ASSERT_EQUAL( r, (int)c.Red() ); \ - CPPUNIT_ASSERT_EQUAL( g, (int)c.Green() ); \ - CPPUNIT_ASSERT_EQUAL( b, (int)c.Blue() ) +typedef wxColour::ChannelType ChannelType; -#define ASSERT_EQUAL_RGBA(c, r, g, b, a) \ - ASSERT_EQUAL_RGB(c, r, g, b); \ - CPPUNIT_ASSERT_EQUAL( a, (int)c.Alpha() ) +class ColourRGBMatcher : public Catch::MatcherBase +{ +public: + ColourRGBMatcher(ChannelType red, ChannelType green, ChannelType blue) + : m_red(red), + m_green(green), + m_blue(blue) + { + } + + bool match(const wxColour& c) const wxOVERRIDE + { + return c.Red() == m_red && c.Green() == m_green && c.Blue() == m_blue; + } + + std::string describe() const wxOVERRIDE + { + return wxString::Format("!= RGB(%#02x, %#02x, %#02x)", + m_red, m_green, m_blue).ToStdString(); + } + +protected: + const ChannelType m_red, m_green, m_blue; +}; + +class ColourRGBAMatcher : public ColourRGBMatcher +{ +public: + ColourRGBAMatcher(ChannelType red, ChannelType green, ChannelType blue, + ChannelType alpha) + : ColourRGBMatcher(red, green, blue), + m_alpha(alpha) + { + } + + bool match(const wxColour& c) const wxOVERRIDE + { + return ColourRGBMatcher::match(c) && c.Alpha() == m_alpha; + } + + std::string describe() const wxOVERRIDE + { + return wxString::Format("!= RGBA(%#02x, %#02x, %#02x, %#02x)", + m_red, m_green, m_blue, m_alpha).ToStdString(); + } + +private: + const ChannelType m_alpha; +}; + +inline +ColourRGBMatcher +RGBSameAs(ChannelType red, ChannelType green, ChannelType blue) +{ + return ColourRGBMatcher(red, green, blue); +} + +inline +ColourRGBAMatcher +RGBASameAs(ChannelType red, ChannelType green, ChannelType blue, ChannelType alpha) +{ + return ColourRGBAMatcher(red, green, blue, alpha); +} // ---------------------------------------------------------------------------- // tests @@ -45,51 +98,51 @@ TEST_CASE("wxColour::GetSetRGB", "[colour][rgb]") wxColour c; c.SetRGB(0x123456); - CPPUNIT_ASSERT_EQUAL( 0x56, (int)c.Red() ); - CPPUNIT_ASSERT_EQUAL( 0x34, (int)c.Green() ); - CPPUNIT_ASSERT_EQUAL( 0x12, (int)c.Blue() ); - CPPUNIT_ASSERT_EQUAL( wxALPHA_OPAQUE, c.Alpha() ); + CHECK( c.Red() == 0x56 ); + CHECK( c.Green() == 0x34 ); + CHECK( c.Blue() == 0x12 ); + CHECK( c.Alpha() == wxALPHA_OPAQUE ); - CPPUNIT_ASSERT_EQUAL( wxColour(0x123456), c ); - CPPUNIT_ASSERT_EQUAL( 0x123456, c.GetRGB() ); + CHECK( c == wxColour(0x123456) ); + CHECK( c.GetRGB() == 0x123456 ); c.SetRGBA(0xaabbccdd); - CPPUNIT_ASSERT_EQUAL( 0xdd, (int)c.Red() ); - CPPUNIT_ASSERT_EQUAL( 0xcc, (int)c.Green() ); - CPPUNIT_ASSERT_EQUAL( 0xbb, (int)c.Blue() ); + CHECK( c.Red() == 0xdd); + CHECK( c.Green() == 0xcc); + CHECK( c.Blue() == 0xbb); // wxX11 doesn't support alpha at all currently. #ifndef __WXX11__ - CPPUNIT_ASSERT_EQUAL( 0xaa, (int)c.Alpha() ); + CHECK( c.Alpha() == 0xaa ); #endif // __WXX11__ // FIXME: at least under wxGTK wxColour ctor doesn't take alpha channel // into account: bug or feature? - //CPPUNIT_ASSERT_EQUAL( wxColour(0xaabbccdd), c ); - CPPUNIT_ASSERT_EQUAL( 0xbbccdd, c.GetRGB() ); + //CHECK( c == wxColour(0xaabbccdd) ); + CHECK( c.GetRGB() == 0xbbccdd ); #ifndef __WXX11__ - CPPUNIT_ASSERT_EQUAL( 0xaabbccdd, c.GetRGBA() ); + CHECK( c.GetRGBA() == 0xaabbccdd ); #endif // __WXX11__ } TEST_CASE("wxColour::FromString", "[colour][string]") { - ASSERT_EQUAL_RGB( wxColour("rgb(11, 22, 33)"), 11, 22, 33 ); + CHECK_THAT( wxColour("rgb(11, 22, 33)"), RGBSameAs(11, 22, 33) ); // wxX11 doesn't support alpha at all currently. #ifndef __WXX11__ - ASSERT_EQUAL_RGBA( wxColour("rgba(11, 22, 33, 0.5)"), 11, 22, 33, 128 ); - ASSERT_EQUAL_RGBA( wxColour("rgba( 11, 22, 33, 0.5 )"), 11, 22, 33, 128 ); + CHECK_THAT( wxColour("rgba(11, 22, 33, 0.5)"), RGBASameAs(11, 22, 33, 128) ); + CHECK_THAT( wxColour("rgba( 11, 22, 33, 0.5 )"), RGBASameAs(11, 22, 33, 128) ); #endif // __WXX11__ - ASSERT_EQUAL_RGB( wxColour("#aabbcc"), 0xaa, 0xbb, 0xcc ); + CHECK_THAT( wxColour("#aabbcc"), RGBSameAs(0xaa, 0xbb, 0xcc) ); - ASSERT_EQUAL_RGB( wxColour("red"), 0xff, 0, 0 ); + CHECK_THAT( wxColour("red"), RGBSameAs(0xff, 0, 0) ); wxColour col; - CPPUNIT_ASSERT( !wxFromString("rgb(1, 2)", &col) ); - CPPUNIT_ASSERT( !wxFromString("rgba(1, 2, 3.456)", &col) ); - CPPUNIT_ASSERT( !wxFromString("rgba(1, 2, 3.456, foo)", &col) ); + CHECK( !wxFromString("rgb(1, 2)", &col) ); + CHECK( !wxFromString("rgba(1, 2, 3.456)", &col) ); + CHECK( !wxFromString("rgba(1, 2, 3.456, foo)", &col) ); } TEST_CASE("wxColour::GetLuminance", "[colour][luminance]") From 9890cf6bac143b07a15d5e1009e3a0c51ab1eee9 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 11 Jul 2020 14:31:54 +0200 Subject: [PATCH 3/3] Return empty string from wxColour::GetAsString() if it's invalid This is consistent with wxToString(wxColour) and seems more useful than either returning black string representation (as wxMSW used to do) or asserting (as wxGTK did). Document this behaviour and add a test checking for it. Closes #18623. --- interface/wx/colour.h | 9 +++++---- src/common/colourcmn.cpp | 3 +++ tests/graphics/colour.cpp | 10 ++++++++++ 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/interface/wx/colour.h b/interface/wx/colour.h index 8f3a8535bc..4b35c82598 100644 --- a/interface/wx/colour.h +++ b/interface/wx/colour.h @@ -113,10 +113,11 @@ public: @c wxC2S_HTML_SYNTAX, to obtain the colour as "#" followed by 6 hexadecimal digits (e.g. wxColour(255,0,0) == "#FF0000"). - This function never fails and always returns a non-empty string but - asserts if the colour has alpha channel (i.e. is non opaque) but - @c wxC2S_CSS_SYNTAX (which is the only one supporting alpha) is not - specified in flags. + This function returns empty string if the colour is not initialized + (see IsOk()). Otherwise, the returned string is always non-empty, but + the function asserts if the colour has alpha channel (i.e. is non + opaque) but @c wxC2S_CSS_SYNTAX (which is the only one supporting + alpha) is not specified in @a flags. @note For non-solid (i.e. non-RGB) colour this function returns "rgb(??, ?? ??)" or "#??????". diff --git a/src/common/colourcmn.cpp b/src/common/colourcmn.cpp index 9d0b40a46c..56ed78321a 100644 --- a/src/common/colourcmn.cpp +++ b/src/common/colourcmn.cpp @@ -218,6 +218,9 @@ bool wxColourBase::FromString(const wxString& str) wxString wxColourBase::GetAsString(long flags) const { + if ( !IsOk() ) + return wxString(); + wxString colName; if ( IsSolid() ) diff --git a/tests/graphics/colour.cpp b/tests/graphics/colour.cpp index ec26cfefa2..865d92f662 100644 --- a/tests/graphics/colour.cpp +++ b/tests/graphics/colour.cpp @@ -145,6 +145,16 @@ TEST_CASE("wxColour::FromString", "[colour][string]") CHECK( !wxFromString("rgba(1, 2, 3.456, foo)", &col) ); } +TEST_CASE("wxColour::GetAsString", "[colour][string]") +{ + CHECK( wxColour().GetAsString() == "" ); + + wxColour red("red"); + CHECK( red.GetAsString() == "red" ); + CHECK( red.GetAsString(wxC2S_CSS_SYNTAX) == "rgb(255, 0, 0)" ); + CHECK( red.GetAsString(wxC2S_HTML_SYNTAX) == "#FF0000" ); +} + TEST_CASE("wxColour::GetLuminance", "[colour][luminance]") { CHECK( wxBLACK->GetLuminance() == Approx(0.0) );