-
-
Notifications
You must be signed in to change notification settings - Fork 257
Fix: Improve Sentry Integration tests #421
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
Conversation
Codecov Report
@@ Coverage Diff @@
## main #421 +/- ##
==========================================
- Coverage 90.02% 89.63% -0.40%
==========================================
Files 54 49 -5
Lines 1684 1514 -170
==========================================
- Hits 1516 1357 -159
+ Misses 168 157 -11 Continue to review full report at Codecov.
|
channel = MethodChannel('sentry_flutter'); | ||
options = SentryFlutterOptions()..dsn = fakeDsn; | ||
hub = Hub(options); |
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.
every other fixture initializes the fields directly, why is it within a ctor and making them late
, is there any benefit in Dart?
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.
I can't initialize Hub
in a field because it needs SentryOptions
. I think all other properties could be initialized in a field.
I would like to land #413 before this, so I can implement #390 (comment) in this 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.
left a suggestion, other than that, LGTM
📜 Description
This PR improves the tests which make sure that every platform uses the correct integrations.
#skip-changelog
💡 Motivation and Context
#390
💚 How did you test it?
I've written new tests.
📝 Checklist
🔮 Next steps