ci: Add code integration testing to repo #1569
Conversation
There was a problem hiding this comment.
Thank you for submitting this PR @cbaker6!
This could definitely be helpful moving forward. Just a few comments.
.github/workflows/ci.yml
Outdated
| @@ -0,0 +1,27 @@ | |||
| name: CI | |||
There was a problem hiding this comment.
It would probably be helpful to make this name a bit more descriptive if possible.
There was a problem hiding this comment.
Do you have something in mind? I think the default action name GitHub provides for this is swift and it usually is the same name of the file, swift.yml.
If there was a special build.yml and release.yml then we could name them build and release. Do you think build works better here?
There was a problem hiding this comment.
Because of the current name, this will show as ci / test (platform=iOS\ Simulator,OS=17.2,name=iPhone\ 15\ Pro\ Max, ResearchKit) (pull_request) and ci / test (platform=iOS\ Simulator,OS=17.4,name=iPhone\ 15\ Pro\ Max, ORKCatalog) (pull_request) in the build view and settings which is more specific. For example:
There was a problem hiding this comment.
I think having it say something along the lines of Build / RK Unit Tests or something close would be fine.
If we could avoid displaying the destination information here (platform, simulator, etc) that would also be great.
There was a problem hiding this comment.
These now show up as Build / ResearchKit Unit Tests:
.github/workflows/ci.yml
Outdated
|
|
||
| on: | ||
| push: | ||
| branches: [ 'main' ] |
There was a problem hiding this comment.
Could we include stable as well for push and pull requests?
.github/workflows/ci.yml
Outdated
|
|
||
| jobs: | ||
| test: | ||
| runs-on: macos-13 |
There was a problem hiding this comment.
Could we update this to macos-latest?
There was a problem hiding this comment.
Github action still defaults macos-latest to version 12.x.x which is missing the latest Xcode https://github.com/actions/runner-images/blob/macOS-12/20240329.1/images/macos/macos-12-Readme.md#xcode
I've switched this to macos-14 instead. Let me know if this works
There was a problem hiding this comment.
It's saying it defaults to 14 here. But specifying macOS-14 is fine! Thanks for updating.
There was a problem hiding this comment.
I think they haven't updated the latest images to point to macOS 14, when I tried latest, it should it's macOS 12 https://github.com/cbaker6/ResearchKit/actions/runs/8620787466/job/23628313753#step:1:8
.github/workflows/ci.yml
Outdated
| strategy: | ||
| matrix: | ||
| destination: ['platform=iOS\ Simulator,OS=17.4,name=iPhone\ 15\ Pro'] | ||
| scheme: ['ResearchKit', 'ORKCatalog'] |
There was a problem hiding this comment.
Lets keep this specific for testing ResearchKit only for now.
There was a problem hiding this comment.
Do you want me to move all of the ORKCatalog test updates out of this PR? I've tested the changes locally and they all pass now
There was a problem hiding this comment.
Yes, keeping the catalog app as is for now would be best.
There was a problem hiding this comment.
Reverted, moved to #1570
.github/workflows/ci.yml
Outdated
| @@ -0,0 +1,27 @@ | |||
| name: CI | |||
There was a problem hiding this comment.
I think having it say something along the lines of Build / RK Unit Tests or something close would be fine.
If we could avoid displaying the destination information here (platform, simulator, etc) that would also be great.
|
Thanks @cbaker6! Really appreciate you taking the time to make the updates. |
I believe you have to approve the PR for it to run the CI for the first time.
You can see the CI results on my fork: cbaker6#3