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

State Leakage with sidekiq_job Component Causes Test Failures in RSpec #142

Open
kylesnowschwartz opened this issue Nov 18, 2024 · 0 comments

Comments

@kylesnowschwartz
Copy link

When using Marginalia with the sidekiq_job component (Marginalia::Comment.components << :sidekiq_job), my test suite encounters state leakage issues. Specifically, RSpec mocks (instance_double or instance_spy) created in one example leak into subsequent examples, causing failures. This problem occurs even when I attempt to disable Marginalia annotations entirely during tests.

The issue appears to be related to how Marginalia interacts with thread-local state or components when constructing SQL comments.

Error:
The following error frequently appears in my test suite:

Failure/Error: DatabaseRewinder.clean unless example.metadata[:skip_db_clean]

#<InstanceDouble(Delayed::Backend::ActiveRecord::Job) (anonymous)> was originally created in one example but has leaked into another example and can no longer be used. rspec-mocks' doubles are designed to only last for one example, and you need to create a new one in each example you wish to use it for.
# ./lib/marginalia/comment.rb:107:in `sidekiq_job'
# ./lib/marginalia/comment.rb:25:in `block in construct_comment'
# ./lib/marginalia/comment.rb:24:in `each'
# ./lib/marginalia/comment.rb:24:in `construct_comment'

Replication Steps:
1. Add Marginalia to the application and enable the sidekiq_job component:

Marginalia::Comment.components << :sidekiq_job

2.	Set up a Sidekiq worker or a test that involves job processing. In tests, use an instance_double to mock the job, such as:
let(:job) do
  instance_double(
    Delayed::Backend::ActiveRecord::Job,
    attributes: { 'id' => 1 }
  )
end
3.	Run an RSpec test that interacts with SQL and Sidekiq, ensuring DatabaseRewinder or a similar cleanup strategy is used in after(:suite) or before(:each) hooks.
4.	Observe the leakage error:
#<InstanceDouble(Delayed::Backend::ActiveRecord::Job)> was originally created in one example but has leaked into another example.

Workarounds Tried:
• Resetting Marginalia::Comment.components in the test environment test.rb:

Marginalia::Comment.components = []

No effect; the error persists.

•	Not adding the `:sidekiq_job` to the list of components:

Tests pass without issues when Marginalia is not present.

Expected Behavior:
Marginalia annotations should not interfere with test isolation or persist any state that causes RSpec mocks to leak between examples.

Actual Behavior:
The use of the sidekiq_job component introduces state leakage that causes test failures. The exact cause appears to be related to Marginalia’s interaction with thread-local or global state during SQL comment construction.

Environment:
• Marginalia version: 1.11.1
• Rails version: 6.1.7.10
• Ruby version: 3.1.6
• Test framework: RSpec (3.13.0)
• Database: MySQL 8.0.36
• Sidekiq version: sidekiq-pro (7.3.2)

Request:
Please investigate how the sidekiq_job component interacts with thread-local or global state. It would be helpful to ensure that Marginalia’s components do not cause interference in test suites or persist state between tests.

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

No branches or pull requests

1 participant