Chaining different block-expecting matchers

[Nick Sutterer]

Hello my friends, happy hollidays!

I'm referring to this issue: https://github.com/rspec/rspec-expectations/issues/1336 (I wish I had waited one more ticket so I'd grab #1337)

Basically, I'm trying to implement a matcher that can be chained with others after an `expect {}` , as in the following snippet.

```ruby
expect { run({duration: "2.24"}) }
      .to pass
      .and change { "yo" }.by(1)

```

When running this, I get this error:
NoMethodError:
       undefined method `success?' for #<Proc:0x000055697246eea8 /home/nick/projects/rspec-trailblazer/spec/expectation_spec.rb:46>

My matcher, when used in a chain, doesn't receive the `actual` result object (specific to `run` and our library) but a Proc instance. How would I write a matcher that supports the block syntax?

Cheers,

Nick

[Phil Pirozhkov]

Hi Nick!

> #1337

It's still up for the grabs ;)

> I'm referring to this issue: https://github.com/rspec/rspec-expectations/issues/1336

Basically, I'm trying to implement a matcher that can be chained with others after an `expect {}` , as in the following snippet.

```ruby
expect { run({duration: "2.24"}) }
      .to pass
      .and change { "yo" }.by(1)

```

When running this, I get this error:
NoMethodError:
       undefined method `success?' for #<Proc:0x000055697246eea8 /home/nick/projects/rspec-trailblazer/spec/expectation_spec.rb:46>

My matcher, when used in a chain, doesn't receive the `actual` result object (specific to `run` and our library) but a Proc instance. How would I write a matcher that supports the block syntax?

I believe the distinction is that block matchers check side effects, while value matchers - returned value.

The documentation for building block matchers is here https://relishapp.com/rspec/rspec-expectations/v/3-10/docs/custom-matchers/define-a-matcher-supporting-block-expectations

But I believe it's confusing in terms of telling a story how to create a block matcher. Frankly, I can't create a usable composable block matcher using those docs.

To add to the confusion, there are actually two ways of creating matchers. The first one is with `RSpec::Matchers.define`, and another one is by writing a regular class.

I usually start with `define`, get quickly frustrated and switch to a class.

One example is this matcher https://github.com/pirj/rspec-enqueue_sidekiq_job/blob/main/lib/rspec/enqueue_sidekiq_job.rb#L73

But it's not checking the returned value along the way.

So I quickly came up with this:

```

$foo = 0

class Bar
  include RSpec::Matchers::Composable

  def initialize(expected_return_value)
    @expected_return_value = expected_return_value
  end

  def matches?(block)
    @actual_return_value = block.call

    values_match?(@expected_return_value, @actual_return_value)
  end

  def failure_message(*args)
    "Expected: #{@expected_return_value}, got: #{@actual_return_value}"
  end

  def supports_block_expectations?
    true
  end
end

module RSpec
  module Matchers # reopen for simplicity. you can `config.include MyModule` in spec_helper
    def bar(expected_return_value)
      Bar.new(expected_return_value)
    end
  end
end

RSpec.describe "a custom block matcher" do
  specify do
    expect { $foo = 2 }
      .to bar(2)
      .and change { $foo }.by(2)
  end
end

```

and it works!

However, if you change the order:

```

    expect { $foo = 2 }
      .to change { $foo }.by(2)
      .and bar(2)

```

it starts complaining that `@actual_return_value` (the result of the block being called) is `true`, not `2`.

It seems to me that only the innermost matcher can get the return value of the block, and it's never passed over to composed matchers.

Can you work with that? This would imply that the matcher that checks the return value is the always the first one in the chain.

It feels doable to pass the returned value of the block along.

We've restricted value matchers from working with block expectations in RSpec 4, i.e.

```

expect { 1 }.to eq(1)

```

will raise an error.

But I have no certainty if this feature will not be confusing. Even though it might be usable in some cases, e.g.:

```

expect { post :delete }

  .to be_ok

  .and change { User.count }.by(-1)

```

instead of

```

expect { post :delete }

  .to change { User.count }.by(-1)

expect(response).to be_ok

```

Let me know what you think.

> happy holidays

Happy holidays to you too, Nick!

- Phil

[Nick Sutterer]

Phil, thank you so much for your help! <3 This is really really appreciated!

> Can you work with that? This would imply that the matcher that checks the return value is the always the first one in the chain.

Yeah, but don't you think this will be confusing at some point? Seems like I might have to think about a different approach.

Right now, I am wondering how RSpec power-users tackle this very common test case:

1. take a snapshot of ABC

2. run your test code
3. compare side-effects of ABC

