Add StagehandTesting to SPM package, update CI#76
Conversation
luispadron
force-pushed
the
luis/add-stagehand-testing-to-spm
branch
3 times, most recently
from
baab24e to
2ac4019
Compare
dfed
left a comment
dfed
left a comment
There was a problem hiding this comment.
Per my comment on #75, it'd be good to re-add unit tests to CI. Could maybe be part of this PR?
Ah my bad, yes I can look at getting tests added via the SPM workflow / pod lib lint. |
luispadron
force-pushed
the
luis/add-stagehand-testing-to-spm
branch
from
2ac4019 to
8b79a7f
Compare
|
@dfed see #77 as to why re-adding example CI is out-of-scope for this PR. I hope ill have time to get the examples updated but right now GitHub actions do not support the iOS versions / Xcode versions the example tests require. I'm focused on adding better support for SPM as that's what were moving to internally. |
dfed
left a comment
dfed
left a comment
There was a problem hiding this comment.
Change here generally makes sense to me, but getting CI's pod linting in sync with the Cocoapods version in Gemfile.lock is blocking imo.
| - name: Lint StagehandTesting Podspec | ||
| run: bundle exec --gemfile=Example/Gemfile pod lib lint --verbose --fail-fast --include-podspecs=Stagehand.podspec StagehandTesting.podspec | ||
| - name: Lint podspecs | ||
| run: pod lib lint --verbose --fail-fast --include-podspecs=Stagehand.podspec StagehandTesting.podspec |
There was a problem hiding this comment.
why do we not want bundle exec here? We should always ensure we're using the same cocoapods version to lint as the one we're going to publish with.
There was a problem hiding this comment.
I had removed that because the root of the repo doesnt have a Gemfile and it was referring to Example here which i was going to address build & testing in #77. I added back the bundle exec until we add a root Gemfile
| swift build \ | ||
| --sdk "$(xcode-select -p)/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS.sdk" \ | ||
| --triple "arm64-apple-ios13.0" | ||
| # select minimum supported Xcode version |
There was a problem hiding this comment.
future-looking comment:
we have some prior art for dropping Xcode version support in minor version bumps. We also do not state the minimum required Xcode version in this project's README 😅 so there's no real contract to break. (maybe we should fix that last part, but... that also doesn't need to be today)
There was a problem hiding this comment.
The comment is referring to the minimum Xcode version on macos-latest so we can target the earliest iOS version on those workers, but Stagehand still supports iOS 12 and that hasn't changed. Updating the Xcode version used in CI for builds/tests shouldn't effect semver afaik
| @@ -1,4 +1,4 @@ | |||
| // swift-tools-version:5.0.1 | |||
| // swift-tools-version:5.8 | |||
| .library( | ||
| name: "StagehandTesting", | ||
| targets: ["StagehandTesting"] | ||
| ), |
There was a problem hiding this comment.
love that this is getting exposed via SPM now!
luispadron
force-pushed
the
luis/add-stagehand-testing-to-spm
branch
6 times, most recently
from
5345169 to
0cf0c9d
Compare
Co-authored-by: Dan Federman <dfed@me.com>
luispadron
force-pushed
the
luis/add-stagehand-testing-to-spm
branch
from
0cf0c9d to
407c5cc
Compare
4 participants
Exposes StagehandTesting via the
Package.swift. Cleans up some more Bazel references and updates CI to test SPM viaxcodebuildwhich allows building the package targets but with the correct platforms. Usingswift buildit was trying to link the macOS version of XCTest even though i was specifying the iOS sdk 🤷🏼