Heredoc and expect to change causing `SyntaxError: compile error`

[KNejad]

Subject of the issue

A strange combination of a heredoc in a before block and an expect {}.not_to change {} is causing a SyntaxError.

Your environment

• Ruby version: 2.7
• rspec-core version: 3.9.2

Steps to reproduce

Run the following spec:

RSpec.describe Array do
  before do
    allow(File).to receive(:read).and_return(<<~CSV)
      1,2
    CSV
  end

  it 'fails' do
    expect { 32 }.not_to(change { rand })
  end
end

It will fail as it should, but it will not show the correct failure reason

Expected behavior

I expect an output like the following:

  1) Array fails
     Failure/Error: expect { 32 }.not_to(change { rand })
       expected `rand` not to have changed, but did change from 0.32202744992924015 to 0.686499723014205

Actual behavior

I get this

1) Array fails
     Failure/Error: Unable to find matching line in /home/keeyan/projects/test_rspec/failing_spec.rb

     SyntaxError:
       compile error

When I change rand to a static value (e.g. 0) the test passes as it should. It's only when the test fails that I get an issue. It's definitely not a Ruby syntax error since it is valid code.

If the heredoc is not the argument of the allow(File).to receive methods, it works fine.

If I replace the test with a simple expect(12).to eq(13) it works as expected.

It's only this specific combination of allow, Heredoc and expect... to change which is causing this issue.

[JonRowe]

:wave: This is weird, its actually an issue with our source extraction code (that parses specs to produce nice looking failures), specifically we expect Ripper to be able to parse all valid Ruby and give us a valid ast to produce snippets for failure messages.

It looks like for some reason this blows up with this specific combination, but I have no idea why...

@penelopezone, as our most famous haunted Ruby alum can you shed any light on this? If I remember rightly you had difficulty with heredocs for rubyfmt?

[pirj]

It's interesting as you mention that it's this specific combination that triggers the error.

Doesn't seem that HEREDOC parsing is to blame, I've reproduced it with a string literal as well. Dissecting it further reveals the following difference between those two expectations:

    expect { true }.to change(false)
# vs
    expect { true }.to change { false }

RSpec::Support::Source#source is the whole spec in the former case

RSpec.describe Array do
  before do
    allow(File).to receive(:read).and_return("1, 2")
  end

  it 'fails' do
    expect { true }.to change(false)
  end
end

and it's just the string "1, 2" in the latter.

I'll dig it deeper.

[KNejad]

Oh you are right, sorry about that red herring, I must have changed the string content when I tried with a simple string.

Could it be because I am changing the behaviour of File.read? Maybe when Ripper is trying to parse the file, it uses File.read but instead of getting the actual file back it is getting the string I am stubbing it with which isn't valid ruby?

That would explain why you get a syntax error when it's 1, 2 which isn't valid Ruby, but if I put foo which is valid Ruby I just get Failure/Error: Unable to find matching line in /home/keeyan/failing_spec.rb without the syntax error.

EDIT:

I think that must be the case since the following works as expected:

RSpec.describe Array do
  before do
    allow(File).to receive(:read).and_return(File.open('failing_spec.rb').read)
  end

  it 'fails' do
    expect { true }.to change { false }
  end
end

[JonRowe]

It could well be the stubbing of File is interfering with Ripper... can't believe I didn't think of that 🤦

[pirj]

It's this line that gets affected by the stub.

Well done research, @KNejad!

I guess if you add a qualifier for the passed argument, it won't be affecting rspec-support's internals:

allow(File).to receive(:read).with('filename.csv')

[KNejad]

Thanks for the help guys, would you say this is a bug with RSpec, since I wouldn't expect the RSpec internals to care about the stubs like this, or should it just be considered user error and simply close this issue?

[pirj]

RSpec is used to test RSpec, and it's no exception to stub some Ruby core/stdlib methods, and e.g.:

spec/rspec/core/configuration_spec.rb|704| 24:        allow(File).to receive(:directory?) do |path|
spec/rspec/core/configuration_spec.rb|708| 23:        allow(Dir).to receive(:[]).and_return([])
spec/rspec/core/metadata_filter_spec.rb|20| 27:          allow(RSpec).to receive(:world) { world }

If we would run unstubbed ones, some code would become barely testable.

So I believe it is expected behaviour.

Thanks for reporting, that was a very captivating puzzle!

[KNejad]

Makes sense, thanks again for the help @pirj

[JonRowe]

If you add a conditional qualifier here, and add a fallback and_call_original the problem also goes away.

RSpec.describe do
  before do
    allow(File).to receive(:read).and_call_original
    allow(File).to receive(:read).with('my_file.txt').and_return(<<~CSV)
      1,2
    CSV
  end

  it 'fails' do
    expect { 32 }.not_to(change { rand })
  end
end