4. test additional effects on D, ...

I know from several consulting projects that people use something along the following lines.

```
expect {
          expect(operation.call.success?).to eq true
        }.to change { A.count } .by(1)
          .and change { B.count } .by(1)
          .and change { C.count } .by(1)

        expect(D.last.status).to eq("active")

       # ...

```

I'd love to have that look as follows.

```
expect {
          operation.call # provides "actual"
        }.to pass # needs "actual"

          .and change { A.count } .by(1)
          .and change { B.count } .by(1)
          .and change { C.count } .by(1)

        expect(D.last.status).to eq("active")

       # ...

```

However, we ran into the problems described above, not receiving the actual value, and so on. I'm a bit clueless right now as of I don't know if I am missing anything RSpec-y here (am I trying to do something "stupid" that RSpec is not designed for?) or is it RSpec that might benefit from the concept of ordered chained matchers that allow accessing actual after it has been evaluated?

Thanks again!

[Nick Sutterer]

Sorry, I forgot to formulate what is on my mind: it seems to me that chained `change` matcher share the "actual" value or at least evaluate it only once. Couldn't another 3rd-party matcher `pass` (or whatever) be part of that chain? Or, to put it the other way round, how do the `change` matcher make sure they evaluate the assertion code only once?

[Phil Pirozhkov]

> chained `change` matcher share the "actual" value or at least evaluate it only once

If you expand the code

```

expect { action! }

  .to change { A.count }

  .and change { B.count }

```

it will be something like

```

prev_b = exp_block_b.call # last `change`

prev_a = exp_block_a.call # first `change`

action_block.call # and the return value is not passed around, but the matcher has access to it

final_a = exp_block_a.call # first `change`

final_b = exp_block_b.call # last `change`

prev_a != final_a && prev_b != final_b

```

> Couldn't another 3rd-party matcher `pass` (or whatever) be part of that chain?

Yes, but with the current implementation only if it's the first in the chain. I.e. `expect { action! }.to pass.and change { A.count }` would work, but `expect { action! }.to change { A.count }.and pass` won't.

> expect {
>   expect(operation.call.success?).to eq true
> }.to change { A.count } .by(1)

That's the approach to use. However, I don't see an obvious way of making a slick DSL out of it.

Let me check if it's possible to make Composable to pass not only the "matches" flag, but also the return value of `action!` so that it's automatically compatible with all block matchers.

- Phil

[Phil Pirozhkov]

Hi Nick,

I came up with a hacky solution that might do the trick for your use case.

```

module MyMatchers
  class Pass

    include RSpec::Matchers::Composable

    def initialize(expected_return_value)
      @expected_return_value = expected_return_value
    end

    def description
      "pass with #{@expected_return_value} return value"
    end

    def matches?(block)
      if block.source_location.first.end_with?('compound.rb')
        fail "You should use this matcher at the top of the chain, immediately following the `expect { ... }`"
      end

      @actual_return_value = block.call

      values_match?(@expected_return_value, @actual_return_value)
    end

    def failure_message(*args)
      "Expected: #{@expected_return_value}, got: #{@actual_return_value}"
    end

    def supports_block_expectations?
      true
    end
  end

  def pass(expected_return_value)
    Pass.new(expected_return_value)
  end
end

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

$foo = 0
```

Now, when you run:

```

RSpec.describe "a custom block matcher" do
  specify do
    expect { $foo = 2 }

      .to pass(2)

      .and change { $foo }.by(2)
  end
end

```

it will be green.

However, if `pass` is not the first matcher, it will blow up:

```

RSpec.describe "a custom block matcher" do
  specify do
    expect { $foo = 2 }

      .to change { $foo }.by(2)
      .and pass(2)
  end
end

```

with:

```

a custom block matcher
  is expected to change `$foo` by 2 and pass with 2 return value (FAILED - 1)

Failures:

  1) a custom block matcher is expected to change `$foo` by 2 and pass with 2 return value
     Failure/Error: fail "You should use this matcher at the top of the chain, immediately following the `expect { ... }`"

     RuntimeError:
       You should use this matcher at the top of the chain, immediately following the `expect { ... }`
 ```

I hope that works.

There's room to make this generic by:

 - wrapping the initial expect block into a proc that would save the return value of a wrapped initial block into a variable

 - pass the return value to other matchers along with the block around in NestedEvaluator if matcher's `matches?` accepts two arguments

This would allow for using `pass` in any position in the chain.

I'm still not entirely convinced it will be widely used, since the `expect { expect(action!).to eq(return_value) }.to check_side_effect }` trick works quite fine for non-DSL cases.

- Phil