Monday, November 28, 2022
HomeRuby On RailsCheck which jogged my memory why I do not actually like RSpec

Check which jogged my memory why I do not actually like RSpec


Lately, our pal from a distinct software program firm requested us for some assist with mutant setup. We requested to share a pattern check to find what might be improper. Once I learn the snippet on a slack channel, I had instantly written:

Oh man, this instance jogs my memory why I don’t like RSpec.

Be happy to overview it, it’s written by one among our juniors and I would like to provide him some suggestions quickly. — he responded.

Simply to be clear: I’ve no drawback with the RSpec library itself. It’s an amazing software, I’ve used it in a number of initiatives, however I don’t like how lots of people utilise it. Let’s take a look at it.

Let’s take a look on the authentic instance:

# frozen_string_literal: true

require "rails_helper"

RSpec.describe Api::College students::Replace do
  let(:consumer) { create(:consumer) }
  let(:params) { { last_name: "one thing" } }
  topic(:end result) { described_class.name(current_user: consumer, params: params) }

  context "when consumer is a scholar" do
    it { is_expected.to be_success }
  finish

  context "when consumer is instructor" do
    let(:consumer) { create(:consumer, :instructor) }
    it { is_expected.to be_failure }
  finish

  describe "checking Deal with replace" do
    let(:new_zip_code) { Faker::Deal with.zip_code }
    earlier than { params[:zip_code] = new_zip_code }

    context "when consumer has handle and need to change one thing of their handle" do
      let!(:handle) { create(:handle, proprietor: consumer) }

      it "will succeed" do
        count on(end result.success?).to eq(true)
        count on(handle.reload.zip_code).to eq(new_zip_code)
      finish
    finish

    context "when consumer has not have any handle and need to change one thing of their handle" do
      let(:new_zip_code) { Faker::Deal with.zip_code }
      earlier than { params[:zip_code] = new_zip_code }
      it { is_expected.to be_success }
      it "create handle with given params" do
        count on { end result }.to change(Deal with.all, :depend).from(0).to(1)
        count on(Deal with.final.zip_code).to eq(new_zip_code)
        count on(consumer.handle).to eq(Deal with.final)
      finish
    finish
  finish

  describe "checking dad or mum replace" do
    let(:parent_first_name) { Faker::Title.female_first_name }
    let(:params) { { dad or mum: { first_name: parent_first_name } } }

    context "when consumer has dad or mum and need to change one thing" do
      let(:dad or mum) { create(:dad or mum) }
      let(:consumer) { create(:consumer, occupation: scholar) }
      let(:scholar) { create(:scholar, dad or mum: dad or mum) }

      it { is_expected.to be_success }
      it "will succeed" do
        end result
        count on(dad or mum.reload.first_name).to eq(parent_first_name)
      finish
    finish

    context "when consumer has not have any dad or mum and need to change one thing of their dad or mum" do
      it { is_expected.to be_success }
      it "will create dad or mum with given params" do
        count on { end result }.to change(Mother or father.all, :depend).from(0).to(1)
        count on(Mother or father.final.reload.first_name).to eq(parent_first_name)
        count on(consumer.occupation.dad or mum).to eq(Mother or father.final)
      finish
    finish
  finish
finish

Right here’s an inventory of my ideas after studying check above:

  • RSpec particular syntax sugar to precise check code ratio is just too d*mn excessive
  • Assessments needs to be verbose about their topic, not concerning the plumbing round.
  • I don’t get what the check is about, it’s unreadable due to the nested it in context

hey, however now we have IDEs which might fold the blocks of code…

Cool, however not right here on Slack, nor on GitHub the place you normally make your code critiques. I merely don’t need to soar across the file to see what’s the enter to a service name.

  • Setup is finished by means of FactoryBot which units some synthetic database state, typically not following the enterprise guidelines (if your online business guidelines dwell in ActiveRecord fashions — I’m sorry, we’re previous that since years). It’s higher to make use of area companies to setup the preliminary state. I’ve seen a number of codebases battling gigantic check execution time due to too many database object being created due to how FactoryBot was used.
  • context is barely used to overwrite let, so there’s completely different setup in several examples. Why not maintain the construction flat and do the setup express in each instance? If you happen to want one thing completely different that declared in let, simply use native variable within the instance. let is nice for specifying dependencies and issues that don’t change per every check case.
  • The principle enter to this class giving completely different outcomes are params — this needs to be clearly seen how they differ within the enter and what output they offer, why not go them explicitly to name?
  • Scope the check expectations, making assertion on Mother or father.all gained’t give any assure that the service assigned information to a desired Mother or father object.

Speak is reasonable, so I did a 5 minutes refactoring ensuing on this:

# frozen_string_literal: true

require "rails_helper"

RSpec.describe Api::College students::Replace do
  check "when consumer is a scholar" do
    count on(Api::College students::Replace.name(current_user: consumer, params: { last_name: "one thing" })).to be_success
  finish

  check "when consumer is a instructor" do
    count on(Api::College students::Replace.name(current_user: instructor, params: { last_name: "one thing" })).to be_failure
  finish

  check "when consumer has handle and need to change one thing of their handle will succeed" do
    handle = create(:handle, proprietor: consumer)

    end result = Api::College students::Replace.name(current_user: consumer, params: { last_name: "one thing", zip_code: new_zip_code })

    count on(end result.success?).to eq(true)
    count on(handle.reload.zip_code).to eq(new_zip_code)
  finish

  check "when consumer has not have any handle and need to change one thing of their handle create handle with given params" do
    end result =
      count on {
        Api::College students::Replace.name(current_user: consumer, params: { last_name: "one thing", zip_code: new_zip_code })
      }.to change(Deal with.all, :depend).from(0).to(1)

    count on(end result.success?).to eq(true)
    count on(Deal with.final.zip_code).to eq(new_zip_code)
    count on(consumer.handle).to eq(Deal with.final)
  finish

  check "when consumer has dad or mum and need to change one thing" do
    consumer = create(:consumer, occupation: scholar)

    end result = Api::College students::Replace.name(current_user: consumer, params: { dad or mum: { first_name: parent_first_name } })

    count on(end result).to be_success
    count on(dad or mum.reload.first_name).to eq(parent_first_name)
  finish

  check "when consumer has not have any dad or mum and need to change one thing of their dad or mum" do
    consumer = create(:consumer, occupation: scholar)

    end result =
      count on {
        Api::College students::Replace.name(current_user: consumer, params: { dad or mum: { first_name: parent_first_name } })
      }.to change(Mother or father.all, :depend).from(0).to(1)

    count on(end result).to be_success
    count on(Mother or father.final.reload.first_name).to eq(parent_first_name)
    count on(consumer.occupation.dad or mum).to eq(Mother or father.final)
  finish

  let(:consumer) { create(:consumer) }
  let(:instructor) { create(:consumer, :instructor) }
  let(:scholar) { create(:scholar, dad or mum: dad or mum) }
  let(:dad or mum) { create(:dad or mum) }
  let(:new_zip_code) { Faker::Deal with.zip_code }
  let(:parent_first_name) { Pretend::Title.female_first_name }
finish

What are your ideas? Which model is extra readable to you?



RELATED ARTICLES

LEAVE A REPLY

Please enter your comment!
Please enter your name here

Most Popular

Recent Comments