Tests: Enabling CTest alongside module testing with import std
Builds on changes from #2248. Progress for #1993. Resolves #2331.
The tests were not well isolated and had to go through hoops to get e.g. modules with separate compilation flags.
They also did not use any of the convenient options that are provided in our top-level CMakeLists.txt.
Running the tests via CTest gives us full control over them, avoiding current issues like compile definitions bleeding over, adding special compilation flags for certain tests (partiularly for dynamic vs static dispatch testing, it was previously impossible to run both). It will also allow to selectively enable import std (soon).
CTest also allows running individual tests only, making debugging easier. Also leads to easier error reading in CI logs by showing exactly which tests fail.
I've tried to logically separate the commits, but here's a summary of the changes:
- All targets in tests now use the default
Vulkan::HpporVulkan::HppModule(or Video variant), with no manual include directories that muddy the waters. The target setup was placed intovulkan_hpp__setup_vulkan_targetsto get consistent path setups, no matter where it is included from. Modules would occasionally break until now due to relative paths, for example. - Tests use e.g.
set(VULKAN_HPP_NO_CONSTRUCTORS ON)as settings, rather than manually setting compile definitions. This required raising the minimum CMake version to 3.13 for the proper policy. - More module tests were fixed and enabled
- CI was updated to run ctest
Note that the current setup will, unlike before, try to test both dynamic and static dispatch. I don't know how CI will handle this and I remember there being an issue with an internal git repo before. Likely a good idea to just conditionally disable static dispatch testing again before merging.
Where have all the tests gone?
In VS I just have the UniqueHandle test, that's all.
UniqueHandle is the only test that I have not converted to CTest, as it requires lots of dependency juggling and is not really standalone.
The tests are handled by the CTest executable, should be installed alongside CMake. Does VS not have CMake tools integration?
If you want to use it manually, cd into your build folder and do ctest -j and it should work.
@asuessenbach if you are used to the workflow of having add_subdirectory for testing and want to keep it that way, let me know! I will redo the PR without CTest integration. I just thought to attempt the conversion as CTest seems pretty standard across CMake projects.
Having CTests would be cool. But not having the test files available in the VS solution feels wrong. Can't we have both, CTest and files listed in VS? If that's not (easily) possible, I'd prefer to not use CTest for now.
I will try to find a solution to have both.
As for CTest in VS, you can view all the tests with the Test Explorer (Test > Test Explorer).
Under the same menu in Test, you can find Run CTests for VulkanHppGenerator if you just want to run all tests (it seems to not run them in parallel though).
Most of the tests can now handle modules and import std. Remains to be seen if CI is able to handle it (currently only enabled for windows and ubuntu-ext).
All tests can be built as before via subdirectory (default behavior) or optionally be built via CTest instead as via VULKAN_HPP_TESTS_CTEST. Can you check if this works as expected for your workflow @asuessenbach?
Default setup of the std module is now handled in vulkan.hpp directly, so that the module no longer needs to include the vulkan_hpp_macro.hpp header. Module fails to build at current head due to lack of this definition, so give it a quick read if it's okay @sharadhr!
so that the module no longer needs to include the
vulkan_hpp_macro.hppheader
It should; we still have VULKAN_HPP_STRINGIFY for the warnings! But this wasn't removed anyway...
Also, I'm not quite able to parse this statement:
Module fails to build at current head due to lack of this definition
In which scenario does the module fail to build?
@sharadhr the error I get is:
In file included from Vulkan-Hpp/vulkan/vulkan.cppm:28:
Vulkan-Hpp/vulkan/vulkan.hpp:41:8: fatal error: module 'VULKAN_HPP_STD_MODULE' not found
41 | import VULKAN_HPP_STD_MODULE;
but you are right, it does include the macro header (was I drunk?). It is once again an issue with the
defined( __cpp_modules ) && defined( __cpp_lib_modules )
so this could probably just be shortened to check for VULKAN_HPP_CXX_MODULE instead (defined at top of vulkan.cppm). What do you think?
@asuessenbach do you know of a way to test static dispatch in CI? It obviously won't be able to find Vulkan packages, so it likely isn't possible (same as before).
D:\a\Vulkan-Hpp\Vulkan-Hpp\tests\ExtensionInspection\ExtensionInspection.cpp(82,100): warning C4996: 'vk::EXTDebugReportExtensionName': The VK_EXT_debug_report extension has been deprecated by VK_EXT_debug_utils.
D:\a\Vulkan-Hpp\Vulkan-Hpp\tests\ExtensionInspection\ExtensionInspection.cpp(86,98): warning C4996: 'vk::AMDNegativeViewportHeightExtensionName': The VK_AMD_negative_viewport_height extension has been obsoleted by VK_KHR_maintenance1.
Warnings pop up for both VK_EXT_debug_report and VK_AMD_negative_viewport_height.
It tests for whether these are deprecated, but throws the warning regardless, simply because the enum is used, e.g.:
static_assert( vk::isObsoletedExtension( vk::AMDNegativeViewportHeightExtensionName ), "static assert test failed" );
static_assert( vk::getExtensionObsoletedBy( vk::AMDNegativeViewportHeightExtensionName ) == vk::KHRMaintenance1ExtensionName,
"static assert test failed" );
The guard for these deprecation tests is also a bit peculiar, why the restrictions?
#if ( 201907 <= __cpp_constexpr ) && ( !defined( __GNUC__ ) || ( 110400 < GCC_VERSION ) )
Current state of my descent into CI debugging insanity:
- Clang++-20: CMake does not provide the
[[ TARGET __CMAKE::CXX23 ]]target. - Clang++-19: CMake does provide the
[[ TARGET __CMAKE::CXX23 ]]target, but has issues with the standard library (using-stblib=libc++) as seen here: https://github.com/M2-TE/Vulkan-Hpp/actions/runs/18653373179/job/53176398437. Seems to affect compilation ofutils.hpp.
I believe this is because it defaults to an older standard library version, as clang is installed in the runners. Overriding the specific libc++ version seems like a pain: https://releases.llvm.org/14.0.0/projects/libcxx/docs/UsingLibcxx.html
Identical setup in ubuntu:24.04 docker setup works just fine with modules, so not exactly sure what is different.
I think it's best to postpone the CI testing of modules. I will work on this as a separate PR, so things don't get too messy. All the tests will now work with a properly set up toolchain that can handle modules and import std.
Some other things added:
- Properly set the build type on windows via
--config $BUILD_TYPE - Fixed the extension inspection test on windows by ignoring the deprecation warning via pragma
With this, I would say the PR is pretty much complete. The concerns discussed with @sharadhr related to CI testing should be handled in a separate PR imo. On windows, building with Ninja seems to still have an issue or two in CI anyways.
Side note: I realised #warning is in the C++23 standard, but MSVC hasn't implemented it yet (sigh).
Brought the PR up-to-date and managed to get module testing working. C++Modules are being tested via ~Ninja+Clang on Windows~ and Ninja+Clang on ubuntu-ext. Testing on MSVC would required a separate runner that uses Ninja (this PR already ballooned enough as is).
PR is ready for review, so please let me know what you think @sharadhr.
A cursory glance shows no glaring issues. That singular build failure should probably be resolved by rebasing atop main. However I am surprised it did not fail on any other platform, especially Windows + MSVC, which does support import std. There might be a missing test case scenario; see this action failure. (I've been thinking about using that CI setup, which does seem very comprehensive in terms of platform coverage, and yet quite simple in its implementation).
I do recall there was discussion of splitting up this PR: one to set up CTest, and another to set up the module tests.
Well at least now we know that the module tests work ;)
But yes I remember us talking about splitting this up. The changes here are quite interlinked (CTest and Modules), so if it is still desired to split it into two separate PRs, I would simply exclude the module-related things from CI. Is that acceptable or should I tear these apart on a deeper level?
I am surprised it did not fail on any other platform
I only "activated" the ubuntu-ext one so far. Windows Clang has too old of a CMake version, same for MSVC. Since you mentioned wanting to adopt those other CI workflows, I simply focused on having one single working test to improve the current.. lack of module tests.
I do quite like those workflows too, so I would vote to adopt those as well! Hopefully after merging this though, upkeep is a bit of work with the merge conflicts..
Windows Clang has too old of a CMake version, same for MSVC
This is surprising! Visual Studio bundles CMake 4.1. Or we can always install the upstream CMake with the same get-cmake workflow.
Yea I was suprised as well. Check the GitHub hosted runners: Windows 2022 and Windows 2025.
We can probably just use a baseline like
- name: Setup CMake and Ninja
if: matrix.env.modules
uses: lukka/get-cmake@latest
with:
cmakeVersion: 4.1.2 # (or latest?)
ninjaVersion: latest
Should be a better solution than the get-cmake thing I've done before.
I would simply exclude the module-related things from CI. Is that acceptable or should I tear these apart on a deeper level?
I think just having the module stuff separate would be sufficient. Although I am decidedly not the final arbiter, and maybe @asuessenbach should also input.
It seems, you're running this PR on a slightly outdated Vulkan-Headers version. Would be great, if you'd rebase it accordingly.
I would simply exclude the module-related things from CI. Is that acceptable or should I tear these apart on a deeper level?
I think just having the module stuff separate would be sufficient. Although I am decidedly not the final arbiter, and maybe @asuessenbach should also input.
Agreed, "simply excluding the module-related things from CI" is probably the right thing to do.
Agreed, "simply excluding the module-related things from CI" is probably the right thing to do.
Done!
It seems, you're running this PR on a slightly outdated Vulkan-Headers version
It should be on the newest version, as I've kept it up to date with main and it is currently at 2fa2034. Could you check it again or tell me which commit hash it should be using @asuessenbach?
It should be on the newest version
You're right. Don't know what I've seen before. Sorry, please.
I had accidentally removed the ctest calls for Windows CI before.. sorry about that!