-
-
Notifications
You must be signed in to change notification settings - Fork 482
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
New features in 2.7.0 break my tests #773
Comments
Ah crap I forgot this didn't work correctly, I'll push a fix tomorrow. |
Awesome! Thanks Nate. |
This breaks for me on a normal production |
That is strange that build has passed all 1760 test examples at CI and fails on simple run within controller. Shouldn't this default use-case been covered with test? |
If it's helpful to have a link to an open-source project with failing specs, there's one here. Looks like tests are failing on 60% of the PRs Dependabot created for this update this morning, so I'll comment in on all of those and let people know you've got a fix incoming. |
Given all the feedback here, yanking 2.7.0. |
This change worked for my project: diff --git a/lib/raven/integrations/rails/controller_transaction.rb b/lib/raven/integrations/rails/controller_transaction.rb
index da12a46..23506ba 100644
--- a/lib/raven/integrations/rails/controller_transaction.rb
+++ b/lib/raven/integrations/rails/controller_transaction.rb
@@ -2,12 +2,10 @@ module Raven
class Rails
module ControllerTransaction
def self.included(base)
- base.class_eval do
- around_action do |controller|
- Raven.context.transaction.push "#{controller.class}##{controller.action_name}"
- yield
- Raven.context.transaction.pop
- end
+ base.around_action do |controller, block|
+ Raven.context.transaction.push "#{controller.class}##{controller.action_name}"
+ block.call
+ Raven.context.transaction.pop
end
end
end I haven't find a way to reproduce it in a test case though :/ |
There were a few messups here: * The Rails controller tests never tested a "happy case" (that is, no exceptions) * The Rails controller tests didn't check to see the exception they were actually catching - so the LocalJumpError went unnoticed! * You can't `yield` from the block form of `around_action` * I'm not sure why I thought class_eval was necessary here - it isn't. Fix #773
I'm 99.8% sure I got this, but can someone confirm that their applications test's pass on #774? |
Sure thing - checking now. |
There were a few messups here: * The Rails controller tests never tested a "happy case" (that is, no exceptions) * The Rails controller tests didn't check to see the exception they were actually catching - so the LocalJumpError went unnoticed! * You can't `yield` from the block form of `around_action` * I'm not sure why I thought class_eval was necessary here - it isn't. Fix #773
There was an issue with gem raven-sentry v2.7.0 so we better pin it to 2.6.x for now. getsentry/sentry-ruby#773
Fixed in 2.7.1. |
Thanks Nate! |
The functionality in
controller_transaction.rb
seems to blow up my tests with the following error:2.6.3 works just fine. @nateberkopec it looks like someone else mentioned this in the comments on PR #743, so I thought I'd open an issue.
The text was updated successfully, but these errors were encountered: