Improvements for Swift 5.1, Swift 5.3, Swift 5.4, watchOS 7.4, Xcode 12.5, Xcode 13 beta

[chris-araman]

Fixes:

/Users/runner/work/CombineCloudKit/CombineCloudKit/.build/checkouts/CombineExpectations/Sources/CombineExpectations/Recorder.swift:211:17: error: cannot find 'XCTFail' in scope
529
                  XCTFail("Publisher recorder is already subscribed")
530
                  ^~~~~~~

https://developer.apple.com/documentation/xcode-release-notes/xcode-13-beta-release-notes

Xcode no longer passes testing search paths to all targets.

With this change, Swift package library targets (but not test targets) which import XCTest or StoreKitTest must now explicitly reference those frameworks in the package manifest using linker settings:
linkerSettings: [.linkedFramework("XCTest")]

This update doesn’t apply to test targets, which can still import test frameworks by default without any explicit mention in linker settings. (75061901)

[chris-araman]

Fleshing out my test matrix, I ran into a few more issues. Fixing them here.

[chris-araman]

@groue, I think this is ready to go. Did you want to take a look at this before merging? I didn't want to merge without at least a gut check or smoke test from another contributor. 😅

[groue]

Hello @chris-araman,

Why do we need to support Swift 5.1 in the first place (Xcode 11 -> 11.3.1)?

[chris-araman]

Fair question, @groue. 😃

Combine was introduced with the SDKs that were introduced with Swift 5.1 and Xcode 11. I'm building a package that builds on Combine, and therefore has a minimum requirement of Swift 5.1. If a third party is already building on Swift 5.1, and picks up my library, I'd like them to be able to run my unit tests to verify the behavior of my library when built with Swift 5.1.

[groue]

OK, @chris-araman, I get it. Thank you very much!

One last nitpick, and we're good to go:

• we may need to add 5.1 in s.swift_versions in the podspec
• we need to update the requirements in the README

[groue]

FYI, I'm not sure CombineExpectation tests pass with the buggy early versions of Combine. If I remember well, those failures are not a bug in this lib, but a bug in Combine itself. I don't know if this is still accurate. But this has been a reason for the bumped requirements of the lib.

EDIT: I can find trace of this up to v0.5:

🐛 CombineExpectations tests crash or fail until Xcode 11.3. They rely on fixes to Combine that were only introduced in the SDK shipped with Xcode 11.3. However, the library itself works fine right from Xcode 11.0.

However this sentence was removed in 0.6, while keeping the Swift 5.1+ / Xcode 11.0+ requirements. I don't recall if I could workaround the problem or not.

And... you bumped requirements to Swift 5.2+ in #16.

Anyway. I guess we are converging, now 😅

[chris-araman]

Haha, okay. 😊

It looks like the 5.2 requirement came in for SPM here, and it was in the original Podspec. Then, I updated the README to match in #16.

Would you be open to me adding a CI matrix for test validation to this repository using GitHub Actions? That way, we could verify with Swift 5.1 and either fix any issues, or explicitly add support. Moving forward, we could catch breaks as they happen. Something similar to this:
https://github.com/chris-araman/CombineCloudKit/actions/workflows/ci.yml

I'll go ahead and update the README and Podspec to match my change to Package.swift.

[chris-araman]

Looks like the ci.yml won't take effect until it is merged to master. If you're open to it, I could cherry-pick it to master in order to verify the 5.1 changes in this PR before merging the rest.

[chris-araman]

@groue, I went ahead and enabled a simple CI workflow. It does look like there are some intermittent test failures, likely due to timing/virtualization. I'm confident we can resolve them.

• RecorderTests.testAvailableElementsAsync
• RecorderTests.testNextNext
• RecorderTests.testNext2Next2
• RecorderTests.testWaitForNextInverted

It doesn't look like there are any differences in test behavior between Swift versions, including 5.1.

I'll wait to enable watchOS in the CI matrix until I know you have access to the macos-11 runners, otherwise, it'll only generate noise. Please accept my apologies for the noise generated so far. 😊

[groue]

I really appreciate your efforts with Github actions here 💯 🎉 🤩 🙌

Tell me when you want me to do something!

[chris-araman]

Glad to hear it, @groue. 😊

I think I just need to figure out how to ensure the few intermittent test failures are stabilized. It's going to be difficult to make strict assumptions about the intricate timings of 0.01s with Timer.publish or Empty<>.delay in the GitHub Actions (or any virtualized) environment. Can you think of another way to validate the test cases I've listed above?

If you're able to fill out the form linked here, you might get early access to the macos-11 runners in the coming days. We could then enable Swift 5.4 and 5.5 beta in the CI matrix.

[chris-araman]

I've adjusted the timeouts of the tests that had been failing intermittently. They seem to be passing consistently now. GitHub Actions is a constrained environment, so waiting 0.2s for a 0.1s delay may not always be sufficient. 😊

If you're able to fill out the form linked here, you might get early access to the macos-11 runners in the coming days. We could then enable Swift 5.4 and 5.5 beta in the CI matrix.

5.4 and 5.5 are now in the matrix, but they won't actually run until you fill out the form and get access to the macos-11 runners. Once that happens, we can remove the continue-on-error directive.

That said, I think this PR is ready to land and release.

[groue]

If you're able to fill out the form linked here, you might get early access to the macos-11 runners in the coming days. We could then enable Swift 5.4 and 5.5 beta in the CI matrix.

Done

That said, I think this PR is ready to land and release.

All right! I'll be back soon :-)

[chris-araman]

Looks like you have access to the macos-11 runners now, so I've removed continue-on-error.

[chris-araman]

👋🏼 @groue, might you have some time this week to review this and groue/CombineTraits#5? I'm hoping to pull these into the next release of my CombineCloudKit.

[groue]

Hello @chris-araman, v0.10.0 has shipped, thank you very much!