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 CI #2014

Merged
merged 4 commits into from
Mar 8, 2023
Merged

Fix CI #2014

merged 4 commits into from
Mar 8, 2023

Conversation

st0012
Copy link
Collaborator

@st0012 st0012 commented Mar 4, 2023

There are several issues causing the CI to fail:

Fix sentry-rails' test failures caused by Zeitwerk exception

  1. By default, Rails eager loads the app through Zeitwerk and registers
    future eager load calls with the after_class_unload hook. This hook
    makes sure the app would be eagerly reloaded after the app is unloaded.
    (code)

  2. Since version 2.6.5, Zeitwerk raises an error when the app is being
    eager loaded without a proper setup.

  3. Our test app is not properly setup for being reloaded. So when the app
    is unloaded, it's not ready for the next eager load call, which means
    it'd trigger the error mentioned in 2.

So this PR simply marks the app as not reloadable, which means it doesn't register the callback mentioned in 1 and avoids the later errors.

Fix sentry-resque's test failure caused by mono_logger

It's caused by mono_logger (a resque dependency)'s conflict with Ruby 3.3's Logger. I've opened an issue and worked around it by changing Resque's logger in our tests.

Fix sentry-ruby's test failure caused by error message change

#skip-changelog

st0012 added 2 commits March 4, 2023 11:00
1. By default, Rails eager loads the app through Zeitwerk and registers
    future eager load calls with the `after_class_unload` hook. This hook
    makes sure the app would be eagerly reloaded after the app is unloaded.

    https://github.com/rails/rails/blob/64962266e38817e025de53234accc1e54ed2b32e/railties/lib/rails/application/finisher.rb#L77-L81

2. Since version 2.6.5, Zeitwerk raises an error when the app is being
    eager loaded without a proper setup.
3. Our test app is not properly setup for being reloaded. So when the app
    is unloaded, it's not ready for the next eager load call, which means
    it'd trigger the error mentioned in 2.

So this commit simply marks the app as not reloadable, which means it
doesn't register the callback mentioned in 1 and avoids the later errors.
@codecov
Copy link

codecov bot commented Mar 4, 2023

Codecov Report

Patch coverage: 66.66% and project coverage change: +1.83 🎉

Comparison is base (803ba73) 96.73% compared to head (5ead502) 98.57%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2014      +/-   ##
==========================================
+ Coverage   96.73%   98.57%   +1.83%     
==========================================
  Files         129      157      +28     
  Lines        8578    10075    +1497     
==========================================
+ Hits         8298     9931    +1633     
+ Misses        280      144     -136     
Impacted Files Coverage Δ
sentry-ruby/spec/sentry/client_spec.rb 96.33% <50.00%> (-0.37%) ⬇️
...try-rails/spec/dummy/test_rails_app/configs/7-1.rb 100.00% <100.00%> (ø)
...ry/rails/tracing/active_storage_subscriber_spec.rb 94.87% <0.00%> (ø)
sentry-rails/app/jobs/sentry/send_event_job.rb 60.00% <0.00%> (ø)
...ry/rails/breadcrumbs/active_support_logger_spec.rb 100.00% <0.00%> (ø)
...ntry-rails/spec/sentry/rails/configuration_spec.rb 100.00% <0.00%> (ø)
sentry-rails/lib/sentry/rails/error_subscriber.rb 100.00% <0.00%> (ø)
...rails/tracing/action_controller_subscriber_spec.rb 100.00% <0.00%> (ø)
sentry-rails/lib/sentry/rails/action_cable.rb 100.00% <0.00%> (ø)
sentry-rails/spec/dummy/test_rails_app/app.rb 92.64% <0.00%> (ø)
... and 34 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@st0012 st0012 changed the title Fix sentry-rails' test failures caused by Zeitwerk exception Fix CI Mar 4, 2023
@st0012
Copy link
Collaborator Author

st0012 commented Mar 6, 2023

@sl0thentr0py There still are some test failures to address. I'll try to fix everything this week.

@st0012 st0012 requested a review from sl0thentr0py March 8, 2023 09:24
@st0012 st0012 merged commit a471812 into master Mar 8, 2023
@st0012 st0012 deleted the fix-sentry-rails-test-app branch March 8, 2023 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants