With upcoming work on interoperability between Swift and C++, we anticipate that we will need to change a lot of existing interop tests (to validate that C features continue to work when we switch the language mode to C++ in ClangImporter, to separate existing interop tests into pure C tests and tests that depend on Objective-C), and we will add many new tests for C++ interop itself. Therefore, we would like to reorganize the existing interop tests to move closer to achieving the goals we list below.
@scentini has created a pull request that sketched out the idea, but during the code review we understood that a more detailed writeup and discussion is probably needed.
What do people think? @Douglas_Gregor @Slava_Pestov @John_McCall @Michael_Gottesman @codafi
Goals for interop tests
We would like Swift tests for C, Objective-C, and C++ interoperability to be a reliable indicator of interop quality while being easy to work with. Specifically:
- It should be easy to understand which C, Objective-C, and C++ features work.
- It should be easy to understand which features are covered by tests.
- It should be difficult to miss adding a certain kind of tests for a newly implemented interop feature. For example, it should be obvious that it is customary to have tests for the type checker, SILGen, IRGen, and execution tests, but a pull request is only adding type checker tests.
- Where we currently have such testing gaps, they should be obvious.
- Tests for a given interop feature should be easy to find.
- All inputs for a given test should be easy to find.
- It should be obvious which input files are shared between tests.
- It should be easy to share input files between different kinds of tests for one interop feature.
- It should be difficult to accidentally share input files across unrelated tests.
- Tests should be easy to change. Developers should be able to change tests confidently. Changing one test should not subtly break unrelated tests.
Current difficulties
We believe the way the interop tests are organized right now makes it difficult to achieve the goals listed above.
- Tests for a given C interop feature (for example, interop with anonymous enums) are located in multiple directories (
test/ClangImporter,test/Sema,test/SILGen,test/IRGen,test/Interpreteretc.)- These directories contain lots of tests for features other than interop, so finding interop tests is non-trivial. Some interop tests are named starting with
objc_, but it is not a hard rule. - Multiple tests for the same interop feature are often named differently across multiple directories.
- Tests for an interop feature that located in different directories don’t reuse input files (for example, C headers).
- It is difficult to understand where the gaps in testing are, and where coverage is inconsistent.
- Interop features are often tested inconsistently. For a given interop feature we often have tests for one component of the compiler, but not for other components. It does not immediately look wrong though, because to notice it you have to look in multiple directories.
- Tests are often organized around how or when the feature was implemented, or how a bug was fixed, not around the aspect being tested.
- These directories contain lots of tests for features other than interop, so finding interop tests is non-trivial. Some interop tests are named starting with
- Tests are located in unintuitive places, for example, tests for IRGen for interop exist under both
test/ClangImporterandtest/IRGen. - Test directories contain too many files in a flat list (
test/ClangImporter-- 190,test/SILGen-- 419,test/IRGen-- 457).- Therefore, finding interop tests (or any other type of tests!) is difficult.
- Large test directories each have exactly one “Inputs” directory.
- Most Swift tests don’t use input files, they are contained in one
.swiftor.silfile. Interop tests are different from most other tests: each interop test uses one or more C header files. Therefore, the issues with input files affect interop tests the most severely. - Loosely-related and unrelated interop tests tend to reuse the input files (C headers). Often they use completely disjoint parts of those headers.
- Due to reuse, headers tend to grow bigger and bigger, as more tests use them and need to add “just one more thing”.
- The “Inputs” directory is accessible to all tests, making it difficult to understand which tests use which input files.
- Changing files in the “Inputs” directory is scary because the scope of their reuse is unclear. It is much easier to just append stuff at the bottom.
- Due to reuse of input files in new and creative ways, their incidental parts over time become load-bearing. This makes input files and tests that depend on them more difficult to change.
- Most Swift tests don’t use input files, they are contained in one
Suggested layout for the test directory
All our suggestions are based on the observation that interop is a cross-cutting feature. It is not the case for many other features and aspects of the Swift compiler. Therefore, we think it makes sense to group tests differently.
For features isolated to a specific component, it makes sense to store tests in a directory related to that component. For example, a test that checks that the SIL optimizer transforms a specific code pattern should be located under test/SILOptimizer, because there is very little chance that there are type checker or IRGen concerns with that code pattern.
However, for interop, we think it makes more sense to group the tests around the C, Objective-C, and C++ features. Almost any interop feature has a non-trivial interaction with every compiler component (type checker, SILGen, IRGen, execution), and therefore should be validated at each stage.
Baseline suggestion: organize interop tests by language and feature
test/
Interop/
ObjC/
class/
Cxx/
class/
template/
enum/
Inputs/
shared/
CoreCooling.h // A curated header intended for reuse.
anonymous-enums.h
enum-class.h
module.map
anonymous-enums-typechecker.swift
anonymous-enums-print-module-interface.swift
anonymous-enums-silgen.swift
anonymous-enums-irgen.sil
anonymous-enums-execution.swift
enum-class-typechecker.swift
enum-class-print-module-interface.swift
enum-class-...
Note that there is no test/Interop/C directory. From the interop point of view, C is a subset of C++, and all C headers should continue to be importable into Swift once we turn on C++ interop. Furthermore, at some point C++ interop will become on by default, and therefore, there will be no separate C interop as such. Objective-C interop is different because it is only available on select targets.
Advantages:
- Tests that validate a given interop feature in different compiler components can reuse input files.
- Reuse of input files is limited to logically related tests, assuming tests don’t reach out into unrelated directories. Tests within a focused directory, like
test/Interop/Cxx/enum, can share input files, and it should not cause much trouble because they are related. - We can furthermore establish an “Inputs/shared” subdirectory for shared inputs and avoid sharing other inputs as a convention.
- It is clear which platforms run which tests. All target platforms will run
test/Interop/Cxxtests. Only platforms with Objective-C interop will runtest/Interop/ObjCtests. We will not need to add “REQUIRES: objc_interop” tags.
Disadvantages:
- Tests are no longer organized only by compiler component. The only reason we could think of why this is valuable today is described in the section below, along with a suggested alternative workflow ( Composable option:
ninja check-swift-<compiler-component>).
Alternative: flatten Interop/ObjC and Interop/Cxx to InteropObjC and InteropCxx
test/
InteropObjC/
class/
InteropCxx/
enum/
class/
template/
...
This alternative creates fewer nested directories, but otherwise achieves the same goals, and has the same advantages/disadvantages as the baseline suggestion.
Alternative: group tests only by language feature, not by language
test/
Interop/
enum/ // ObjC and C++ enums are similar enough
// to go into one directory.
template/ // Templates are obviously C++-only.
cxx-class/ // Disambiguate C++ and ObjC classes.
objc-class/
This alternative has the same advantages/disadvantages as the baseline suggestion, except that we still have to annotate every Objective-C interop test with “REQUIRES: objc_interop”.
Alternative: tests with inputs must be in a separate directory
To further limit unintentional sharing of input files and clarify which input files are used by which test, we could require that tests with inputs must be in a separate directory.
test/
Interop/
Cxx/
SharedInputs/
CoreCooling.h
anonymous-enums/
Inputs/
anonymous-enums.h
module.map
anonymous-enums-typechecker.swift
anonymous-enums-print-module-interface.swift
anonymous-enums-silgen.swift
anonymous-enums-irgen.sil
anonymous-enums-execution.swift
enum-class/
Inputs/
enum-class.h
module.map
enum-class-typechecker.swift
enum-class-print-module-interface.swift
enum-class-...
Composable option: test/ClangImporter instead of test/Interop
We could put the proposed directory structure under test/ClangImporter instead of test/Interop. We are suggesting to not do that because “ClangImporter” is a compiler component, and putting non-ClangImporter tests there (for example, type checker tests, SILGen, IRGen tests) can be confusing.
Composable option: ninja check-swift-<compiler-component>
Currently it is possible to run most tests for a given compiler component by pointing lit at a subdirectory like test/IRGen. It is a useful workflow for developers who iterate on the code in a particular component of the compiler. However, this workflow only runs most tests, not all of them; for example, the test/ClangImporter directory also contains IRGen and SILGen tests; execution tests indirectly test most components of the compiler and the standard library, etc. Therefore, this workflow is only useful as a smoke test while rapidly iterating on the code. One still needs to run the whole testsuite before merging any changes.
This proposal will weaken the lit test/IRGen workflow further by moving out interop tests into a different directory. If it is a concern, we could add ninja targets like check-swift-typechecker, check-swift-silgen, check-swift-irgen etc. that run only the tests that are named or tagged accordingly.
This option composes with any alternative described above.