Should use `Object#public_send` instead of `Object#send`.

[mbj]

I just found that rspec-its is able to call private getters on subject, this is probably not intentional and a side effect from using Object#send instead of Object#public_send.

rspec-its/lib/rspec/its.rb

Line 115 in 38fdc24

inner_subject.send(attr)

[palfvin]

I agree with you, but am a little reluctant to make this change, since it will be a "breaking" one and the Ruby and RSpec precedent is to avoid those at pretty much all costs. Any of the few watchers want to chime in on this?

[mbj]

@palfvin True. It might break peoples specs. But I'd actually think people will be happy to get noticed that their specs where using private interface. //cc @dkubb

[myronmarston]

I agree with you, but am a little reluctant to make this change, since it will be a "breaking" one and the Ruby and RSpec precedent is to avoid those at pretty much all costs. Any of the few watchers want to chime in on this?

The important thing isn't to avoid these sorts of changes; it's to follow SemVer so that users are aware of when new releases have breaking changes. In RSpec we usually hold changes like this off until the next major version, or introduce a config option that enables it (and defaults to false for backwards compatibility).

For rspec-its, I think a config option is kinda overkill (and there's no easy way to hook into rspec --init to make the generated spec_helper.rb turn the option on so that new projects get the benefit of being more prepared for the next major version). Instead, I'd suggest you consider changing this and doing a major version bump of rspec-its. After all, this project is de-coupled from RSpec's release cycle and versioning so you can do a major version bump any time you want :).

One issue to bear in mind is Ruby 1.8.7 -- it lacks public_send. If you're going to switch to public_send you either have to find a fallback for 1.8.7 or drop support for 1.8.7. In RSpec proper we plan to drop support for 1.8.7 in RSpec 4 (which is a long ways out -- we have no plans to start on that anytime soon) but you're free to drop support for 1.8.7 for this gem anytime you want as long as you do so in a major version bump (which this would be anyway)...

[jamesottaway]

My vote would be for dropping 1.8.7 support (since it is EOL and unsupported), but that's just my 2¢.

I fixed the failed build over in #33 without removing support for 1.8.7, but the code would obviously be a lot simpler without the #respond_to? conditional.

[tonyta]

@palfvin I would love to see this merged in for the next major release. This was weird gotcha for us that felt like unexpected behavior. Thanks for all your great work! 😸

[palfvin]

@tonyta Since I'm not sure when/if there will be another "major" release of this little gem, or at least one prior to RSpec 4, would a config option help you out in the meantime?

[mbj]

What about to simply be more opinionated. The use of #send could be delcared a bug, that gets fixed.

I do not have a real problem to force better design on the users of my tools. Especially on a development tool that will NOT be part of production a bigger change might be justifiable.

[jamesottaway]

The question I have is whether #33 as it stands right now is considered to be a breaking change?

I don't think it is, because it still supports the fallback to #send on Ruby 1.8.7, but the argument could be made that because users of newer Rubies will be moved over to #public_send then it is a breaking change which would break tests using rspec-its to tests private methods.

If #33 is considered to be a breaking change, then I vote we drop support for 1.8.7 and simplify the code by not supporting the fallback. In this case I can update #33 and I see no reason to wait for RSpec 4 to release a new major version of RSpec::Its.

If #33 is not considered to be a breaking change, then I think #33 should be merged and released as a minor version bump. In this case, a new PR to drop support for Ruby 1.8.7 should be proposed and the decision for whether to couple the major version bump of rspec-its to RSpec 4 moved to the new PR.

EDIT: Was referring to #31 when I meant #33.

[jamesottaway]

@palfvin Any further thoughts? I'd love to get #33 merged soon.

EDIT: Was referring to #31 when I meant #33.

[pwim]

I just got bitten by this issue as well. I expected the following to be equivalent:

its(:foo) { should eq("bar") }
it { expect(subject.foo).to eq("bar") }

I think it is only in exceptional circumstances that you want to test private methods, so breaking compatibility isn't a bad thing.

[bobf]

This seems like a bug in the rspec-its implementation to me. I would expect it to only test public methods. I just noticed this by luck but I think this is a bug magnet.

Since this behaviour is not documented, do we have to consider it as a breaking change ? I wonder if a configuration option could be added to use the previous functionality.

I guess it's down to your interpretation of "breaking change". Whatever the case, I would really like to see this changed ! Would you accept a PR ?

[pirj]

It is a breaking change, yes. #33 (comment)

If you are up to helping with updating rspec-its to be compatible with RSpec 4, that could become part of its 2.0 release, @bobf.