More CppUnit tests upgraded to Catch2 (PR #25758)

Blake-Madden

unread,

Sep 2, 2025, 6:11:54 PM (5 days ago) Sep 2

to wx-...@googlegroups.com, Subscribed

You can view, comment on, or merge this pull request online at:

https://github.com/wxWidgets/wxWidgets/pull/25758

Commit Summary

d78c6d1 More CppUnit tests upgraded to Catch2

File Changes

(7 files)

M tests/fontmap/fontmaptest.cpp (28)
M tests/formatconverter/formatconvertertest.cpp (316)
M tests/geometry/region.cpp (54)
M tests/graphics/affinematrix.cpp (53)
M tests/log/logtest.cpp (3)
M tests/misc/garbage.cpp (106)
M tests/thread/tls.cpp (43)

Patch Links:

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

VZ

unread,

Sep 2, 2025, 6:21:19 PM (5 days ago) Sep 2

to wx-...@googlegroups.com, Subscribed

@vadz commented on this pull request.

Thanks for the updates!

Could you please just make the names of the tests more consistent with the existing ones?

In tests/fontmap/fontmaptest.cpp:

> -    CPPUNIT_TEST_SUITE_END();
-
-    void NamesAndDesc();
-
-    wxDECLARE_NO_COPY_CLASS(FontMapperTestCase);
-};
-
-// register in the unnamed registry so that these tests are run by default
-CPPUNIT_TEST_SUITE_REGISTRATION( FontMapperTestCase );
-
-// also include in its own registry so that these tests can be run alone
-CPPUNIT_TEST_SUITE_NAMED_REGISTRATION( FontMapperTestCase, "FontMapperTestCase" );
-
-
-void FontMapperTestCase::NamesAndDesc()
+TEST_CASE("Names And Desc", "[fontmapper]")

I'd rather avoid using spaces in the test names as this makes it less convenient to run them from command line (they need to be quoted). And it is also convenient to use common prefix for the related tests. So maybe something like this:

⬇️ Suggested change

-TEST_CASE("Names And Desc", "[fontmapper]")
+TEST_CASE("wxFontMapper::NamesAndDesc", "[fontmapper]")

In tests/formatconverter/formatconvertertest.cpp:

> +    // all of them are unused in some build configurations
+    wxUnusedVar(expectedScanf);
+    wxUnusedVar(expectedUtf8);
+    wxUnusedVar(expectedWcharUnix);
+    wxUnusedVar(expectedWcharWindows);

It could be useful to use INFO() to log all of those: this will give useful information in case of a test failure and will avoid the need for these lines.

In tests/formatconverter/formatconvertertest.cpp:

>  }
 
-void FormatConverterTestCase::format_hd()
+TEST_CASE("format_hd", "[formatconverter]")

For consistency I'd use

⬇️ Suggested change

-TEST_CASE("format_hd", "[formatconverter]")
+TEST_CASE("Format::hd", "[formatconverter]")

here and below.

In tests/geometry/region.cpp:

>      (
-        "Default constructed region must be invalid",
-        !r.IsOk()
+        // Default constructed region must be invalid

Minor, but I don't think it makes sense to keep comments inside CHECKs: messages had to be there because they were used by the macro, but this is ignored by CHECK, so could just as well move it above it to make the check shorter and more readable.

In tests/graphics/affinematrix.cpp:

> -    CPPUNIT_TEST_SUITE_END();
-
-    void InvertMatrix();
-    void Concat();
-
-    wxDECLARE_NO_COPY_CLASS(AffineTransformTestCase);
-};
-
-// register in the unnamed registry so that these tests are run by default
-CPPUNIT_TEST_SUITE_REGISTRATION( AffineTransformTestCase );
-
-// also include in its own registry so that these tests can be run alone
-CPPUNIT_TEST_SUITE_NAMED_REGISTRATION( AffineTransformTestCase, "AffineTransformTestCase" );
-
-void AffineTransformTestCase::InvertMatrix()
+TEST_CASE("Invert Matrix", "[affine-transform]")

⬇️ Suggested change

-TEST_CASE("Invert Matrix", "[affine-transform]")
+TEST_CASE("AffineTransform::InvertMatrix", "[affine-transform]")

In tests/thread/tls.cpp:

> @@ -72,54 +72,31 @@ class TLSTestThread : public wxThread
 // test class
 // ----------------------------------------------------------------------------
 
-class TLSTestCase : public CppUnit::TestCase
+TEST_CASE("Int", "[tls]")

⬇️ Suggested change

-TEST_CASE("Int", "[tls]")
+TEST_CASE("TLS::Int", "[tls]")

—
Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.

Blake-Madden

unread,

Sep 2, 2025, 6:37:48 PM (5 days ago) Sep 2

to wx-...@googlegroups.com, Subscribed

Closed #25758.

—
Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.

Lauri Nurmi

unread,

Sep 3, 2025, 2:00:56 AM (5 days ago) Sep 3

to wx-...@googlegroups.com, Subscribed

lanurmi left a comment (wxWidgets/wxWidgets#25758)

FYI: You do not need to create a new PR if you want to change something in an existing one. Just push/force-push to your branch.