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

Proposal: Inform user when testing a private method #82

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
2 changes: 0 additions & 2 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,3 @@ gem 'contracts', '< 0.16' if RUBY_VERSION < '1.9.0'
gem 'coveralls', :require => false, :platform => :mri_20

eval File.read('Gemfile-custom') if File.exist?('Gemfile-custom')

gem 'pry'
4 changes: 2 additions & 2 deletions features/its.feature
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ Feature: attribute of subject
person
end

its("phone_numbers.first") { should eq("555-1212") }
its("phone_numbers.first") { is_expected.to eq("555-1212") }
end
end
"""
Expand All @@ -61,7 +61,7 @@ Feature: attribute of subject
Person
with one phone number (555-1212)
phone_numbers.first
should eq "555-1212"
is expected to eq "555-1212"
"""

Scenario: specify value of an attribute of a hash
Expand Down
28 changes: 12 additions & 16 deletions lib/rspec/its.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
require 'rspec/its/version'
require 'rspec/core'

RSpec.deprecate <<EOS
rspec-its deprecates calling _send_ in favour of _public_send_ internally.
This change will be introduced with version 2.0.
Maybe you're testing private methods in your test suite without your intention.
If you need more information about which test is affected,
set `config.its_raise_errors_for_private_method_calling = true`
EOS

Copy link
Member

Choose a reason for hiding this comment

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

This will be quite annoying. I would expect to see a deprecation message when I'm testing a private method and there's really a difference between using send and public_send.

Please also check usages of deprecate, its interface is not too friendly, but surely you'll find an example that would fit this usage scenario.

module RSpec
module Its

Expand Down Expand Up @@ -95,7 +103,7 @@ module Its
#
# # This ...
# describe Array do
# its(:size, focus: true) { should eq(0) }
# its(:size, :focus) { should eq(0) }
# end
#
# # ... generates the same runtime structure as this:
Expand Down Expand Up @@ -132,30 +140,18 @@ def its(attribute, *options, &block)
attribute_chain = attribute.to_s.split('.')
attribute_chain.inject(subject) do |inner_subject, attr|

# When NilClass: user didnt set the rspec setting
# When FalseClass: skip this block
if RSpec.configuration.its_private_method_debug.is_a? NilClass
if RSpec.configuration.its_raise_errors_for_private_method_calling

# respond_to? only looks in the public method space
# TODO Check NoMethodError
# TODO Check if inner_subject some kind of const
# inner_subject.method_defined?(attr) && !inner_subject.respond_to?(attr)
if !inner_subject.respond_to?(attr)

if RSpec.configuration.its_private_method_debug
# TODO if debug: show spec:line_num for better indication
puts its_caller.first
end

puts "You are testing a private method. Set RSpec its_private_method_debug setting to show more info or hide it completly."

# TODO Add deprecation warning to drop private method calling
# for the next major release
raise 'Testing private method' + attr
end
end

inner_subject.send(attr)

end
end
end
Expand Down Expand Up @@ -202,7 +198,7 @@ def should_not(matcher=nil, message=nil)

# Add RSpec configuration setting to allow
# the user to handle private method invoking warning
rspec.add_setting :its_private_method_debug
rspec.add_setting :its_raise_errors_for_private_method_calling
Copy link
Member

Choose a reason for hiding this comment

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

Keeping in mind there's configuration.raise_errors_for_deprecations! I'm on the fence if we need this setting, or a deprecation message will be sufficient.


rspec.extend RSpec::Its
rspec.backtrace_exclusion_patterns << %r(/lib/rspec/its)
Expand Down
25 changes: 21 additions & 4 deletions spec/rspec/its_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,36 @@ module RSpec
its([]) { expect(described_class).to be Its }
end
end
context "" do

context "raises error when calling a private method" do
around(:each) do |example|
RSpec.configuration.its_raise_errors_for_private_method_calling = true
example.run
RSpec.configuration.its_raise_errors_for_private_method_calling = false
end

subject do
Class.new do
private

def name
'Maria'
end
end.new

def self.name
end
private_class_method :name
end
end

context 'on an instance' do
its('new.name') { will raise_error }
end

its(:name, :focus) { should eq('Maria') }
context 'on a class' do
its('name') { will raise_error }
end
end

context "with explicit subject" do
subject do
Class.new do
Expand Down