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 · 3 comments
Open

teardown_sentry_test clears global event processors #2466

gi opened this issue Nov 19, 2024 · 3 comments
Assignees

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.

@getsantry
Copy link

getsantry bot commented Jan 1, 2025

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 getsantry bot added the Stale label Jan 1, 2025
@getsantry getsantry bot closed this as not planned Won't fix, can't repro, duplicate, stale Jan 9, 2025
@st0012 st0012 reopened this Jan 9, 2025
@sl0thentr0py sl0thentr0py removed the Stale label Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

3 participants