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

Fix issues with specs again #2514

Merged
merged 2 commits into from
Jan 21, 2025
Merged

Fix issues with specs again #2514

merged 2 commits into from
Jan 21, 2025

Conversation

solnic
Copy link
Collaborator

@solnic solnic commented Jan 21, 2025

This spec couldn't run reliably on multiple older rubies, couldn't run at all on jruby, and it would hang forever every now and then. So I fixed it here.

This also disables eager_load in sidekiq tests (we did the same thing in rails spec suite last year) as it causes VERY strange errors, this time it was send_event_job.rb causing all sorts of weird crashes with modules or methods being undefined, like this one:

NoMethodError:
       undefined method `lookup' for ActiveJob::QueueAdapters:Module

#skip-changelog

Copy link

codecov bot commented Jan 21, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.75%. Comparing base (d9c279b) to head (869ade2).
Report is 1 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #2514       +/-   ##
===========================================
- Coverage   98.16%   69.75%   -28.41%     
===========================================
  Files         128      125        -3     
  Lines        4849     4722      -127     
===========================================
- Hits         4760     3294     -1466     
- Misses         89     1428     +1339     
Components Coverage Δ
sentry-ruby 60.56% <ø> (-37.99%) ⬇️
sentry-rails 96.76% <ø> (-0.31%) ⬇️
sentry-sidekiq 97.09% <ø> (ø)
sentry-resque 92.85% <ø> (ø)
sentry-delayed_job 95.65% <ø> (ø)
sentry-opentelemetry 99.31% <ø> (ø)

see 75 files with indirect coverage changes

@solnic solnic force-pushed the solnic/fix-trap-spec branch from 2149910 to 30bac7e Compare January 21, 2025 11:32
This makes code loading so unstable that it makes no sense.
@solnic solnic force-pushed the solnic/fix-trap-spec branch from 9da1c7a to 869ade2 Compare January 21, 2025 12:34
@solnic solnic marked this pull request as ready for review January 21, 2025 12:46
@solnic solnic changed the title Fix and improve trap spec Fix issues with specs again Jan 21, 2025
@solnic solnic requested review from st0012 and sl0thentr0py January 21, 2025 12:46
Copy link
Member

@sl0thentr0py sl0thentr0py left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

much nicer :)

@solnic solnic merged commit cceaa6a into master Jan 21, 2025
150 of 151 checks passed
@solnic solnic deleted the solnic/fix-trap-spec branch January 21, 2025 14:35
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.

2 participants