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

undetermined behaviour using parenthesis or curly-braces in expect #525

Closed
aaronlevin opened this issue Apr 16, 2014 · 10 comments
Closed

undetermined behaviour using parenthesis or curly-braces in expect #525

aaronlevin opened this issue Apr 16, 2014 · 10 comments

Comments

@aaronlevin
Copy link

It is unclear where this issue resides, but I'm pretty certain it's in expect and happens when you alternate between using curly or parenthesis. In this case, I've created a class that calls super in various ways and in two cases using expect( ... ) produces an ArgumentError.

The main issue is that the error returned (ArgumentError) is unclear and obscures the real issue.

I am submitting this as an issue because the behaviour is unclear. It seems that using curly braces is the preferred solution always, but nonetheless, if they're not being used a suitable error should be thrown.

Here is how you can reproduce it simply:

# This class calls `super` and its `initialize` takes a parameter
# *causes error*
class A
  def initialize(foo = nil)
    super
  end
end

# This class does not call `super`
# no error
class B
end

# This class calls `super` and its `initialize` takes no parameters
# no error
class C
  def initialize
    super
  end
end

# This class calls `super` and its `initialize` takes parameters
# it passes those parameters to `super` explicitly
# *causes error*
class D
  def initialize(foo = nil)
    super(foo)
  end
end

# This class calls `super` and its `initialize` takes a parameter, it "splats" the arguments to `initialize`.
# no error
class E
  def initialize(foo = nil)
    super(*foo)
  end
end

# The tests below check for an error being raised when calling `expect? X.new ?.not_to be(nil)` 
# where ? is either '{' or '(' and X is one of (A,B,C,D,E).

describe 'Ruby keyword `super`:' do
  it 'In a class using `super`, expect{ A.new } does not raise ArgumentError' do
    expect{
      expect{ A.new }.not_to be(nil)
    }.not_to raise_error
  end
  it 'In a class using `super`, expect( A.new ) does not raise ArgumentError' do
    expect{
      expect( A.new ).not_to be(nil)
    }.not_to raise_error
  end
  it 'In a class not using `super`, expect{ B.new } does not raise ArgumentError' do
    expect{
      expect{ B.new }.not_to be(nil)
    }.not_to raise_error
  end
  it 'In a class not using `super`, expect( B.new ) does not raise ArgumentError' do
    expect{
      expect( B.new ).not_to be(nil)
    }.not_to raise_error
  end
  it 'In a class not using `super`, expect{ C.new } does not raise ArgumentError' do
    expect{
      expect{ C.new }.not_to be(nil)
    }.not_to raise_error
  end
  it 'In a class not using `super`, expect( C.new ) does not raise ArgumentError' do
    expect{
      expect( C.new ).not_to be(nil)
    }.not_to raise_error
  end
  it 'In a class not using `super`, expect{ D.new } does not raise ArgumentError' do
    expect{
      expect{ D.new }.not_to be(nil)
    }.not_to raise_error
  end
  it 'In a class not using `super`, expect( D.new ) does not raise ArgumentError' do
    expect{
      expect( D.new ).not_to be(nil)
    }.not_to raise_error
  end
  it 'In a class not using `super`, expect{ E.new } does not raise ArgumentError' do
    expect{
      expect{ E.new }.not_to be(nil)
    }.not_to raise_error
  end
  it 'In a class not using `super`, expect( E.new ) does not raise ArgumentError' do
    expect{
      expect( E.new ).not_to be(nil)
    }.not_to raise_error
  end

end

The result is:

☭ rspec rspec_super_issue.rb 
.F.....F..

Failures:

  1) Ruby keyword `super`: In a class using `super`, expect( A.new ) does not raise ArgumentError
     Failure/Error: expect{
       expected no Exception, got #<ArgumentError: wrong number of arguments (1 for 0)> with backtrace:
         # ./rspec_super_issue.rb:5:in `initialize'
         # ./rspec_super_issue.rb:5:in `initialize'
         # ./rspec_super_issue.rb:47:in `new'
         # ./rspec_super_issue.rb:47:in `block (3 levels) in <top (required)>'
         # ./rspec_super_issue.rb:46:in `block (2 levels) in <top (required)>'
     # ./rspec_super_issue.rb:46:in `block (2 levels) in <top (required)>'

  2) Ruby keyword `super`: In a class not using `super`, expect( D.new ) does not raise ArgumentError
     Failure/Error: expect{
       expected no Exception, got #<ArgumentError: wrong number of arguments (1 for 0)> with backtrace:
         # ./rspec_super_issue.rb:27:in `initialize'
         # ./rspec_super_issue.rb:27:in `initialize'
         # ./rspec_super_issue.rb:77:in `new'
         # ./rspec_super_issue.rb:77:in `block (3 levels) in <top (required)>'
         # ./rspec_super_issue.rb:76:in `block (2 levels) in <top (required)>'
     # ./rspec_super_issue.rb:76:in `block (2 levels) in <top (required)>'

Finished in 0.00226 seconds
10 examples, 2 failures

Failed examples:

rspec ./rspec_super_issue.rb:45 # Ruby keyword `super`: In a class using `super`, expect( A.new ) does not raise ArgumentError
rspec ./rspec_super_issue.rb:75 # Ruby keyword `super`: In a class not using `super`, expect( D.new ) does not raise ArgumentError
@aaronlevin
Copy link
Author

Some discussion here: https://gist.github.com/weirdcanada/1313a95e3d891eebca32

@aaronlevin
Copy link
Author

(I am also open to this potentially not being an issue in rspec)

@myronmarston
Copy link
Member

Parenthesis vs curly braces are a red herring here. The issue is #new being called or not. When you pass a value to expect (as in expect(A.new)), A.new is evaluated immediately in order to get the value to pass to expect. When you pass a block to expect, the block gets passed on to the matcher (not the return value of the block), and the block will only be evaluated if the matcher calls the block. The be matcher doesn't call the block (why would it? a block is not nil), so for your examples that use curly braces #new isn't even being called.

In general, you should be passing an argument to expect. The only time to use a block is when you are specifying something that cannot be represented as an object, such as:

  • raising an error (raise_error)
  • throwing a symbol (throw_symbol)
  • yielding (yield_control and friends)
  • changing a value (change)

Let me know if that doesn't make sense.

@aaronlevin
Copy link
Author

@myronmarston thanks for your detailed response and for re-writing the issue.

I'm not sure if I quite follow you, though. By your explanation, the following should not throw the ArgumentError, yet it still does:

x = A.new
expect( x ).not_to be(nil)

Am I understanding your delineation between explicitly passing a value and not?

@aaronlevin
Copy link
Author

@myronmarston ah, nm, I see what you're saying now. A.new causes an error and this is not being caught in { } because the matcher compares a block to nil and already knows its not nil.

This seems to be a bug as the proc may simply return nil, no?

@cupakromer
Copy link
Member

@weirdcanada that's not exactly correct. The be matcher never sends the call message to the block. So the ArgumentError is never raised. Instead the block is treated as a Proc instance and compared as such.

Only certain matchers expect blocks.

@aaronlevin
Copy link
Author

@cupakromer ah, I see. Thank you. Today we all learn a valuable lesson in referential transparency :)

@myronmarston
Copy link
Member

BTW, this inspired me to open #526 to help better guide users of when to pass an argument vs a block.

@myronmarston
Copy link
Member

@weirdcanada -- if you try your example against master, you'll find it provides much clearer errors.

@aaronlevin
Copy link
Author

@myronmarston awesome. thank you. I'll try this at work wednesday!

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

3 participants