Improving spec clarity though with block arguments.

better_spec_through_block_arguments.markdown

Better Specs Through Block Arguments

Me, fixing my specs c.333 BC

let considered harmful

If not used carefully, the powerful lazy-let construct in Rspec can quickly turn your specs into a spaghetti monster throwing Gordian knots.

When used correctly, let is the best thing ever.

In a previous gist, I discussed the FieldHand pattern as a set of rules to keep the lets from getting out of pocket. Here we look at another pattern for clean specs.

---

A non-trivial full-stack request spec

# spec/requests/a_real_world_spec.rb

describe "creating a new drawing issue notice" do

  before do
    post "/drawing_issue_notices", drawing_issue_notice: din_attrs
  end

  describe "email side effects" do

    it "sent an email to the PIC" do
      expect(emails_to(pic).size).to eq(1)
    end

  end

  # Interesting for this spec
  let(:pic)            { din_engineer }


  # The rest of this is not really interesting for this spec
  let(:dc_number)      { SecureRandom.uuid }
  let(:document_attrs) { support_document_attributes }

  let(:din_attrs) do
    support_drawing_issue_notice_attributes
    .merge(
      design_change_number: dc_number,
      primary_platform_id: platform.id,
    )
    .merge(documents_attributes: [document_attrs])
  end

  let(:platform) do
    din_platform { |p| p.primary_engineer_id = pic.id }
  end

end

About half of that spec is a bunch of un-intersting lets. This is usually a sign that your system is too tightly coupled. In this case we are testing that the entire system works togeather for a feature that lets the user upload files for a one-to-many nested resource, and sends some emails. The spec complexity is unavoidable.

But we can clean up the specs, by stashing those boring lets someplace else:

# spec/support/context_drawing_issue_notice.rb

module ContextDrawingIssueNotice

  # `shared_context` works similar to `shared_examples_for`
  shared_context "drawing_issue_notice" do
    let(:filename)       { SecureRandom.uuid }
    let(:pic)            { din_engineer }
    let(:dc_number)      { SecureRandom.uuid }
    let(:document_attrs) { support_document_attributes }

    let(:din_attrs) do
      support_drawing_issue_notice_attributes
      .merge(
        design_change_number: dc_number,
        primary_platform_id: platform.id,
      )
      .merge(documents_attributes: [document_attrs])
    end

    let(:platform) do
      din_platform { |p| p.primary_engineer_id = pic.id }
    end
  end
  
end

# spec/requests/a_real_world_spec.rb

describe "creating a new drawing issue notice" do
  include ContextDrawingIssueNotice

  before do
    post "/drawing_issue_notices", drawing_issue_notice: din_attrs
  end

  describe "email side effects" do

    it "sent an email to the PIC" do
      expect(emails_to(pic).size).to eq(1)
    end

  end

  # Interesting for this spec
  let(:pic) { din_engineer }

  include_context "drawing_issue_notice"

end

There, that's much more intention revealing. Now that we have that boilerplate extracted to an included context, we can use it in another test. Here we look at the test that confirms that the uploaded files are processed:

# spec/requests/a_real_world_spec_processing_files.rb

describe "creating a new drawing issue notice" do

  before do
    post "/drawing_issue_notices", drawing_issue_notice: din_attrs
  end

  describe "the file side effects" do

    it "moves the input file to it's home" do
      final_filepath = Settings.processed_file_dir.join(filename)
      expect(File.exist? final_filepath).to be_true
    end

  end

  # Interesting for this spec
  let(:filename)       { SecureRandom.uuid }

  include_context "drawing_issue_notice"

  # this spec needs to know about the filename, so we override
  # the `document_attrs` `let` that is defined in our included context.  
  # THIS IS NOT GOOD :-(
  #
  let(:document_attrs) do
    support_document_attributes
    .merge(filename: filename)
  end

end

Overriding the document_attrs let is a really bad idea. From this file, it's not obvious that it's overriding something. Becuase document_attrs is not directly referenced in this file, it looks like it can be deleted! Here we fix it:

# spec/requests/a_real_world_spec_processing_files.rb

describe "creating a new drawing issue notice" do

  before do
    post "/drawing_issue_notices", drawing_issue_notice: din_attrs
  end

  describe "the file side effects" do

    it "moves the input file to it's home" do
      final_filepath = Settings.processed_file_dir.join(filename)
      expect(File.exist? final_filepath).to be_true
    end

  end

  # Interesting for this spec
  let(:filename)       { SecureRandom.uuid }

  # `include_context` takes a block!
  include_context "drawing_issue_notice" do
    let(:document_attrs) do
      support_document_attributes
      .merge(filename: filename)
    end
  end

end

Using the block argument to include_context we can make it clear that document_attrs is dependent on something the "drawing_issue_notice" context is doing. This is much better

As of this writing the block argument isn't documented, but I've put in a pull request to document it.

-- Mark!