Add feature to disable building select C++ tests#57
Closed
rtmadduri wants to merge 2 commits intoROCm:amd-integrationfrom
Closed
Add feature to disable building select C++ tests#57rtmadduri wants to merge 2 commits intoROCm:amd-integrationfrom
rtmadduri wants to merge 2 commits intoROCm:amd-integrationfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds the ability to selectively disable building and running specific C++ HIP tests to prevent failures from breaking the amd-integration build. Tests that are under active development can be skipped during CI builds while still being compilable individually for testing purposes.
Key changes:
- Introduces a
SKIP_TESTSvariable containing a list of tests to exclude from the build - Implements skip logic in the HIP test configuration loop to filter out excluded tests
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
diptorupd
reviewed
Nov 17, 2025
| foreach(test_source IN LISTS HIP_TEST_SOURCES) | ||
| get_filename_component(test_name ${test_source} NAME_WE) | ||
|
|
||
| if(test_name IN_LIST SKIP_TESTS) |
Collaborator
There was a problem hiding this comment.
I do not think this is the right approach. I will still like to have build targets for the broken tests. Having a build target will help during development when a broken test is getting fixed or otherwise needs reevaluation.
I suggest the target should not be added to the build_tests convenience target that builds all tests in one shot. That way CI or otherwise can still build all "good" tests in one go, but a developer has the option to easily try out a broken test using cmake.
Collaborator
Author
There was a problem hiding this comment.
Are you suggesting something like
foreach(test_target IN LISTS ALL_TEST_TARGETS)
if(TARGET ${test_target})
if(test_name IN_LIST SKIP_TESTS)
message(STATUS "Skipping HIP test: ${test_name}")
continue()
endif()
gtest_discover_tests(${test_target})
endif()
endforeach()
Collaborator
Author
There was a problem hiding this comment.
Wouldnt this only work for tests that build successfully but fail during runtime? Right now test_batch_prefill does not even build because changes to BatchPrefillWithPagedKVCache did not go in yet.
demandal25
pushed a commit
that referenced
this pull request
Nov 18, 2025
The PR adds a list to tests to skip from the CMake `build_tests` target. The tests in the skip list can still be individually built. E.g. ```bash # The `test_batch_prefill.cpp` is currently broken and added to the skip list. cmake -DFLASHINFER_ENABLE_HIP=ON -DFLASHINFER_UNITTESTS=ON -GNinja .. ninja build_tests # does not build the test_batch_prefill.cpp tests # The test file can still be built individually using ninja test_batch_prefill_hip ``` Also added the fix to `mma_debug_utils_hip.hpp` from #52 Supersedes #52, #57
Collaborator
|
Superseded by #59 |
diptorupd
pushed a commit
that referenced
this pull request
Dec 5, 2025
This PR ports over the JIT build infra to start using the gpu_iface. With this PR, we have completely ported over all of Flashinfer ROCm to use gpu_iface and we will soon deprecate the HIP headers - [x] Installed JIT and verified JIT installation is working through `gpu_iface` by running `test_batch_decode_example.py` - [x] Installed AOT and verified AOT installation is working through `gpu_iface` by running `test_batch_decode_example.py` - [x] Verified C++ Unit Tests
diptorupd
added a commit
that referenced
this pull request
Dec 5, 2025
The PR adds a list to tests to skip from the CMake `build_tests` target. The tests in the skip list can still be individually built. E.g. ```bash # The `test_batch_prefill.cpp` is currently broken and added to the skip list. cmake -DFLASHINFER_ENABLE_HIP=ON -DFLASHINFER_UNITTESTS=ON -GNinja .. ninja build_tests # does not build the test_batch_prefill.cpp tests # The test file can still be built individually using ninja test_batch_prefill_hip ``` Also added the fix to `mma_debug_utils_hip.hpp` from #52 Supersedes #52, #57
zhenhantech
pushed a commit
to zhenhantech/flashinfer
that referenced
this pull request
Jan 9, 2026
This PR ports over the JIT build infra to start using the gpu_iface. With this PR, we have completely ported over all of Flashinfer ROCm to use gpu_iface and we will soon deprecate the HIP headers - [x] Installed JIT and verified JIT installation is working through `gpu_iface` by running `test_batch_decode_example.py` - [x] Installed AOT and verified AOT installation is working through `gpu_iface` by running `test_batch_decode_example.py` - [x] Verified C++ Unit Tests
zhenhantech
pushed a commit
to zhenhantech/flashinfer
that referenced
this pull request
Jan 9, 2026
The PR adds a list to tests to skip from the CMake `build_tests` target. The tests in the skip list can still be individually built. E.g. ```bash # The `test_batch_prefill.cpp` is currently broken and added to the skip list. cmake -DFLASHINFER_ENABLE_HIP=ON -DFLASHINFER_UNITTESTS=ON -GNinja .. ninja build_tests # does not build the test_batch_prefill.cpp tests # The test file can still be built individually using ninja test_batch_prefill_hip ``` Also added the fix to `mma_debug_utils_hip.hpp` from ROCm#52 Supersedes ROCm#52, ROCm#57
diptorupd
pushed a commit
to diptorupd/flashinfer
that referenced
this pull request
Jan 28, 2026
This PR ports over the JIT build infra to start using the gpu_iface. With this PR, we have completely ported over all of Flashinfer ROCm to use gpu_iface and we will soon deprecate the HIP headers - [x] Installed JIT and verified JIT installation is working through `gpu_iface` by running `test_batch_decode_example.py` - [x] Installed AOT and verified AOT installation is working through `gpu_iface` by running `test_batch_decode_example.py` - [x] Verified C++ Unit Tests
diptorupd
added a commit
to diptorupd/flashinfer
that referenced
this pull request
Jan 28, 2026
The PR adds a list to tests to skip from the CMake `build_tests` target. The tests in the skip list can still be individually built. E.g. ```bash # The `test_batch_prefill.cpp` is currently broken and added to the skip list. cmake -DFLASHINFER_ENABLE_HIP=ON -DFLASHINFER_UNITTESTS=ON -GNinja .. ninja build_tests # does not build the test_batch_prefill.cpp tests # The test file can still be built individually using ninja test_batch_prefill_hip ``` Also added the fix to `mma_debug_utils_hip.hpp` from ROCm#52 Supersedes ROCm#52, ROCm#57
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR makes changes to
libflashinfer/tests/CMakeLists.txtto allow for disabling select C++ tests.Part of the C++ test suite is currently being used to facilitate active development. As a result, some of the tests fail during runtime. To prevent
amd-integrationfrom being broken, this PR adds a feature to disable building and running specific C++ tests.These tests can be compiled directly or individually for testing.