TestDependencies in Release Code

[natemann]

We currently wrap all our client testValues in #if DEBUG statements.

struct Client {}

#if DEBUG
extension Client {
  static func testValue() -> Self 
}
#endif

This is causing a problem when implementing TestDependencyKeys and building for production. The build fails because it has to build the TestDependencyKey conformance, but cannot find out testValue.

So, we have discussed a couple of options.

1. we could simply remove the #if DEBUG statements and live with the slightly less safe codebase
2. change the testValue to a computed property and move the if DEBUG there

extension Client: TestDependencyKey {
  public static var testValue: Self {
    #if DEBUG
    return testValue()
    #else
    // not sure yet (fatalError / live / noop)
    #endif
  }
}

but this does not feel much safer

Finally, I wrapped previewValue and testValue in #if DEBUG at the protocol declaration.

public protocol TestDependencyKey {
  associatedtype Value = Self

  #if DEBUG
  static var previewValue: Value { get }
  static var testValue: Value { get }
  #endif 
}

TCA still builds and all tests still pass. The only issue I can see is if someone runs tests in something other than DEBUG. Any thoughts? Am I missing something?

I opened a PR here #1560

[stephencelis]

@natemann While we used to wrap our test clients with #if DEBUG we don't really think this is necessary. When SwiftUI first came out it similarly generated its Xcode preview code inside #if DEBUG blocks, but no longer does these days.

Do you have any concerns with removing the #if DEBUG here and allowing release builds to see the test and preview values?

[tgrapperon]

If I recall correctly, there was a bug/issue with the dynamic nature of SwiftUI Previews that prevented them to be pruned in release builds when they released SwiftUI on iOS 13.0, but they fixed it a few releases later.

EDIT: They fixed it for the official release:

The #if/#endif compiler conditionals surrounding PreviewProvider types have been removed from SwiftUI templates. PreviewProviders aren’t properly removed from built products when archived. (51539802)

[stephencelis]

Also, if you really would prefer fatal errors in release builds, could you extend TestDependencyKey with default conformances directly in your package?

extension TestDependencyKey {
  public static var previewValue: Value { fatalError() }
  public static var testValue: Value { fatalError() }
}

It might require a type alias for the release branch depending on how you define your dependencies, but should be doable from outside the library.

[natemann]

@stephencelis My concern is mostly due to having a large code base with multiple devs working in it. Also, for a more 'real world' example, take something like a databaseClient. We have a databaseClient that fetches / saves many different types of objects. Each of of these objects has a mock value that is wrapped in #If DEBUG, and these mocks are referenced in the testValue of the databaseClient. We would have to remove quite a bit of #if DEBUG statements to simply get a testValue building in release.

I figured wrapping previewValue and testValue in DEBUG at the declaration probably has ramifications that you or I could not know yet.

If you do not want to go down that route, we will probably go with the default values.

[natemann]

@tgrapperon yes, it would. is TCA tested in Release mode. I did not think it was common to run tests in Release mode

[tgrapperon]

Yes it is, for about a month.

[mbrandonw]

I think the main reason we do not want to #if DEBUG the test value is that it completely precludes running tests in release mode.

Now technically it is not possible to run tests in release today due to the fact that XCTFail is shimmed in XCTDynamicOverlay and currently we have no choice but to stub it as an empty function in release. But, if someday we figure out how to work around that (and hopefully soon), then we think it would be really great to be able to run tests in release mode, after all that is the true code running on the device when downloaded from the App Store.

Also, if you are following the pattern of "unimplemented" test values, then you won't expose any mocks for that value. It should just be an implementation that does XCTFail under the hood.

An argument could maybe be made for hiding previewValue behind #if DEBUG since it will use mocks.

[mbrandonw]

While TCA is tested in release mode, any test using TestStore is not actually asserting anything because of the XCTFail issue I mentioned above. But there are other assertions in the test suite that are truly run in release mode, and so we are at least getting coverage on that.

[natemann]

@mbrandonw we are using unimplemented, but still often have to provide values for the placeholder. These are often mock values we have implemented.