Skip to content

Rework Configuration-Time Test Category Filtering Mechanism #192

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

Merged
merged 1 commit into from
Oct 14, 2021

Conversation

psalz
Copy link
Contributor

@psalz psalz commented Oct 5, 2021

I'm proposing to change the mechanism for filtering the test categories (= subfolders inside tests/ containing a CMakeLists.txt file) during configuration time.

Whereas the previous approach (SYCL_CTS_TEST_FILTER) used to specify all test categories that are to be included in the compilation, I'm proposing to do the opposite, i.e., list all categories that are to be excluded. To underline this change, I've renamed the CMake config parameter to SYCL_CTS_EXCLUDE_TEST_CATEGORIES.

Switching from inclusion to exclusion has the benefit that (when a filter is used) newly added tests are being compiled by default, instead of having to be added to a list manually. If we end up compiling tests during CI (which I'm still very much planning on doing), we will have to limit the set of tests compiled by each implementation somehow (as no implementation can currently compile all tests). Using an exclusion filter will therefore highlight potential problems for different SYCL implementations early.

I've added filters for ComputeCpp, DPCPP and hipSYCL, reflecting their current state of CTS support. It would be great if someone could confirm these on their setup (either use them and check that everything else compiles, or ideally re-generate the filters -- see below).

Having these filters within the repository for each implementation also serves as a nice way of tracking the current state of CTS support, where an empty list indicates that all tests compile successfully. It also shows which test categories we may want to focus on most. For example, currently none of the implementations seem to be able to compile the accessor tests.

As tests and implementations improve over time, these filters should be regenerated and updated within the repository. To streamline this process, I'm introducing a new utility script called generate_exclude_filter.py that makes this rather simple. Basically, the script attempts to compile all targets for a given SYCL implementation, spitting out the failing ones, which can then be used as a filter.

I've removed the ability to specify a filter when generating a conformance report using the run_conformance_tests.py script. Frankly I never quite understood why this feature was there (and mandatory!) in the first place. We can re-introduce it should the need arise.

Lastly, I've removed the conformance_filters directory containing the core.csv and codeplay.csv filters.

Replace `SYCL_CTS_TEST_FILTER`, which specifies a list of test
categories to compile, by `SYCL_CTS_EXCLUDE_TEST_CATEGORIES`, which does
the opposite.

Introduce `generate_exclude_filter.py` script for automatically
generating filters for a given SYCL implementation.

This includes generated filters for all three supported SYCL
implementations, using the versions currently used for CI runs.
@bader bader requested a review from ProGTX October 11, 2021 12:53
@psalz
Copy link
Contributor Author

psalz commented Oct 12, 2021

Any more concerns with this, or can I go ahead and merge?

@bader
Copy link
Contributor

bader commented Oct 12, 2021

Any more concerns with this, or can I go ahead and merge?

@ProGTX requested changes, so I think we need to get his approval to merge this.

@psalz psalz merged commit fa63d3d into KhronosGroup:SYCL-2020 Oct 14, 2021
@psalz psalz deleted the rework-config-time-filtering branch October 14, 2021 13:08
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.

4 participants