Proposal: Inform user when testing a private method #82
Conversation
as a migration path to safely move from send to public_send
Gemfile
Outdated
| @@ -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' | |||
There was a problem hiding this comment.
Gemfile-custom is the best place for it.
features/its.feature
Outdated
| @@ -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" | |||
lib/rspec/its.rb
Outdated
| @@ -95,7 +95,7 @@ module Its | |||
| # | |||
| # # This ... | |||
| # describe Array do | |||
| # its(:size, :focus) { should eq(0) } | |||
| # its(:size, focus: true) { should eq(0) } | |||
lib/rspec/its.rb
Outdated
|
|
||
| # When NilClass: user didnt set the rspec setting | ||
| # When FalseClass: skip this block | ||
| if RSpec.configuration.its_private_method_debug.is_a? NilClass |
There was a problem hiding this comment.
RSpec.configuration.its_private_method_debug.nil? ?
lib/rspec/its.rb
Outdated
| 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." |
There was a problem hiding this comment.
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.
spec/rspec/its_spec.rb
Outdated
| end.new | ||
| end | ||
|
|
||
| its(:name, :focus) { should eq('Maria') } |
There was a problem hiding this comment.
JFYI should is going away in RSpec 4.
when calling a private method
|
Added the deprecation warning as you mentioned. |
lib/rspec/its.rb
Outdated
| 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 | ||
|
|
There was a problem hiding this comment.
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.
| @@ -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 | |||
There was a problem hiding this comment.
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.
|
Now deprecation warnings are thrown up once private method tests were detected, as you mentioned. If the user test suite has a bunch of private method tests, the deprecation warnings would grow to a possible annoying size. |
We are printing deprecations in the footer. There's also a limit for the number of deprecations of the same kind https://github.com/rspec/rspec-core/blob/ea8554afd1a2b63677c6593059fa8f2476181deb/lib/rspec/core/formatters/deprecation_formatter.rb#L136 I didn't try it, but I would suggest relying on this existing functionality. |
Hello guys,
please take a look about this feature proposal
for safely migrate from send to public_send
mentioned #28
I'd like to know if this would be an approach you would accept.
Then I would finalize this little piece.
Cheers