[test] Improve testing of Swift features #76740
Draft
+1,053
−258
Conversation
This file contains 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
|
@swift-ci please test |
drodriguez
force-pushed
the
test-swift-features
branch
from
e7c0602
to
14e2017
Compare
|
@swift-ci please test |
|
This seems like a good idea to me. |
tshortli
reviewed
Sep 27, 2024
drodriguez
force-pushed
the
test-swift-features
branch
from
14e2017
to
e6de4f2
Compare
|
@swift-ci please test |
Take the `Features.def` file used in other parts of the code and create a file that can be used from the LLVM Lit configuration files to add new available features that can be checked from the tests with `REQUIRES` and others. The file `lit.swift-features.cfg.inc` is preprocessed by Clang and generates a file with Python syntax that can be loaded from both `lit.site.cfg.in` files. The preprocessing output is copied into the different test directories in the build directory, and added it is added as a dependency of them, so it will be generate when the test run or when `Features.def` changes. `EXPERIMENTAL_FEATURES` are only enabled if they are available in production or the compiler is being built with assertions, while `UPCOMING_FEATURES` and the rest of the `LANGUAGE_FEATURES` are always available.
Find all the usages of `--enable-experimental-feature` or `--enable-upcoming-feature` in the tests and replace some of the `REQUIRES: asserts` to use `REQUIRES: swift-feature-Foo` instead, which should correctly apply to depending on the asserts/noasserts mode of the toolchain for each feature. Remove some comments that talked about enabling asserts since they don't apply anymore (but I might had miss some). All this was done with an automated script, so some formatting weirdness might happen, but I hope I fixed most of those. There might be some tests that were `REQUIRES: asserts` that might run in `noasserts` toolchains now. This will normally be because their feature went from experimental to upcoming/base and the tests were not updated.
The test will look for other tests using `RUN:` lines that use experimental or upcoming features and will check that the tests also are checking with the right `REQUIRES:` lines for the used features. This should avoid new tests being introduced without the right `REQUIRES` and should avoid breaking the toolchain in less tested configurations.
drodriguez
force-pushed
the
test-swift-features
branch
from
e6de4f2
to
8ca4461
Compare
|
@swift-ci please test |
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.
Features in Swift can be experimental, upcoming or base features. During their lifetime features evolve from one category to other categories. Usually experimental features are only available in "asserts" compilers, but they can also be experimental features in "non asserts" compilers. Upcoming and base features are available in both "asserts" and "non asserts" compilers.
When coding a new feature, the feature normally will start as an experimental feature. In the past this has required to mark the tests that use that feature with
REQUIRES: assertsto avoid the test failing when testing in a "non asserts" compilers. This requisite is not always follow, and we forget to add thoseREQUIRES: assertsfrom time to time. This causes breakages for people that test the "non asserts" compilers. For some experimental features that are available in production compilers theREQUIRES: assertsis not even needed, which can make adding those lines work against one intentions. When the feature graduates from experimental to upcoming or base, we sometimes forget to update the the related tests to remove thoseREQUIRES: asserts, so a "non asserts" compiler will not actually execute those tests, and bugs can be introduced by mistake in those compilers.The changes in this PR introduce a system that aims to simplify testing those experimental features and avoid some of the problems noted above.
The first change is take the canonical
Features.defand transform its contents in something that LLVM Lit can createavailable_featuresfor. This is done abusing the Clang preprocessor to transform the.deffile into a Python file that can be loaded bylit.site.cfgduring testing. This is done for each build and will pick up changes in theFeatures.defas they happen, so it will always be up-to-date. Additionally it understand when features as available depending on "asserts" or "non asserts" compilers, and will not incorrectly require an "asserts" compiler for non-production features, or let experimental features be tested in a "non asserts" compiler.The second part of the change is keeping the tests up-to-date with the features they are testing, so each test that uses a feature is marked as such. This is done with a test itself, which greps through the existing tests, checks for the usage of
-enable-experimental-featureor-enable-upcoming-featureand warns the user about the missingREQUIRES:lines (failing the test suite, so nobody can submit a test that skips the requirements).Finally, the last change is modifying a huge number of tests to follow the new rules. All the tests that currently use
-enable-experimental-featureor-enable-upcoming-featurehave been annotated withREQUIRES:lines, and the (now unnecessary)REQUIRES: assertshave been removed.