Skip to content

teardown_sentry_test helper should clear global even processors too#2342

Merged
st0012 merged 4 commits intomasterfrom
fix-#2051
Jul 23, 2024
Merged

teardown_sentry_test helper should clear global even processors too#2342
st0012 merged 4 commits intomasterfrom
fix-#2051

Conversation

@st0012
Copy link
Copy Markdown
Contributor

@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.)

@codecov
Copy link
Copy Markdown

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
@st0012 st0012 deleted the fix-#2051 branch July 23, 2024 12:13
@gi
Copy link
Copy Markdown

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