-
-
Notifications
You must be signed in to change notification settings - Fork 377
chore: Add try? linter for tests and fix current cases
#7361
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. Bug Fixes 🐛
Internal Changes 🔧Deps
Other
Other
🤖 This preview updates automatically when you update the PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
Tests/SentryTests/Integrations/Performance/IO/SentryFileIOTrackingIntegrationTests.swift
Show resolved
Hide resolved
| return actualItem.data == expectedItem.data | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Envelope comparison throws instead of continuing search on parse failure
Medium Severity
The contains closure now uses try JSONSerialization.jsonObject instead of try?. When iterating over actual envelope items to find a match, if any item contains non-JSON data (like binary attachments), the parsing throws and the entire contains call fails. Previously, parse failures would return false and continue checking other items. This breaks the search pattern—even if a matching JSON item exists in the envelope, encountering a non-parseable item first causes the test to fail.
Tests/SentryTests/Integrations/Performance/CoreData/TestCoreDataStack.swift
Outdated
Show resolved
Hide resolved
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7361 +/- ##
=============================================
+ Coverage 85.243% 85.267% +0.023%
=============================================
Files 477 478 +1
Lines 28577 28589 +12
Branches 12418 12430 +12
=============================================
+ Hits 24360 24377 +17
+ Misses 4173 4168 -5
Partials 44 44 see 4 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
… CoreData and FileIO tests


📜 Description
This PR adds a custom SwiftLint rule to prevent the use of
try?(optional try) in test files and fixes all existing occurrences in the codebase.If we need to use
try?in some cases, we can use// swiftlint:disable:next no_try_optional_in_tests💡 Motivation and Context
Using
try?in tests is problematic because it silently converts errors intonil, which can:By enforcing explicit error handling with
try, tests will fail immediately and clearly when errors occur, improving test reliability and debuggability.💚 How did you test it?
📝 Checklist
You have to check all boxes before merging:
sendDefaultPIIis enabled.Closes #7362