Skip to content

Fix undefined method `example_group' for nil & NoMethodError: render_views? for feature specs #1827

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Fix undefined method `example_group' for nil & NoMethodError: render_views? for feature specs #1827

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bfa6c3f
Fix undefined method `example_group' for nil & NoMethodError: render_…
aCandidMind May 31, 2017
ad40aae
Only attach `LogSubscriber` for `Rails::ViewRendering` when needed
aCandidMind May 31, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions lib/rspec/rails/view_rendering.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,17 +64,19 @@ def self.nullify_template_rendering(templates)
# @private
class LogSubscriber < ::ActiveSupport::LogSubscriber
def current_example_group
RSpec.current_example.example_group
# When running feature specs current_example can be nil if the subscriber runs in a different context
# than the specs themselves. E.g. in a capybara thread.
RSpec.current_example.try!(:example_group)
Copy link
Member

Choose a reason for hiding this comment

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

Please don't use try! here, we try to avoid the usage of non standard Ruby, you could add a guard to return nil if theres no current example but really we should figure out why its nil.

Copy link
Author

Choose a reason for hiding this comment

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

RSpec.current_example is nil in the case where the request came from the (Selenium) webdriver out of the browser (e.g. via AJAX) and not from an RSpec example group. I'll add that little extra info to the code comment.

Copy link
Member

Choose a reason for hiding this comment

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

No don't thats not the cause, this is only executed during a test process when this should have a thread value, its not selenium causing it.

end

def render_template(_event)
return if current_example_group.render_views?
info(" Template rendering was prevented by rspec-rails. Use `render_views` to verify rendered view contents if necessary.")
example_group = current_example_group
if example_group.respond_to?(:render_views?) && !example_group.render_views?
Copy link
Member

Choose a reason for hiding this comment

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

We prefer guard condition style returns, prehaps just change the first line to:
return if !example_group.respond_to?(:render_views?) || !example_group.render_views?

info(" Template rendering was prevented by rspec-rails. Use `render_views` to verify rendered view contents if necessary.")
end
end
end

LogSubscriber.attach_to(:action_view)
Copy link
Member

Choose a reason for hiding this comment

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

Why did this have to move?

Copy link
Author

Choose a reason for hiding this comment

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

It's only an optimization for the case when no controller specs are run. E.g. when only running feature specs it should make things a little faster since not every template rendering is going through that Subscriber and so on. I could easily refrain from moving it.


# Delegates all methods to the submitted resolver and for all methods
# that return a collection of `ActionView::Template` instances, return
# templates with modified source
Expand Down Expand Up @@ -146,6 +148,9 @@ def _path_decorator(*paths)
end

included do
# Make sure to log when template rendering was prevented by rspec-rails
EmptyTemplateResolver::LogSubscriber.attach_to(:action_view)

before do
unless render_views?
@_original_path_set = controller.class.view_paths
Expand Down
2 changes: 1 addition & 1 deletion spec/rspec/rails/view_rendering_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ def logger
ActiveSupport::LogSubscriber.logger
end

pending 'does not cause an error' do
it 'does not cause an error' do
expect(logger).not_to receive(:error).with(a_string_starting_with('Could not log "render_template.action_view" event.'))
view.render(body: 'foo')
end
Expand Down