rspec/rspec-core

Feature request: shorthand for `expect { subject }.to change`

okuramasafumi opened this issue · 3 comments

Subject of the issue

Current syntax to assert a method to change something with subject looks like this:

expect { subject }.to change(Foo, :count)

This syntax has some problems.

  • It's not readable especially subject is not named because subject doesn't imply it has some side effects
  • It looks similar to expect(subject) but works totally differently and to understand its behavior we need to learn how Ruby blocks work first

Proposal

My proposal is to add something like below to rspec-core or rspec-expections.

is_expected_to_change(Foo, :count)

This is essentially the same as:

expect { subject }.to change(Foo, :count)

Notes

Current behavior of is_expected evaluates subject when it's called so we cannot use it here.

I believe this syntax is more readable and worth adding for readability that rspec values.

pirj commented

Thanks for the suggestion. I have some doubts:

  • is_expected_to_change sounds overly specific
  • how would a negated expect { subject }.not_to change look like?
  • if we'd introduce a negated form, would it be composable with .and/.or?
  • what about other block matchers, e.g. raise/yield/output?
expect { subject }.to change(Foo, :count)

This syntax has some problems.

It's not the syntax, it's the usage.

It's not readable especially subject is not named

subject is not meant to be used directly (see this code doc).

to use subject explicitly in examples ... we recommend that you define a method with an intention revealing name instead

subject doesn't imply it has some side effects

How about?

subject(:perform_request!) { get :show }

it do
  expect { perform_request! }.to change(Foo, :count)
end

It looks similar to expect(subject) but works totally differently and to understand its behavior we need to learn how Ruby blocks work first.

Indeed it does. In this regard, won't is_expected_to_change create even more confusion when compared to is_expected? What if the block of code has no side effects, e.g.

subject { 'foo' }

and is_expected_to_change will be used with it?

It's not a big deal to globally define

def is_expected_to_change(...)
  expect { subject }.to change(...)
end

and include it in your RSpec config if it matches your coding style.
But I'm not convinced enough it is something we want to include in the library for widespread use.

Thanks for taking the time to propose this but we prefer to encourage naming subjects and not use implicit subjects at all, other implicit usage has been extracted out over time to other gems e.g. rspec-its .

I see, thanks for clarifying it!