Shania
Shania

Reputation: 317

Rubocop RSpec/MultipleMemoizedHelpers issue on pundit spec tests

I use pundit for authorization and RSpec for testing in my rails app. Due to this, I had to create specs for the policies.

However, I am having a problem with rubocop throwing an error: RSpec/MultipleMemoizedHelpers. I understand that this means I have too many let and subject calls. My issue is I'm not quite sure how to resolve or refactor my code so it adheres to the proper number of calls I should make.

Another thing, is it okay to disable RSpec/MultipleMemoizedHelpers for spec files?

Here are three of the policy spec files that are an issue.

require "rails_helper"

describe AnswerPolicy do
  subject { described_class }

  let(:user_admin) { build(:user, :admin) }
  let(:consultant) { build(:consultant) }
  let(:user_consultant) { build(:user, :consultant, consultant: consultant) }
  let(:client) { build(:client, consultant: consultant) }
  let(:user_client) { build(:user, :client, client: client) }
  let(:other_client) { build(:client, consultant: build(:consultant)) }
  let(:answer) { build(:answer, client: client) }
  let(:other_answer) { build(:answer, client: other_client) }

  permissions :update? do
    it "allows access to admin" do
      expect(described_class).to permit(user_admin)
    end

    it "prevents consultants to update other non-client answers" do
      expect(described_class).not_to permit(user_consultant, other_answer)
    end

    it "prevents clients to update their answers" do
      expect(described_class).not_to permit(user_client, answer)
    end

    it "allows consultants to update their client's answers" do
      expect(described_class).to permit(user_consultant, answer)
    end
  end
end
describe AssessmentStepPolicy do
  subject { described_class }

  let(:user_admin) { build(:user, :admin) }
  let(:consultant) { build(:consultant) }
  let(:user_consultant) { build(:user, :consultant, consultant: consultant) }
  let(:client) { build(:client, consultant: consultant) }
  let(:user_client) { build(:user, :client, client: client) }
  let(:other_client) { build(:client, consultant: build(:consultant)) }

  permissions :view? do
    it "allows access to admin" do
      expect(described_class).to permit(user_admin)
    end

    it "prevents consultants to view other non-client assessment details" do
      expect(described_class).not_to permit(user_consultant, other_client)
    end

    it "allows clients to view their assessment details" do
      expect(described_class).to permit(user_client, client)
    end

    it "prevents clients to view other client's assessment details" do
      expect(described_class).not_to permit(user_client, other_client)
    end

    it "allows consultants to view their client's answers" do
      expect(described_class).to permit(user_consultant, client)
    end
  end

  permissions :create? do
    it "allows access to any admin" do
      expect(described_class).to permit(user_admin)
    end

    it "prevents consultants to assess other clients" do
      expect(described_class).not_to permit(user_consultant, other_client)
    end

    it "prevents clients to assess themselves" do
      expect(described_class).not_to permit(user_client, client)
    end

    it "allows consultants to assess their clients" do
      expect(described_class).to permit(user_consultant, client)
    end
  end
end
require "rails_helper"

describe ReportPolicy do
  subject { described_class }

  let(:user_admin) { build(:user, :admin) }
  let(:consultant) { build(:consultant) }
  let(:user_consultant) { build(:user, :consultant, consultant: consultant) }
  let(:client) { build(:client, consultant: consultant) }
  let(:user_client) { build(:user, :client, client: client) }
  let(:other_consultant) { build(:consultant) }
  let(:other_client) { build(:client, consultant: other_consultant) }

  permissions :dashboard? do
    it "allows access to admin" do
      expect(described_class).to permit(user_admin)
    end

    it "prevents clients to view other client dashboards" do
      expect(described_class).not_to permit(user_client, other_client)
    end

    it "prevents consultants to view other non-client dashboards" do
      expect(described_class).not_to permit(user_consultant, other_client)
    end

    it "allows clients to view their dashboard" do
      expect(described_class).to permit(user_client, client)
    end

    it "allows consultants to view their client's dashboards" do
      expect(described_class).to permit(user_consultant, client)
    end
  end
end

Upvotes: 13

Views: 7496

Answers (2)

Marc-André Lafortune
Marc-André Lafortune

Reputation: 79562

This RSpec/MultipleMemoizedHelpers cop is controversial. It wants you to limit the number of let to an arbitrary number.

I fought hard against it. To me it is similar to a cop that would raise an offense because you have too many variables. rubocop and rubocop-ast have it disabled, while we typically enable more cops than the default. Note that you could change these let to def and the offenses go away (although you haven't changed anything; let is just syntax sugar for a def).

Sharing your factories seems like a good idea, and I recommend disabling the cop too.

Upvotes: 29

lobati
lobati

Reputation: 10215

So I think I can take some amount of credit for starting this debate. I'm a big fan of not using let and subject blocks. They add a lot of indirection in tests that can make it hard to read and extend tests as they get larger. You can read more of my thoughts in the linked thread and its offshoots. Rather than using let, I would start by simply in-lining the dependencies inside the test block:

it "prevents consultants to view other non-client assessment details" do
  consultant = build(:consultant)
  user = build(:user, :consultant, consultant: consultant)
  client = build(:client, consultant: build(:consultant))

  expect(described_class).not_to permit(user, client)
end

That is a bit wordy, but I don't mind some duplication in my tests. My general rule of thumb is that if it's <= 5 lines, then I don't stress too much about it. But still, I might want to pull some of the setup into factories:

it "prevents consultants to view other non-client assessment details" do
  user = build(:user, :consultant)
  client = build(:client, :with_consultant)

  expect(described_class).not_to permit(user, client)
end

The nice thing about this is that it doesn't require you to keep track of the existing tests when writing new ones. It's easier to write variations without risking breaking the let blocks working for all of the other tests.

Upvotes: 0

Related Questions