Skip to content

Conversation

@bneradt
Copy link
Contributor

@bneradt bneradt commented Aug 13, 2025

This updates our Catch2 version from v2.13.8 to the latest v3.9.1. It also transitions us from the deprecated monolithic header file to the new library model using CMake via FetchContent.

This has several advantages:

  • This uses the library model which the authors of Catch2 advocate for. The monolithic header we used before this patch is not "the primarily supported option".
  • Since not all of catch.hpp is built for every UT, compilation times are quicker.
  • Since the new v3 version provides a main function definition via Catch2::Catch2WithMain, we no longer need the empty unit_test_main.cc files we had before.
  • The updated Catch v3 is purported to execute its checks more efficiently (according to the changelog).

As a part of this transition, this removes
src/records/unit_tests/unit_test_main_on_eventsystem.cc since that was added with #7704 for DynamicStats which has since been removed. In v2, this simply didn't run any tests. In v3, this failed because no tests were run.


Note:

  • This is a unit test only change. Aside from adding const to operator swoc::IPAddr() in ink_inet.h
  • Everyone of our tests have at least their includes updated, but the vast majority of the file changes is simply the addition of Catch2 itself into lib/.

@bneradt bneradt added this to the 10.2.0 milestone Aug 13, 2025
@bneradt bneradt requested review from JosiahWI and cmcfarlen August 13, 2025 19:59
@bneradt bneradt self-assigned this Aug 13, 2025
@bneradt bneradt added the Tests label Aug 13, 2025
@bneradt bneradt force-pushed the catch2_update_to_v3 branch 3 times, most recently from 65e8dcb to 5714a57 Compare August 13, 2025 22:13
@bneradt bneradt added the catch2 label Aug 13, 2025
@bneradt bneradt force-pushed the catch2_update_to_v3 branch 3 times, most recently from eef9f6a to 15f9bee Compare August 14, 2025 13:29
cmcfarlen
cmcfarlen previously approved these changes Aug 19, 2025
Copy link
Contributor

@cmcfarlen cmcfarlen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating our catch2 library!

This updates our Catch2 version from v2.13.8 to the latest v3.9.1. It
also transitions us from the deprecated monolithic header file to the
new library model using CMake via FetchContent.

This has several advantages:

* This uses the library model which the authors of Catch2 advocate for.
  The monolithic header we used before this patch is not "the primarily
  supported option".
* Since not all of catch.hpp is built for every UT, compilation times
  are quicker.
* Since the new v3 version provides a main function definition via
  Catch2::Catch2WithMain, we no longer need the empty unit_test_main.cc
  files we had before.
* The updated Catch v3 is purported to execute its checks more
  efficiently (according to the changelog).

As a part of this transition, this removes
src/records/unit_tests/unit_test_main_on_eventsystem.cc since that was
added with apache#7704 for DynamicStats which has since been removed. In v2,
this simply didn't run any tests. In v3, this failed because no tests
were run.
@bneradt bneradt merged commit 36f9095 into apache:master Aug 19, 2025
15 checks passed
@bneradt bneradt deleted the catch2_update_to_v3 branch August 19, 2025 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants