DependencyValues.swift introduces hard dependency on XCTest.dll in Release builds.

[jeffdav]

Description

When building Release builds of an app that depends on swift-dependencies on Windows, we now also have a dependency on XCTest.dll. We do not want to ship XCTest.dll. :)

I believe that DependencyValues.swift, line 402 introduces this issue.

This may be related to #79.

Checklist

• I have determined whether this bug is also reproducible in a vanilla SwiftUI project.
• If possible, I've reproduced the issue using the main branch of this package.
• This issue hasn't been addressed in an existing GitHub issue or discussion.

Expected behavior

No dependency on XCTest.dll in non-test, non-debug code.

Actual behavior

There is a dependency in the release build:

PS C:\src\app\build\bin> dumpbin /IMPORTS:XCTest.dll .\app.exe
Microsoft (R) COFF/PE Dumper Version 14.36.32537.0
Copyright (C) Microsoft Corporation.  All rights reserved.


Dump of file .\app.exe

File Type: EXECUTABLE IMAGE

  Section contains the following imports:

    XCTest.dll
             140F121B8 Import Address Table
             140F0A4B0 Import Name Table
                     0 time date stamp
                     0 Index of first forwarder reference

                          72 $s6XCTest0A11ObservationMp
                          73 $s6XCTest0A11ObservationPAAE17testCaseDidFinishyyAA0aD0CF
                          75 $s6XCTest0A11ObservationPAAE18testSuiteDidFinishyyAA0aD0CF
                          76 $s6XCTest0A11ObservationPAAE18testSuiteWillStartyyAA0aD0CF
                          77 $s6XCTest0A11ObservationPAAE19testBundleDidFinishyy10Foundation0D0CF
                          78 $s6XCTest0A11ObservationPAAE19testBundleWillStartyy10Foundation0D0CF
                          79 $s6XCTest0A11ObservationPAAE8testCase_22didFailWithDescription6inFile6atLineyAA0aD0C_S2SSgSitF
                          7F $s6XCTest0A17ObservationCenterC6sharedACvgZ
                          82 $s6XCTest0A17ObservationCenterCMa

Steps to reproduce

Build a project on Windows.

Dependencies version information

9e92056

Destination operating system

Windows 11

Xcode version information

n\a

Swift Compiler version information

compnerd.org Swift version 5.9-dev (LLVM c8ca153def6029d, Swift 1381d9a7b2555c6)

[stephencelis]

@jeffdav Is it possible to compile with --static-swift-stdlib in your release builds to avoid bringing in XCTest? Ideally, we'd like folks to be able to run their tests in release mode if possible. In fact, we'd like to remove the #if DEBUGs in XCTest Dynamic Overlay in the future, too.

We're definitely open to other options, but SwiftPM's package-level configuration options are pretty much nonexistent right now, so I think we need to rely on other ambient compile-time configuration/flags.

[jeffdav]

Unfortunately we cannot, as --static-stdlib isn't supported on Windows. I'm all for being able to run tests in release mode (e.g. we have performance tests that run in release), but we should be able to avoid compiling the test code in the product. Would refactoring the code that requires XCTest, etc, into a common support module that can be explicitly linked only by tests be possible?

[stephencelis]

@jeffdav Unfortunate that the flag is unavailable 😕

Would refactoring the code that requires XCTest, etc, into a common support module that can be explicitly linked only by tests be possible?

I may not fully realize what you have in mind, but if this requires a user to remember to do an extra link/import in tests or otherwise risk subtly breaking things, I'm afraid that would be a non-starter in library usability. We'd probably be better off in not supporting release tests at all in that case.

We are definitely open to any and all solutions here, but we may need your help in figuring them out, especially since we're a little less versed in Windows.

As for including XCTest.dll in a Windows build, can you help us understand all the concerns? Is it simply a matter of binary size, or are there even more problems associated with that? While not ideal, I wonder if there are custom build phases you could introduce that could remove the XCTest.dll and strip any symbols from the final product? Or does this break the build in some other way?

[jeffdav]

I'm not an expert, so I'm not 100% sure of a solution either, but I was imaging something similar to how swift-collections handles their _CollectionsTestSupport code. It seems to be a fairly common pattern and it's just an extra entry in the .target dependencies list.

Binary size is a concern, and static linking--even if it were available--would be to each binary we produce in the package. I'm not sure about a post-build step. And it just seems strange for XCTest.dll to show up in a released product's installer.

[mbrandonw]

Hi @jeffdav, swift-collection's _CollectionsTestSupport seems to be a bit of a different beast. It looks like it's just an internal module used to aid in testing the library, and it isn't exposed to the outside at all.

If I'm understanding your suggestion correctly, we would make a DependenciesTestSupport library that does the import XCTest you linked to before. First of all I'm not sure that's even possible because the point of that code is to set up test observers the first time dependencies are accessed. I don't see how that could be moved to another module.

But, even if it was possible, it would mean people need to know that they have to import DependenciesTestSupport in order to write tests for code that touches dependencies. But in reality, people are not going to know they have to do that, and they will get a lot of mysterious test failures.

Really it just seems that we are grappling with Swift's weird testing infrastructure. It's being held back by compatibility with Xcode and super old Objective-C APIs, and it's the whole reason XCTestDynamicOverlay exists in the first place. But we don't think a test support library is the right fix (if even possible). It seems more correct to directly deal with Swift's weirdness by either figuring out how to excise XCTest.dll from the final product, or just accepting it as an artifact of the product.

We definitely agree that this isn't ideal, and we are definitely open to any and all explorations to make it better.

[compnerd]

@mbrandonw so, the idea is that this is for testing only, and the usability is critical, so perhaps we can make a different trade off?

We could remove the import XCTest and make everything go through an indirection which is setup by means of looking up symbols dynamically at runtime. If the symbols are not found, we would simply short circuit. The problem is that weak linking is not supported by PE/COFF and thus unavailable on Windows, so this has to be a loader lookup for the symbols (i.e. dlopen + dlsym / LoadLibraryW, GetProcAddress).

[mbrandonw]

Hi @compnerd, dynamically loading the functionality is absolutely a possibility, and we've already done similar things in a number of places, so it's not something we shy away from. We just aren't entirely sure of how to do it in this situation, but we would love some help!

Also, for Windows in particular, we really don't have any experience with dynamically loading symbols. We even had to drop Windows support from our withMainSerialExecutor tool here because we are not sure what the equivalent of dlopen is.

[compnerd]

The equivalent of dlopen is (as stated above), LoadLibraryW. I have had to do some wrapping of interfaces for SourceKit-LSP, which you might find helpful at: https://github.com/apple/sourcekit-lsp/blob/main/Sources/SKSupport/dlopen.swift

[mbrandonw]

Thanks. I think overall we would need help with wielding these tools. Especially with how it relates to dynamically performing these lines:

swift-dependencies/Sources/Dependencies/DependencyValues.swift

Lines 402 to 408 in 720e68a

	import XCTest
	private final class TestObserver: NSObject, XCTestObservation {
	func testCaseWillStart(_ testCase: XCTestCase) {
	DependencyValues._current.cachedValues.cached = [:]
	}
	}

[compnerd]

Hmm, one immediate workaround that comes to mind - exile the XCTestObservation conformance to a shared library, perform a load to attempt to get a retroactive conformance registered at runtime. Something along the lines of:

@_spi(DependenciesTestSupport)
public class TestObserver: NSObject {
  init() {
    _ = "DependenciesTestSupport.dll".withCString(encodedAs: UTF16.self, LoadLibraryW)
  }
}

@_spi(DependenciesTestSupport) import SwiftDependencies
import XCTest

public extension TestObserver: XCTestObservation {
  public func tesCaseWillStart(_ testCase: XCTestCase) {
    DependencyValues._current.cachedValues.cached = [:]
  }
}

[stephencelis]

@compnerd Sorry we lost track of this. I'm not sure I see the full picture, but would you be willing to try and PR such a solution?

[compnerd]

I'm not sure if I have the time to do this right now, but will see if I can get around to do it at some point. In the mean time, this should be something that you should be able to do with the XCTest documentation already.

[stephencelis]

@jeffdav @compnerd Have either of you looked more deeply into this? I'm inclined to convert this to a discussion for now, as it's not a bug with the library per se, but definitely an enhancement that we would take for non-Apple platforms.

[compnerd]

#169 I think might be a start towards a solution.

[stephencelis]

I believe this is fixed now by merging #169. But let me know if we should reopen it.

[jeffdav]

Thanks all!