Skip to content
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

[Pitch] Restrict testability to direct in-package source module dependencies of test targets #7904

Open
rauhul opened this issue Aug 21, 2024 · 0 comments

Comments

@rauhul
Copy link
Member

rauhul commented Aug 21, 2024

Description

SwiftPM's current behavior around @testable/testability can cause confusion and build performance issues.


When building in debug mode, all modules are built with testability. This means programmers can @testable import other modules from any package, producing code that violates public API contracts. For example, consider the following modules in two distinct packages:

// Package.swift
.target(
  name: "MyModule",
  dependencies: [.product(name: "OtherModule", package: "OtherPackage")])
.excutableTarget(
  name: "MyExecutable",
  dependencies: ["MyModule"]),

// OtherModule
let x = 1

// MyModule
@testable import OtherModule
let y = x + 1 // ok in debug mode!

// MyExecutable
@testable import MyModule
print(y) // ok in debug mode!

This code seems like a pretty clear misuse of the testability feature, but compiles and runs "correctly" in debug mode. However in release mode the developer will hit a compilation error.


The benefit of building with testability by default in debug mode (as SwiftPM currently does), is that modules do not need to be rebuilt when running swift test. For example, the following test target does not require MyModule to be rebuilt when running swift test after swift build:

// Package.swift
.testTarget(
  name: "MyTest",
  dependencies: ["MyModule"]),

// MyTest
@testable import MyModule
#expect(y == 2)

However this "no-rebuild" benefit only applies in debug mode. In release mode, modules built during swift build -c release do not have testability enabled, meaning when swift test -c release is run, all modules must be rebuilt.


If we consider a package vending a macro, we can see how the interaction of rebuilding the entire package graph in release mode with testabillity can really hamper developer productivity.

// Package.swift
.target(
  name: "MyLibrary",
  dependencies: ["MyMacros"]),

.macro(
  name: "MyMacros",
  dependencies: [
    .product(name: "SwiftCompilerPlugin", package: "swift-syntax"),
    .product(name: "SwiftDiagnostics", package: "swift-syntax"),
    .product(name: "SwiftOperators", package: "swift-syntax"),
    .product(name: "SwiftSyntax", package: "swift-syntax"),
    .product(name: "SwiftSyntaxBuilder", package: "swift-syntax"),
    .product(name: "SwiftSyntaxMacroExpansion", package: "swift-syntax"),
    .product(name: "SwiftSyntaxMacros", package: "swift-syntax"),
  ]),

.testTarget(
  name: "MyMacrosTests",
  dependencies: [
    "MyMacros",
    .product(name: "SwiftSyntax", package: "swift-syntax"),
    .product(name: "SwiftSyntaxMacros", package: "swift-syntax"),
    .product(name: "SwiftSyntaxMacrosTestSupport", package: "swift-syntax"),
  ]),

This package produces a target called "MyLibrary" which acts as the entry point the macros vended by the "MyMacros" target. Additionally, the package tests their macros' implementations in a test target called "MyMacrosTests".

  1. The developer first runs swift build -c release.

SwiftPM builds "MyLibrary", "MyMacros", and all dependent modules in release mode as expected. This can take a while because release builds are slow to begin with and swift-syntax is a very large project on top of that.

  1. The developer then runs swift test -c release to confirm their macro works.

SwiftPM builds "MyMacrosTests" for the first time and rebuilds entire dependency tree of "MyMacrosTests" with testability.

  1. The developer runs swift build -c release again to fix a bug uncovered by testing.

All dependent modules are rebuilt once again with testability disabled.

This pattern effectively doubles (or triples) the already long build time of swift-syntax.

What's most important, however, is that the developer isn't even trying to test the implementation of swift-syntax! "MyMacrosTests" only @testable imports "MyMacros" and (IMO) it should only ever be able to @testable import modules from the same package.

This is a problem I personally face frequently when developing swift-mmio. Swift-mmio is pretty tiny library, but CI can take over 20 minutes to complete (per swift version) due to repeated re-builds of swift-syntax (and other dependencies).


I propose we change SwiftPM to restrict testability to only direct in-package source module dependencies of test targets. With this hcange can we can avoid rebuilding most of the tests' dependencies (e.g. swift-syntax in this example). Additionally, this change would resolve the bug of being able to @testable import modules in non-test targets.

This behavior change would of course be gated on swift-tool-version: next to avoid breaking an existing packages.

Expected behavior

Only build "testable" variants of modules defined in the same package and directly imported by test targets. We could suffix these modules with -testable to disambiguate their paths.

In the case of a shared dependency directly and indirectly used by a test target, the testable variant should be used, e.g.:

  graph LR;
      A-Test --> B-Testable;
      A-Test --> C-Testable;
      B-Testable --> C-Testable;
      B-Testable -. disabled .-> C
Loading

Actual behavior

No response

Steps to reproduce

No response

Swift Package Manager version/commit hash

No response

Swift & OS version (output of swift --version && uname -a)

No response

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant