Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add integration testing #1423

Merged
merged 1 commit into from
Dec 27, 2023
Merged

Add integration testing #1423

merged 1 commit into from
Dec 27, 2023

Conversation

juan-lunarg
Copy link
Contributor

Test find_package(VulkanLoader CONFIG)

Minimal vulkan.pc testing is also added.

Test find_package(VulkanLoader CONFIG)

Minimal vulkan.pc testing is also added.
@ci-tester-lunarg
Copy link

CI Vulkan-Loader build queued with queue ID 107231.

@ci-tester-lunarg
Copy link

CI Vulkan-Loader build # 2431 running.

@ci-tester-lunarg
Copy link

CI Vulkan-Loader build # 2431 passed.

@charles-lunarg charles-lunarg merged commit 741a9df into KhronosGroup:main Dec 27, 2023
42 checks passed
@juan-lunarg juan-lunarg deleted the juan/integration_testing branch December 27, 2023 22:20
Copy link
Contributor Author

@juan-lunarg juan-lunarg left a comment

Choose a reason for hiding this comment

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

@ncesario-lunarg the utility libraries and vulkan headers also have similar integration testing FWIW.

@@ -70,11 +70,6 @@ jobs:
- run: cmake --build build
- run: ctest --output-on-failure --test-dir build/
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI @ncesario-lunarg who is looking to do similar work in glslang.

ctest now handles the find_package integration testing. Making it require little to no CI effort.

Comment on lines -73 to -77
# NOTE: This check is NOT sufficient to ensure vulkan.pc is actually valid
- name: Check vulkan.pc exists and validate it
run: |
cat /tmp/lib/pkgconfig/vulkan.pc
pkg-config --validate /tmp/lib/pkgconfig/vulkan.pc
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The logic is now tested by ctest

Comment on lines +94 to +98
# Test installation
set(test_install_dir "${CMAKE_CURRENT_BINARY_DIR}/install")
add_test(NAME integration.install
COMMAND ${CMAKE_COMMAND} --install ${PROJECT_BINARY_DIR} --prefix ${test_install_dir} --config $<CONFIG>
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

NOTE: You can go 1 step further and test the BOM (build of materials).

See https://cmake.org/cmake/help/latest/prop_test/PASS_REGULAR_EXPRESSION.html

The CMake book covers this fairly well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also I have an example in VVL:

find_program(GNU_NM NAMES nm)
if (GNU_NM)
    # Ensure ANativeActivity_onCreate is being exported
    add_test(NAME ANativeActivity_onCreate COMMAND ${GNU_NM} --dynamic $<TARGET_FILE:vk_layer_validation_tests>)
    set_tests_properties(ANativeActivity_onCreate
        PROPERTIES PASS_REGULAR_EXPRESSION "T ANativeActivity_onCreate"
    )

    # Ensure compatibility with older Android loaders
    add_test(NAME vkEnumerateDeviceExtensionProperties COMMAND ${GNU_NM} --dynamic $<TARGET_FILE:vvl>)
    set_tests_properties(vkEnumerateDeviceExtensionProperties
        PROPERTIES PASS_REGULAR_EXPRESSION "T vkEnumerateDeviceExtensionProperties"
    )

    # Ensure compatibility with older Android loaders
    add_test(NAME vkEnumerateDeviceLayerProperties COMMAND ${GNU_NM} --dynamic $<TARGET_FILE:vvl>)
    set_tests_properties(vkEnumerateDeviceLayerProperties
        PROPERTIES PASS_REGULAR_EXPRESSION "T vkEnumerateDeviceLayerProperties"
    )
endif()


# find_package testing currently doesn't work well under cross-compilation scenarios.
if (CMAKE_CROSSCOMPILING OR
NOT CMAKE_SIZEOF_VOID_P EQUAL 8)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This 1 line was added to make 32 bit builds work without issue. I'm not a fan and it could potentially be handled more elegantly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

NOTE: The testing here is SUPER minimal.

glslang would need more of course.

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.

3 participants