Reputation: 317
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
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
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