rspec-expectations: is_expected with block expectations

The following situation just confused someone here at work:

describe 'is_expected with block expectation' do
  subject { raise }

  it { is_expected.to raise_error }
end

That reads like it should work, but doesn’t because is_expected is defined as expect(subject).

I don’t typically use is_expected, so not sure if there is a canonical way to do this already. It isn’t listed at https://www.relishapp.com/rspec/rspec-core/docs/subject/one-liner-syntax

  1. Is there a “correct way” to do this?
  2. Is one-liner syntax something that we (as a core team) use much, or is it in the expect_any_instance_of bucket of extras?
  3. Should we add one-liner Relish documentation about using block matchers? I’m thinking yes.
  4. Is there a way to fix this transparently, and if yes, should we? My gut says no because it would be switching is_expected implementation based on matcher used and that seems gnarly.

Tangentially related discussion thread on separating block vs value expectations, in which we didn’t discuss this case: https://github.com/rspec/rspec-expectations/issues/526

cc @rspec/rspec

About this issue

  • Original URL
  • State: closed
  • Created 9 years ago
  • Reactions: 11
  • Comments: 23 (11 by maintainers)

Commits related to this issue

Most upvoted comments

Not with the one liner syntax that I’m aware of.

There is, if you’re willing to make your subject a lambda:

describe 'is_expected with block expectation' do
  subject { -> { raise } }

  it { is_expected.to raise_error }
end

…that’s pretty obtuse and not something I’d recommend, generally.

Is one-liner syntax something that we (as a core team) use much, or is it in the expect_any_instance_of bucket of extras?

I consider it in between. It’s not something I use frequently, and it’s an often abused feature of RSpec…but it’s not generally problematic the way expect_any_instance_of is, and there are situations where it works really, really well. I think mustermann has a great example of its usage (although it’s using the older should one-liner syntax, but I consider them interchangable). I think for a case like mustermann’s, you want the details of the exact string you are matching against to be included in the doc output (as coming up with prose descriptions for all those would be tricky and more ambiguous). Letting the matcher generate the description for the example keeps them in sync, ensuring you don’t wind up with doc strings that “lie” like comments can, which can lead to other problems.

Should we add one-liner Relish documentation about using block matchers? I’m thinking yes.

We should mention they’re not supported.

Agreed.

Is there a way to fix this transparently, and if yes, should we? My gut says no because it would be switching is_expected implementation based on matcher used and that seems gnarly.

I think the complexity to doing such a thing would be greater than the benefit. I also think it’s good for people to understand that the expectation is being set on a lambda/proc that wraps an expression, and not on the return value of the expression itself. Any attempt to make this work nicely would hide that distinction which would only lead to more confusion.

Also, you’ll generally get warnings from RSpec 3 when you attempt to use a block matcher against a non-block subject. That falls over for raise_error, unfortunately, because in an expression like expect(raise).to raise_error ruby evaluates the raise first and it aborts before RSpec even has a chance to do anything with it.

Could maybe add a is_expected_block syntax for this?

I don’t think it reads well but if you want to add it to a project (or build a gem that provides it), it’s trivial:

module IsExpectedBlock
  def is_expected_block
    expect { subject }
  end
end

RSpec.configure do |c|
  c.include IsExpectedBlock
end

I ended up calling it “anticipation”. Compared to an “expectation” it describes the expectation of a future state more explicitly in my eyes (english native speakers please correct me).

it { is_anticipated.to raise_exception('Because I made it fail intentionally') }

Using the following configuration:

module IsAnticipated
  def is_anticipated
    expect { subject }
  end
end

RSpec.configure do |config|
  config.include IsAnticipated
end

I’ve decided to use this in my project:

module WithinBlockIsExpected
  def within_block_is_expected
    expect { subject }
  end
end

RSpec.configure do |config|
  config.include WithinBlockIsExpected
end

and in spec:

it { within_block_is_expected.not_to raise_exception }

@myronmarston I basically ended up doing the same thing in a project, but I named it #will_be_expected

@pirj Thank you for summing up the references. The downsides seem plausible to me. But I see myself often use the one-liner syntax and it just bugs me a lot to break the consistency of a test block because is_expected cannot handle blocks.

I seem to like the kind of this pattern:

describe '#do_something' do
  subject { instance.do_something }
  it { is_expected.to be_successful }
  it { is_expected.to have_attributes(blub: …) }
  it { is_expected.to change { sut } }
  it { is_expected.to make(sut).receive(:request) }
  …

For me this reads a lot more like a specification list than what we’re doing with it on multiple lines. In the end I want to define one single behaviour with a list of expected specifications it has to fulfil. And the cool thing about RSpec is that I can do this in a very slick way (but not with a block in the subject).

if is_expected would be defined as

def is_expected
   expect { subject }
end

( using { } instead of ( )) then it should work. Also will help with is_expected.to change when subject is a method call with side effects, and it would make “unit specs” with a lot of describe "#method" do sections much shorter, while mantaining readability.

But it could slow down a little every is_expected call. Any other reasons, why it wasn’t done in that way?

Since it’s related to exceptions it could be something like

def is_expected_to_raise(*args)
  expect { subject }.to raise_exception(*args)
end
# and use it like
it { is_expected_to_raise SomeError, 'error message' }`

Even though I think naming the subject will produce very expressive sentence too.

describe '#some_method' do
  subject(:some_method) { described_class.new(...).some_method(invalid_args) }
  it { expect { some_method }.to raise_exception(SomeError, 'error message') }
end

Any other reasons, why it wasn’t done in that way?

The reason is that expect { subject } only works with block expectations. Non-block expectations (e.g. eq, include, be_within, be <, etc) can only be used off of expect(object), not expect { object }. There’re a few reasons for that:

  • The generic expect {}.to matcher machinery does not evaluate the block – instead the block is passed to the matcher so the matcher can evaluate it. For this block matchers, this is important because the matcher must “wrap” evaluation of the block with special logic (such as rescuing an error, etc).
  • That means that if you tried expect { 5 }.to eq(5), the eq matcher would receive proc { 5 } as an argument to match against 5. The eq matcher checks == of the two arguments. proc { 5 } == 5 is obviously not true so it won’t match.
  • Procs are objects, too, which means that it’s completely valid for eq to compare two proc objects for equality – which in turn means that eq can’t safely evaluate a proc if that’s what it’s given.

See #526 for more background and info on this.