[SYCL][Test] Make testsuite respect SYCL_LIB_DUMPS_ONLY #14879
Conversation
|
An alternative to this is to teach the standalone header format about suffixes: diff --git a/sycl/test/format.py b/sycl/test/format.py
index 19d16a263426..b64f41844222 100644
--- a/sycl/test/format.py
+++ b/sycl/test/format.py
@@ -23,6 +23,8 @@ class SYCLHeadersTest(lit.formats.TestFormat):
for filename in filenames:
if not filename.endswith(".hpp"):
continue
+ if os.path.splitext(filename)[1] not in litConfig.suffixes:
+ continue
filepath = os.path.join(dirpath, filename)
if headers_filter is not None: |
Running the check-sycl-dumps target sets the SYCL_LIB_DUMPS_ONLY param for lit which should only enable the ABI dump tests. However, since 1fde656 we've added a non `lit.formats.ShTest()` which doesn't respect config.suffixes, so we need to account for this.
ldrumm
force-pushed
the
SYCL_LIB_DUMPS_ONLY
branch
from
0d382af to
1d5a436
Compare
This might actually be a better approach, because instead of introducing a custom logic for skipping certain tests, it would mean that tests won't launch themselves thanks to existing/standard/built-in mechanism of filters. The latter is a cleaner design, I think |
|
On Thu Aug 1, 2024 at 10:46 AM BST, Alexey Sachkov wrote:
The latter is a cleaner design, I think
Yeah. Let's go with that. I'll redo it.
|
AlexeySachkov
changed the title
Done :) |
Thanks Alexey. |
We use these tags at least for creating release notes. @bader can provide more reasons if any. |
|
The original idea is these patches are supposed to be upstreamed to llvm/llvm-project repository. In order to do that we are trying to follow LLVM's guidelines and policies. This requirement is specified in https://llvm.org/docs/DeveloperPolicy.html#commit-messages. |
|
CUDA failures seem down to device detection unrelated to this patch @intel/llvm-gatekeepers please merge |
sure thing ;) |
| for filename in filenames: | ||
| if not filename.endswith(".hpp"): | ||
| suffix = os.path.splitext(filename)[1] | ||
| if suffix not in SUFFIXES or suffix not in litConfig.suffixes: |
There was a problem hiding this comment.
@ldrumm, I've just realized that this patch had completely disable the testing suite :)
We don't have .hpp registered as a suffix and therefore we always skip all those tests. This is more of a FYI, I'm working on a revamp/expansion of the suite in scope of #16012, so I will fix it as part of that effort
We have a suite to check that every header contains enough `#include` directives and forward declarations so that it can be included standalone and used. However, that suite got accidentally disabled some time ago in #14879 This PR re-enabled the suite and fixed all issues it detected. Besides that, the suite is upgraded: - it now scans source directory, meaning that it won't complain about headers which were removed from the codebase, but left in build folder - it is now possible to have add a custom test instead of auto-generated one: this is useful when you want to trigger certain template instantiations to make sure that all code paths are covered
4 participants
Running the check-sycl-dumps target sets the SYCL_LIB_DUMPS_ONLY param for lit which should only enable the ABI dump tests. However, since 1fde656 we've added a non
lit.formats.ShTest()which doesn't respect config.suffixes, so we need to account for this.