Add Swift Package Manager support #321
Conversation
crazytonyli
changed the title
This additional step generate .swiftpm which hopefully resolves the 'could not generate PIF' issue.
crazytonyli
force-pushed
the
swift-package-folder-structure
branch
from
eca447e to
2fe6d54
Compare
This is mostly to see if the tooling issue seen in #321 disappears on a newer Xcode version.
It should be redundant, but #321 shows that we need to run it before the tests. The previous commit upgrades CI to Xcode 14.1, hopefully that's enough to get the build for this one to work.
.buildkite/pipeline.yml
Outdated
| ################ | ||
| ######################## | ||
| # Validate Swift Package | ||
| ######################## | ||
| - label: "🧪 Build and Test iOS" |
There was a problem hiding this comment.
Did you decide to keep the label the same to avoid the GitHub checks dance we run into WordPressUI-iOS?
There was a problem hiding this comment.
🙈 I forgot to change it actually... Addressed in 7615808
.buildkite/pipeline.yml
Outdated
| ################ | ||
| # Build and Test | ||
| ################ | ||
| ######################## | ||
| # Validate Swift Package | ||
| ######################## |
There was a problem hiding this comment.
FWIW, my two cents are that we can delete these headers.
| xcodebuild( | ||
| scheme: 'WordPressShared', | ||
| xcargs: '-resolvePackageDependencies' | ||
| ) |
There was a problem hiding this comment.
Interesting... I assume the tests run didn't work without this.
Have you tried passing the xcargs option to run_tests?
The action has parameters such as disable_package_automatic_updates and skip_package_dependencies_resolution (both defaulting to false) which makes me think it should be able to handle this without a dedicated step.
Okay. Silly me, I should have done my reading before commenting 😓
I can see the error this addresses from the CI builds, e.g. this one.
[...] Cannot open "Documentation.docc" as a "Swift Package Folder" because it is already open as a "Folder"."
[...]
[06:35:08]: ▸ ❌ error: Build service could not create build operation:
internal error:
could not generate PIF because the workspace has not finished loading or is still waiting for package resolution
Seems like a tooling bug to me. Worth adding a note in the code about it?
There was a problem hiding this comment.
Seems like a tooling bug to me
I noticed CI for this project is on Xcode 14.0. I opened #323 to see if Xcode 14.1 solved the alleged tooling issue.
There was a problem hiding this comment.
I have seen similar error in Xcode 13 before, my guess is it's caused by some race condition within Xcode between creating the Swift Package's Xcode project and loading the project. But yeah, hopefully this is no longer an issue in Xcode 14.1.
Another thing is, if this issue continues to show up, it would be a good idea to move this additional command into bash-cache's validate_swift_package script. Also, it's probably also worth to make the script self contained, rather than relying on the projects to have a test lane.
There was a problem hiding this comment.
Also, it's probably also worth to make the script self contained, rather than relying on the projects to have a test lane.
Great idea. In particular if we need additional setup.
Co-authored-by: Gio Lodi <[email protected]>
Co-authored-by: Gio Lodi <[email protected]>
|
If I understand it correctly, Test Analytics of this project is set to only collect test data of the trunk branch. I'll merge this PR and check out the trunk branch to make sure the Test Analytics set up is not broken. |
Yep, it's broken. 🤦♂️ I've opened an issue for it #324, and I'll look into it later. |
Unlike similar PRs to other libraries, I didn't split this SPM support change into two PRs, because the complication of "WPAnalyticsTests". I can't get this test to compile in Swift Package Manager, due to we have to use Swift version of Quick and OCMock (which is Objective-C).
And, I ended up deleting this test case. 🥲
Can't get it to compile is the main reason of course, but also I don't see this test provides much value. I'm not familiar with OCMock, but just by reading the code, I think the verification logic (invoked from OCMock) seems all about "verifying a method is indeed called when a method is called".
CHANGELOG.mdif necessary.