Skip to content
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

teardown_sentry_test helper should clear global even processors too #2342

Merged
merged 4 commits into from
Jul 23, 2024

Conversation

st0012
Copy link
Collaborator

@st0012 st0012 commented Jul 14, 2024

Closes #2051

When resetting the test environment with the teardown_sentry_test helper, global event processors should be cleared too.

(I also made sentry-ruby's tests not fail fast as they're quite flaky at times due to dependencies, request issues...etc.)

Copy link

codecov bot commented Jul 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.66%. Comparing base (89f371f) to head (554a439).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2342   +/-   ##
=======================================
  Coverage   98.66%   98.66%           
=======================================
  Files         205      205           
  Lines       13488    13493    +5     
=======================================
+ Hits        13308    13313    +5     
  Misses        180      180           
Components Coverage Δ
sentry-ruby 99.03% <100.00%> (+<0.01%) ⬆️
sentry-rails 97.41% <ø> (ø)
sentry-sidekiq 97.01% <ø> (ø)
sentry-resque 96.79% <ø> (ø)
sentry-delayed_job 98.92% <ø> (ø)
sentry-opentelemetry 100.00% <ø> (ø)
Files Coverage Δ
sentry-ruby/lib/sentry/test_helper.rb 100.00% <100.00%> (ø)
sentry-ruby/spec/sentry/test_helper_spec.rb 100.00% <100.00%> (ø)

@st0012 st0012 requested a review from sl0thentr0py July 23, 2024 09:15
@st0012 st0012 merged commit 18029e3 into master Jul 23, 2024
124 of 125 checks passed
@st0012 st0012 deleted the fix-#2051 branch July 23, 2024 12:13
@gi
Copy link

gi commented Nov 19, 2024

@st0012 I don't think that theteardown_sentry_test method should touch the global event processors.

  1. The setup_sentry_test method does not touch them, so why should we assume that the teardown method should?
  2. Any global event processors setup by the application are unfortunately cleared. This method is suggested to be run after every example/test, so this teardown behavior would necessitate running the application setup before every test.

Also, this PR does not fix the issue that #2051 describes: Sentry.init does not clear the global event processors. This PR is all about the test helper.

I suggest the issue be reopened or closed as "won't fix" and this PR be reverted.

I filed a bug report just in case you agree after some consideration. Feel free to close if you disagree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Global event processors do not clear on Sentry.init
3 participants