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

Is RSpec/ReceiveNever even a valid cop? #1755

Open
ekohl opened this issue Dec 23, 2023 · 2 comments
Open

Is RSpec/ReceiveNever even a valid cop? #1755

ekohl opened this issue Dec 23, 2023 · 2 comments

Comments

@ekohl
Copy link

ekohl commented Dec 23, 2023

Summary

The RSpec/ReceiveNever cop says:

Prefer not_to receive(...) over receive(...).never

When running rspec it says:

allow(...).not_to receive is not supported since it doesn't really make sense. What would it even mean?

Either I'm doing something wrong, or the whole cop is invalid.

Details

In #628 the RSpec/ReceiveNever cop was added, but there's this code:
https://github.com/puppetlabs/puppetlabs-stdlib/blob/7e7ded41443750e6643599fa02ad4456e781fede/spec/functions/loadjson_spec.rb#L31-L35

Or a minimal example: allow(JSON).to receive(:parse).never. The cop suggests this should be allow(JSON).not_to receive(:parse) but when I run that, the tests fail with:

$ bundle exec rspec ./spec/functions/loadjson_spec.rb
...
Failures:

  1) loadjson when calling with valid arguments when a non-existing file is specified 
     Failure/Error: allow(JSON).not_to receive(:parse)
       `allow(...).not_to receive` is not supported since it doesn't really make sense. What would it even mean?
     # ./spec/functions/loadjson_spec.rb:34:in `block (4 levels) in <top (required)>'
     # /home/ekohl/.local/share/gem/ruby/bin/rspec:25:in `load'
     # /home/ekohl/.local/share/gem/ruby/bin/rspec:25:in `<top (required)>'
     # /usr/bin/bundle:25:in `load'
     # /usr/bin/bundle:25:in `<main>'
@pirj
Copy link
Member

pirj commented Dec 23, 2023

Interesting. Does

allow(JSON).to receive(:parse).never

work as you expect it to?
Would calling a JSON.parse later in the example result in a failure?

I’d say:

  • we need to ignore allow, and only flag expect
  • we should extend RSpec to warn of allow.to receive.never just like it does for negatiion

Would you like to send such PRs?

@ekohl
Copy link
Author

ekohl commented Jan 2, 2024

Interesting. Does

allow(JSON).to receive(:parse).never

work as you expect it to?

I think this covers all cases:

describe 'rubocop-rspec 1755' do
  describe 'allow.to receive.never' do
    before { allow(Integer).to receive(:sqrt).never }

    it 'passes' do
      # Nothing here
    end

    it 'fails' do
      Integer.sqrt(9)
    end
  end

  describe 'expect.to receive.never' do
    before { expect(Integer).to receive(:sqrt).never }

    it 'passes' do
      # Nothing here
    end

    it 'fails' do
      Integer.sqrt(9)
    end
  end

  describe 'allow.not_to receive' do
    before { allow(Integer).not_to receive(:sqrt) }

    # This never passes since rspec doesn't allow it

    it 'fails' do
      Integer.sqrt(9)
    end
  end

  describe 'expect.not_to receive' do
    before { expect(Integer).not_to receive(:sqrt) }

    it 'passes' do
      # Nothing here
    end

    it 'fails' do
      Integer.sqrt(9)
    end
  end
end

And when running it:

$ bundle exec rspec
.F.FF.F

Failures:

  1) rubocop-rspec 1755 allow.to receive.never fails
     Failure/Error: Integer.sqrt(9)
     
       (Integer (class)).sqrt(9)
           expected: 0 times with any arguments
           received: 1 time with arguments: (9)
     # ./spec/allow_never_spec.rb:10:in `block (3 levels) in <top (required)>'
     # /usr/share/gems/gems/bundler-2.4.10/libexec/bundle:45:in `block in <top (required)>'
     # /usr/share/gems/gems/bundler-2.4.10/libexec/bundle:33:in `<top (required)>'

  2) rubocop-rspec 1755 expect.to receive.never fails
     Failure/Error: Integer.sqrt(9)
     
       (Integer (class)).sqrt(9)
           expected: 0 times with any arguments
           received: 1 time with arguments: (9)
     # ./spec/allow_never_spec.rb:22:in `block (3 levels) in <top (required)>'
     # /usr/share/gems/gems/bundler-2.4.10/libexec/bundle:45:in `block in <top (required)>'
     # /usr/share/gems/gems/bundler-2.4.10/libexec/bundle:33:in `<top (required)>'

  3) rubocop-rspec 1755 allow.not_to receive fails
     Failure/Error: before { allow(Integer).not_to receive(:sqrt) }
       `allow(...).not_to receive` is not supported since it doesn't really make sense. What would it even mean?
     # ./spec/allow_never_spec.rb:27:in `block (3 levels) in <top (required)>'
     # /usr/share/gems/gems/bundler-2.4.10/libexec/bundle:45:in `block in <top (required)>'
     # /usr/share/gems/gems/bundler-2.4.10/libexec/bundle:33:in `<top (required)>'

  4) rubocop-rspec 1755 expect.not_to receive fails
     Failure/Error: Integer.sqrt(9)
     
       (Integer (class)).sqrt(9)
           expected: 0 times with any arguments
           received: 1 time with arguments: (9)
     # ./spec/allow_never_spec.rb:44:in `block (3 levels) in <top (required)>'
     # /usr/share/gems/gems/bundler-2.4.10/libexec/bundle:45:in `block in <top (required)>'
     # /usr/share/gems/gems/bundler-2.4.10/libexec/bundle:33:in `<top (required)>'

Finished in 0.01234 seconds (files took 0.08014 seconds to load)
7 examples, 4 failures

Failed examples:

rspec ./spec/allow_never_spec.rb:9 # rubocop-rspec 1755 allow.to receive.never fails
rspec ./spec/allow_never_spec.rb:21 # rubocop-rspec 1755 expect.to receive.never fails
rspec ./spec/allow_never_spec.rb:31 # rubocop-rspec 1755 allow.not_to receive fails
rspec ./spec/allow_never_spec.rb:43 # rubocop-rspec 1755 expect.not_to receive fails

So they all appear to be the same, except for the one "What would it even mean?".

we need to ignore allow, and only flag expect

Given how rspec behaves, I'd say that makes sense now.

we should extend RSpec to warn of allow.to receive.never just like it does for negatiion

Agreed. This always confused me. Especially combined with the RSpec/MessageSpies in its default have_received mode which enforces allow + have_received over expect.

Would you like to send such PRs?

I wouldn't know where to get started and I'm afraid I don't have that much time available to dive into it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants