Skip to content

Conversation

@ThomasAdam
Copy link
Contributor

@ThomasAdam ThomasAdam commented Aug 7, 2025

Hi,

This PR introduces the following commits:

  • build: cmake: enable tests

This adds the ability to compile tests as part of using CMake.

Note that, I've had to unwrap some ucl_*() calls from assert() (see comment below on this PR.)

I've also explicitly disabled the tests from running on Windows, as they've not been written with that in mind. However, I have still changed some of the tests in terms of their headers, should we revisit this in the future.

  • build: github: add flags to ctest

This adds a few flags to Github actions to make ctest easier to deal with.

  • build: configure: remove duplicate test

I noticed a duplicate test; msgpack. I presume this is meant to be
test_streaming but this test is broken currently, and hasn't explicitly been running as a test (only compiled).

@ThomasAdam
Copy link
Contributor Author

It looks as though this is failing due to -DCMAKE_BUILD_TYPE=Release -- will check later why this is causing the generate test to fail.

@ThomasAdam ThomasAdam force-pushed the ta/cmake-tests branch 10 times, most recently from a9ef5ea to 9bcb1c0 Compare August 8, 2025 16:46
When using cmake, ensure the tests are available.
Add --progress and --output-on-failure to the ctest step so that we get
more information out of the test suite when it runs.
The msgpack test was being included more than once.
@ThomasAdam
Copy link
Contributor Author

This PR is now ready for review.

The reason the generate test was failing is due to the fact that when you do a CMAKE_BUILD_TYPE=Release, this explicitly passes -DNDEBUG which will disable the use of assert().

There's definitely something possibly amiss here, as this is a bit of an odd situation.

For now, I've explicitly removed some url_*() calls which were being wrapped by assert().

I don't think it'll be a big issue overall.

But the tests could do with some love, I know...

@vstakhov vstakhov merged commit 8c72ca7 into vstakhov:master Nov 4, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants