-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
feat(sdk-crash-detection): Call code in post processing #49170
feat(sdk-crash-detection): Call code in post processing #49170
Conversation
Hey, @getsentry/ingest, I would love to get a review on this PR, please 😃 . |
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 wasn't able to get parameterized tests working with test mixin approach. Does anybody know if that's possible?
pytest.mark.parametrize
does not work in combination with unittest.TestCase
, unfortunately: pytest-dev/pytest#541
Why are you moving from standalone (pytest) functions to class-based tests, if I may ask?
Co-authored-by: Joris Bayer <joris.bayer@sentry.io>
So I can apply the same approach for creating an event in |
Yeah so AFAIK you have to chose between using either |
of note, |
Thanks for pointing that out, @asottile-sentry. I created an issue to change the tests to function tests #50061. |
If you are not @getsentry/ingest you can skip this PR.
Merging master into feature branches accidentally requested a review from almost all teams at Sentry. I'm sorry about this.
PR Details
Call SDK crash detection code in post-processing. I had to adapt the code a bit, cause I tested it with a dictionary instead of a stored event. I adapted the tests to use the same approach as the post-processing tests. The logic of the tests didn't change. I refactored all of them to work with stored events.
The next step is to store the SDK crash event to a specific sentry project with #46177
I wasn't able to get parameterized tests working with test mixin approach. Does anybody know if that's possible?
Fixes GH-46175