Split the unit tests into a separate project

[PatrickDanino]

I don’t know if there’s a need for all the people using this framework to also need to run unit tests for it.

More specifically, the number of things you need to import in order to run these tests is considerable. In a large scale project, every import we do has to be done carefully and we don’t normally import test frameworks.

There is also a bit of code that tries to report unit test errors while in debug mode, which I would reconsider despite whatever benefits come with doing that.

[mbrandonw]

Hi @PatrickDanino, I'm sorry but I'm not sure I understand the concern here. You are concerned about these tests in the library? Those tests are an internal detail of the library, and shouldn't be a hindrance to anyone using Perception.

Can you explain a bit more about the problem you are running into?

[PatrickDanino]

The Package includes a bunch of unit-test specific frameworks. That ends up being required in order to properly import the framework itself.

I ended up re-writing the Package file to this, where only 2 external frameworks are required (one we already had, the other is from Apple), leaving only this one framework as a 3rd party dependency.

The suggestion is therefore to create another project if you'd like that takes this one as a dependency and adds back the unit-test specific dependencies as needed.

// swift-tools-version: 5.9

import CompilerPluginSupport
import PackageDescription

let package = Package(
  name: "swift-perception",
  platforms: [
    .iOS(.v13),
    .macOS(.v10_15),
    .tvOS(.v13),
    .watchOS(.v6),
  ],
  products: [
    .library(name: "Perception", targets: ["Perception"])
  ],
  dependencies: [
    .package(name: "swift-collections", path: "../SwiftCollections"),
    .package(name: "swift-syntax", path: "../SwiftSyntax"),
  ],
  targets: [
    .target(
      name: "Perception",
      dependencies: [
        "PerceptionMacros",
        .product(name: "OrderedCollections", package: "swift-collections"),
      ]
    ),
    .macro(
      name: "PerceptionMacros",
      dependencies: [
        .product(name: "SwiftSyntaxMacros", package: "swift-syntax"),
        .product(name: "SwiftCompilerPlugin", package: "swift-syntax"),
      ]
    ),
  ]
)

[mbrandonw]

Hi @PatrickDanino, are you speaking of these dependencies:

.package(url: "https://github.com/pointfreeco/swift-macro-testing", from: "0.1.0"),
.package(url: "https://github.com/pointfreeco/xctest-dynamic-overlay", from: "1.0.0"),

?

If so, the first is only used to test this library and is never compiled for people using the library. In fact, you won't even see it in your Xcode project when depending on Perception because Xcode knows it's only used in tests.

And the second is necessary for the library to work. We use it to raise a test failure if a @Perceptible model's field is accessed without being in WithPerceptionTracking, which can help catch bugs in people's code. The library will not compile without that. Also, it is not a unit testing framework, but rather it dynamically loads XCTFail so that it can be used in application code.

I'm still not certain I understand the concern here. None of these details should be visible to you as the user of the library. You should be able to just import Perception and nothing else. Is that not the case?

[PatrickDanino]

That is not the case without importing more stuff.

Importing Perception currently depends on importing XCTestDynamicOverlay, which is a framework that is required in order to build Debug code. That code will not compile as-is unless you import the other pointfreeco frameworks you have listed above, which is what we do not want to have to do.

[mbrandonw]

While Perception depends on XCTestDynamicOverlay, you should never need to import XCTestDynamicOverlay yourself. Can you show some code where you are being forced to import XCTDynamicOverlay?

[PatrickDanino]

This framework does get used in a Debug build, which we of course need to do for our own testing. Here's one example in RuntimeWarning:

#if DEBUG
  // import XCTestDynamicOverlay

  #if canImport(os)
    import os

I also had to comment various test-specific functions for our debug builds:

//    if _XCTIsTesting {
//      XCTFail(message)
//    } else {

From our point of view, the technical details of why a framework like this is being used don't matter if the framework is a dependency. As soon as it becomes a dependency, we have to review it, track it and it just becomes additional overhead. This is in part for legal and compliance reasons.

[mbrandonw]

Hi @PatrickDanino, you are definitely more than welcome to maintain a fork of the repo to remove XCTestDynamicOverlay, but it's not reasonable to ask us to remove/split it from the library. We consider it a critical part of the library, and so it will stay.