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

Active Job notifications subscriber #1761

Merged
merged 3 commits into from
Jan 25, 2023
Merged

Active Job notifications subscriber #1761

merged 3 commits into from
Jan 25, 2023

Conversation

fallwith
Copy link
Contributor

Subscribe to Active Job notifications and process the 7 (at the time of Rails v7.1.0.0.alpha) related events to produce additional instrumentation for the library.

resolves #1515

Subscribe to Active Job notifications and process the 7 (at the time of
Rails v7.1.0.0.alpha) related events to produce additional
instrumentation for the library.
@@ -7,4 +7,11 @@
# required Ruby >= 2.5.0 and Ruby 2.6.0 was marked for EOL
SIMPLECOV_MIN_RUBY_VERSION = '2.7.0'

require 'simplecov' if RUBY_VERSION >= SIMPLECOV_MIN_RUBY_VERSION
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently for some tests with some Rubies, we hit a "file not found" error, which can be puzzling because it's not obvious that there's a LoadError involved (no backtrace is provided) and passing things such as --trace won't reach the subshell used for the env tests.

To repro, simply use Ruby 2.7.5 to perform bundle exec rake test:env[rails52]. It's perfectly reasonable that Ruby 2.7.5 might be used in conjunction with Rails 5.2. But while Ruby 2.7+ will satisfy the SimpleCov helper check, the Rails 5.2 test env Gemfile does not specify simplecov.

Now we simply rescue and report load errors but don't let them stop the tests from running.

- leverage the base NotificationsSubscriber class better
- standardize on NewRelic::UNKNOWN
- update test names
- simplify TODO comment
@github-actions
Copy link
Contributor

SimpleCov Report

Coverage Threshold
Line 93.21% 93%
Branch 84.8% 84%

Copy link
Contributor

@kaylareopelle kaylareopelle left a comment

Choose a reason for hiding this comment

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

Thanks for the changes, James!

@fallwith fallwith merged commit df8a99c into dev Jan 25, 2023
@fallwith fallwith deleted the city_gardens branch January 25, 2023 21:08
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.

Rails notifications: subscribe to ActiveJob topics
2 participants