search

open-simulation-platform / libcosim

OSP C++ co-simulation library
https://open-simulation-platform.github.io/libcosim
Mozilla Public License 2.0
58 stars 10 forks source link

Replace boost::test with Catch2 #714

Closed markaren closed 1 year ago

markaren commented 2 years ago

This PR replaces boost::test with Catch2. Catch2 is only downloaded if tests are enabled using CMake FetchContent.

The overall aim is to reduce the dependency on boost. IMO, Catch2 to also an easier and prettier framework to use.

Also closes #702 (3.15 is needed for CMake dependency injection)

With this merged, all non-unittests test should be converted to proper unittests.

kyllingstad commented 2 years ago

Why is it a goal to reduce our dependency on Boost? It's probably the most well-known and most used library collection after std. AFAICT, with this PR and #716, we are increasing the number of dependencies, not reducing it.

markaren commented 2 years ago

Why is it a goal to reduce our dependency on Boost?

It's more of a positive side effect. libcosim currently suffers from issues with Boost linking, which may or may not surface depedening on various settings. That could be fixed in other ways, and I'm pretty sure libcosim will never remove it's boost depedendy, but it should atleast try to keep it out of the public API, which #716 helps do. The implementation there could very well be Boost::log, rather than spdlog. But thats a discussion for that PR.

In any case, i see this PR as valuable as it there is one less library to link and the API is easier to use (Catch2 is header only).

kyllingstad commented 1 year ago

I'm sorry, I just don't see the benefit of this. The code isn't significantly cleaner to my eyes—mostly, it's just line-for-line replacing things like BOOST_REQUIRE with REQUIRE, etc. And as I pointed out above, it introduces a new dependency rather than removing one. AFAICT, catch2 is a nice library, but I would expect that it is less mature and less commonly used than Boost.Test.

markaren commented 1 year ago

In 2021 Catch2 was the second most used test framework after Google.test according to Jetbrains 2021 survey. https://www.jetbrains.com/lp/devecosystem-2021/cpp/

image

Anyway, I will close this unless other voices suggests otherwise. The current use of Boost.test should be cleaned up and non-unit tests should be upgraded though.

kyllingstad commented 1 year ago

I take back the "less used" argument then. :) The "little improvement" and "extra dependency" arguments are sufficient for me. But let's see what others say.

Aside: I do think there is a sub-surface part of the iceberg of C++ developers and projects which the JetBrains survey doesn't reach. Let's call them the "old school" C++ community. But of course, I don't have any actual data to back this up with, so I should probably shut up now.