[PM-14033] Configure test workflow simulator type and iOS version with config files #1080
Conversation
|
No New Or Fixed Issues Found |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1080 +/- ##
=======================================
Coverage 89.49% 89.49%
=======================================
Files 675 675
Lines 42560 42560
=======================================
+ Hits 38088 38090 +2
+ Misses 4472 4470 -2 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
🤔 Looks good! About the configuration being in different files I was thinking that most likely these files will be updated alongside when the Xcode version changes so IMO we could have everything on one file and use the source / . command to load if the file looks something like this:
XCODE_VERSION=15.4
SIMULATOR_NAME=iPhone 15 Pro
SIMULATOR_VERSION=18.0
So we just do . /path/to/theconfigfile and the variables will be automatically available and ready for use in the step to be added to $GITHUB_ENV without needing to do any cat or sed commands.
Thoughts?
.github/workflows/test.yml
Outdated
| - name: Read default Xcode version | ||
| run: | | ||
| echo "DEFAULT_XCODE_VERSION=$(cat .xcode-version | tr -d '\n')" >> "$GITHUB_ENV" | ||
| echo "DEFAULT_SIMULATOR_NAME=$(cat .test-simulator-device-name | tr -d '\n')" >> "$GITHUB_ENV" | ||
| echo "DEFAULT_SIMULATOR_VERSION=$(cat .test-simulator-ios-version | tr -d '\n')" >> "$GITHUB_ENV" |
There was a problem hiding this comment.
🤔 Maybe the name of the step should be u 8000 pdated given that it does more than just the Xcode version. What do you think?
There was a problem hiding this comment.
🤦🏼♀️ It is in the Xcode 16 branch, I just didn't bring it over here
I considered that option, too, and went down a long rabbit hole of how to import environment variables from a file into GitHub actions. The thing is, the Ultimately it's probably six of one, half dozen of another. If I had a good reason for abandoning the |
There was a problem hiding this comment.
Agree makes total sense, thanks for explaining @KatherineInCode ! 🎉
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-14033
📔 Objective
Similar to how we now have an
.xcode-versionfile to denote which version of Xcode should be used when building/testing, this creates.test-simulator-device-nameand.test-simulator-ios-versionfiles to denote which device tests should be run against and which iOS version on that device, respectively.I went back and forth on whether to make this two files like this, or one file with the full
name=[blah],OS=[blah]string; I went with the former because it's more straightforward, and easier to potentially use in other workflows/tools in the future.It would be nice if there were a way to use this in BitwardenTestCase as well, but I don't think that's readily possible without pre-build cleverness unfortunately.
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes