sentry-ruby icon indicating copy to clipboard operation
sentry-ruby copied to clipboard

`teardown_sentry_test` clears global event processors

Open gi opened this issue 1 year ago • 3 comments

Issue Description

The test teardown method teardown_sentry_test clears the global event processors Sentry::Scope.global_event_processors. Any application global event processors are wiped after one test.

Reasoning:

  1. The global event processors are by nature global and any setup by the application are cleared. This makes the test setup and teardown process uneven: what is setup before the test is not retained after the teardown of the test.
  2. This method is suggested to be run after every example/test, so this teardown behavior would necessitate running the application setup before every test in order to setup any global event processors.

This behavior was introduced in #2342.

Reproduction Steps

require "sentry/test_helper"

RSpec.configure do |config|
  config.include Sentry::TestHelper
end

RSpec.describe "#teardown_sentry_test" do
  before do
    Sentry.add_global_event_processor { |event| event }
  end

  it do
    expect { teardown_sentry_test }.not_to change(Sentry::Scope, :global_event_processors)
  end
end

Expected Behavior

The list of global events processors Sentry::Scope.global_event_processors is retained.

Actual Behavior

The list of global events processors Sentry::Scope.global_event_processors is cleared.

Ruby Version

3.3.5

SDK Version

5.21.0

Integration and Its Version

Rails 7.1.4

Sentry Config

No response

gi avatar Nov 19 '24 08:11 gi

will defer this to @st0012, I don't have a strong opinion either way.

sl0thentr0py avatar Nov 19 '24 12:11 sl0thentr0py

Re your original comment in #2342:

Also, this PR does not fix the issue that https://github.com/getsentry/sentry-ruby/issues/2051 describes: Sentry.init does not clear the global event processors.

That's never the goal. We don't expect users to call Sentry.init multiple times for testing because it'd mutate the process's states multiple times and cause unwanted side-effects (e.g. registering multiple at_exit callbacks). Therefore, Sentry.init is never designed to clear previous configurations/states.

This is also why we provide the test helper to reset/clean states users may be testing between test runs, so they don't force-reset the values through Sentry.init.

I think the correct fix to this issue is to "restore" instead of "clear" global processors after each run. And I think we may use setup_sentry_test to capture the original value.

st0012 avatar Nov 25 '24 13:11 st0012

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you remove the label Waiting for: Community, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

getsantry[bot] avatar Jan 01 '25 08:01 getsantry[bot]