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 clears global event processors #2466

Open
gi opened this issue Nov 19, 2024 · 2 comments
Open

teardown_sentry_test clears global event processors #2466

gi opened this issue Nov 19, 2024 · 2 comments

Comments

@gi
Copy link

gi commented Nov 19, 2024

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

@sl0thentr0py
Copy link
Member

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

@st0012
Copy link
Collaborator

st0012 commented Nov 25, 2024

Re your original comment in #2342:

Also, this PR does not fix the issue that #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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Waiting for: Community
Development

No branches or pull requests

3 participants