rspec / rspec-expectations

Provides a readable API to express expected outcomes of a code example
https://rspec.info
MIT License
1.26k stars 397 forks source link

let(:output) definition called when using `output.to_stdout` expectation #1213

Closed ronocod closed 4 years ago

ronocod commented 4 years ago

Subject of the issue

When we use RSpec.configure and config.around to check the output of each test, any let(:output) definition in a test is called before the test is executed.

Your environment

Steps to reproduce

This file shows the issue:

# frozen_string_literal: true

begin
  require "bundler/inline"
rescue LoadError => e
  warn "Bundler version 1.10 or later is required. Please update your Bundler"
  raise e
end

gemfile(true) do
  source "https://rubygems.org"

  gem "rspec", "3.9.0" # Activate the gem and version you are reporting the issue against.
end

puts "Ruby version is: #{RUBY_VERSION}"
require "rspec/autorun"

RSpec.configure do |config|
  config.around do |example|
    expect(&example).to output.to_stdout
  end
end

RSpec.describe "additions" do
  let(:output) { fail "This should not run" }

  it "returns 2" do
    expect(1 + 1).to eq(2)
  end

  it "returns 1" do
    expect(3 - 1).to eq(-1)
  end
end

Expected behavior

The output let definition is not run as the test itself does not call it.

Actual behavior

The output let definition is called before the test runs.

pirj commented 4 years ago

You are using output in around. The problem I see here is that the let definition shadows the output matcher, and there is no warning. The same applies pretty much to every matcher, built-in or custom.

Would you like to add such a warning? You can start in https://github.com/rspec/rspec-core/blob/d9e7d3ddaea72a8e54961a5304607790ddb26ec7/lib/rspec/core/memoized_helpers.rb#L298, and check if it's possible to distinguish a let from a matcher. I'd start with an an arity check, most matchers accept arguments, while memoized helpers don't.

Please let me know if you need any additional information to start hacking.

JonRowe commented 4 years ago

All hooks, before / after / around are part of the example. This is the expected behaviour, you have defined output and called it from within the lifecycle of the test.

ronocod commented 4 years ago

In case anyone else runs into this, the workaround I've started using is to add this unless section at the top of your around block:

RSpec.configure do |config|
  config.around do |example|
    unless output.is_a?(RSpec::Matchers::BuiltIn::Output)
      raise "A spec is overriding the 'output' field, which breaks assertions on STDOUT output"
    end
    expect(&example).to output.to_stdout
  end
end

This will give you a clearer error explaining what the issue is.