chore: Fix watchOS tests and add them to nightly job#7633
chore: Fix watchOS tests and add them to nightly job#7633
Conversation
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨
Bug Fixes 🐛
Internal Changes 🔧Deps
Samples
Other
🤖 This preview updates automatically when you update the PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## itay/cron_job #7633 +/- ##
===================================================
+ Coverage 85.314% 85.360% +0.045%
===================================================
Files 483 483
Lines 28750 28750
Branches 12492 12501 +9
===================================================
+ Hits 24528 24541 +13
+ Misses 4175 4163 -12
+ Partials 47 46 -1
... and 7 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Sentry Build Distribution
|
Performance metrics 🚀
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Sentry Build Distribution
|
| ./scripts/check-clang-format.py -r Sources Tests | ||
| ruby ./scripts/check-objc-id-usage.rb -r Sources/Sentry | ||
| @if [ -n "$(STAGED_SWIFT_FILES)" ]; then \ | ||
| swiftlint --strict --quiet --config .swiftlint.yml $(STAGED_SWIFT_FILES); \ |
There was a problem hiding this comment.
Yes, forcing ./.swiftlint.yml on every check failed, because it was not picking up the more specific config for each subfolder (see https://github.com/getsentry/sentry-cocoa/blob/main/SentryTestUtils/.swiftlint.yml and https://github.com/getsentry/sentry-cocoa/blob/main/Tests/.swiftlint.yml)
| ruby ./scripts/check-objc-id-usage.rb -r Sources/Sentry | ||
| @if [ -n "$(STAGED_SWIFT_FILES)" ]; then \ | ||
| swiftlint --strict --quiet --config .swiftlint.yml $(STAGED_SWIFT_FILES); \ | ||
| swiftlint --strict --quiet $(STAGED_SWIFT_FILES); \ |
There was a problem hiding this comment.
m: We should prefer explicit configuration over defaults
There was a problem hiding this comment.
Swiftlint already looks for the config in the current folder and subfolders: https://github.com/realm/SwiftLint?tab=readme-ov-file#nested-configurations
We shouldn't need to add it manually
| #elif TARGET_OS_WATCH | ||
| // TODO: create a watch UI test target to test this branch | ||
| SENTRY_ASSERT_PREFIX(osVersion, @"2.", @"3.", @"4.", @"5.", @"6.", @"7.", @"8.", @"9.", @"26."); | ||
| XCTAssertEqualObjects(osVersion, @""); |
There was a problem hiding this comment.
m: I don't fully understand why this is now an assertion to an empty string, instead of actually getting an version string
There was a problem hiding this comment.
watchOS always returned an empty string:
sentry-cocoa/Sources/Sentry/SentryDevice.m
Line 186 in 313b966
It was always broken
📜 Description
Fixes watchOS tests
💡 Motivation and Context
💚 How did you test it?
📝 Checklist
You have to check all boxes before merging:
sendDefaultPIIis enabled.Closes #7634