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

Closed
Closed
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: 2 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,5 @@ 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'
Copy link
Member

Choose a reason for hiding this comment

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

Gemfile-custom is the best place for it.

2 changes: 1 addition & 1 deletion features/its.feature
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ Feature: attribute of subject
Person
with one phone number (555-1212)
phone_numbers.first
is expected to eq "555-1212"
should eq "555-1212"
Copy link
Member

Choose a reason for hiding this comment

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

Why?

"""

Scenario: specify value of an attribute of a hash
Expand Down
31 changes: 30 additions & 1 deletion lib/rspec/its.rb
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ module Its
#
# # This ...
# describe Array do
# its(:size, :focus) { should eq(0) }
# its(:size, focus: true) { should eq(0) }
Copy link
Member

Choose a reason for hiding this comment

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

Why?

Copy link
Author

@zirni zirni Mar 19, 2021

Choose a reason for hiding this comment

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

Reverted it again :)

# end
#
# # ... generates the same runtime structure as this:
Expand Down Expand Up @@ -131,7 +131,31 @@ def its(attribute, *options, &block)
else
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
Copy link
Member

Choose a reason for hiding this comment

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

RSpec.configuration.its_private_method_debug.nil? ?


# 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."
Copy link
Member

Choose a reason for hiding this comment

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

You can use RSpec.deprecate.
I personally wouldn't want to support testing private methods implicitly, and then it would just make sense to issue a deprecation warning in 1.x, and move to public_send in 2.0.


# TODO Add deprecation warning to drop private method calling
# for the next major release
end
end

inner_subject.send(attr)

end
end
end
Expand Down Expand Up @@ -175,6 +199,11 @@ def should_not(matcher=nil, message=nil)
end

RSpec.configure do |rspec|

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

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

def name
'Maria'
end
end.new
end

its(:name, :focus) { should eq('Maria') }
Copy link
Member

Choose a reason for hiding this comment

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

JFYI should is going away in RSpec 4.

end
context "with explicit subject" do
subject do
Class.new do
Expand Down
2 changes: 2 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,6 @@ def method_missing(method, *args, &block)
RSpec.configure do |config|
config.run_all_when_everything_filtered = true
config.order = 'random'

# config.its_private_method_debug = true
end