How can I DRY up this RSpec code?

rubyonrails-talk
1

Here are my RSpec examples:

  describe '::import_platforms' do
    let(:header_row) { ['ID', 'PlatformName'] }
    let(:platform_1_row) { [50, 'Android'] }
    let(:platform_rows) { [header_row, platform_1_row] }

    after do
      described_class.import_platforms
    end

    it 'reads platforms CSV file' do
      expect(described_class).to receive(:read_csv_file).with(described_class::PLATFORMS_CSV_FILE).and_return(platform_rows)
    end

    it 'calls ::create_platform per CSV row' do
      expect(described_class).to receive(:read_csv_file).with(described_class::PLATFORMS_CSV_FILE).and_return(platform_rows)
      expect(described_class).to receive(:create_platform).once.with(platform_1_row)
    end
  end

The line:

expect(described_class).to receive(:read_csv_file).with(described_class::PLATFORMS_CSV_FILE).and_return(platform_rows)

is repeated in both examples.

For the first example, I really just want to make sure the method is called, so it is an “expectation” in that sense.

For the second example, I mostly just want it to return the dummy data. I’m not sure what the appropriate term is in this case. A mock? A stub?

So I have this expect(...) line which I’m essentially using for two purposes, to test that something happened, and also to feed test data into the method being tested.

I’m wondering how I might DRY this up.

1 Like
2

OPTION 1: To DRY these up into one thing you can add this let() at the top:

let(:read_csv) do
  -> { expect(described_class).to receive(:read_csv_file).with(described_class::PLATFORMS_CSV_FILE).and_return(platform_rows) }
end

and then in place of those lines in the it blocks, call that lambda:

read_csv.call

OPTION 2: To have the second example use a stub, change that line to use allow() instead of expect():

allow(described_class).to receive(:read_csv_file).with(described_class::PLATFORMS_CSV_FILE).and_return(platform_rows)

OPTION 3: Putting it all together – which might be more confusing than it’s worth! But anyway, to DRY up what you have after implementing option 2, you can use this let() at the top:

let(:read_csv) do |have_expect_or_allow|
  -> { have_expect_or_allow.to receive(:read_csv_file).with(described_class::PLATFORMS_CSV_FILE).and_return(platform_rows) }
end

And then for the first example have this:

read_csv.call(expect(described_class))

And for the second one, this:

read_csv.call(allow(described_class))
1 Like
3

Great question! This kind of thing comes up often.

If this is the only instance of the duplication, I honestly wouldn’t bother. I generally teach that the tolerance for duplication is a lot higher in RSpec descriptions than in production software, because it often improves readability and the RSpec descriptions/examples are less likely to change in the future. I use the following criteria to decide whether the duplication is acceptable or not. (Not all have to apply.)

  1. it is limited in scope and/or use.
  2. it does not decrease readability, or the intent of the interface it’s describing.
  3. it will not need to change due to external factors that are not covered by the test in question.

In your example, all three apply. So I would leave it. 1. The duplication is only two lines, in a single file. 2. The duplication clearly describes the intent of the thing you’re describing. 3. It is unlikely to need to change due to an external change elsewhere in the codebase.

If, however, your example above is complete, I might put the expectation in a before block, which will cause it to run in both places. Alternatively, put the expectation in a helper method. Another example here suggests using let for this, but I tend to use helper methods because it improves readability (namely by removing the use of #call.)

If you have more of these kinds of questions, you can certainly contact me. I offer up to three free 30-min pairing/consulting sessions on these kinds of things to folks who want to learn how to describe their software with RSpec more effectively. (There is absolutely no obligation to purchase anything in the future, I do this to give back to the community that has been great to me.)

(I am the creator of RSpec.)

Cheers!

4 Likes
4

I agree with @srbaker (ps. thanks for rspec!) that these examples seem dry-enough to me and I wouldn’t bother changing them unless there was a lot more duplication. When I find myself writing lots of similar tests, my goto pattern is to create a helper module for custom expectations. For instance, if you had a lot of csv related tests you might do something like:

module RSpecCSVHelpers
  def expect_to_read_csv(rows)
    expect(described_class).to receive(:read_csv_file).with(described_class::PLATFORMS_CSV_FILE).and_return(rows) 	
    #  other grouped expectations ...
  end
end

and then the tests become:

it 'calls ::create_platform per CSV row' do
  expect_to_read_csv platform_rows
  expect(described_class).to receive(:create_platform).once.with(platform_1_row)
end

If you include the module with RSpec.configure the methods are available to all your tests:

RSpec.configure do |c|
  c.include RSpecCSVHelpers
end
3 Likes
5

The mixin is a great solution.

I’ll add that I wouldn’t go down this road until you needed that helper in more than one file. I work hard to avoid optimising for re-use too early, because it’s difficult to predict what the boundaries are until they’re staring you in the face.

Cheers!

1 Like