Skip to content

Enable RSpec/NamedSubject cop rule

Peter Leitzen requested to merge pl-rspec-named-subject into master

What does this MR do and why?

This MR enables 👮 RSpec/NamedSubject to enforce our development guidelines https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#subject-and-let-variables:

  • Avoid referencing subject in examples. Use a named subject subject(:name), or a let variable instead, so the variable has a contextual name.
  • If the subject is never referenced inside examples, then it’s acceptable to define the subject without a name.
# bad
RSpec.describe User do
  subject { described_class.new }

  it 'is valid' do
    expect(subject.valid?).to be(true)
  end
end

# good
RSpec.describe User do
  subject(:user) { described_class.new }

  it 'is valid' do
    expect(user.valid?).to be(true)
  end
end

# also good
RSpec.describe User do
  subject(:user) { described_class.new }

  it { is_expected.to be_valid }
end

RSpec.describe User do
  subject { described_class.new }

  it { is_expected.to be_valid }
end

RSpec.describe User do
  it { is_expected.to be_valid }
end

# accepted in shared examples via `IgnoreSharedExamples: true (default)`
RSpec.shared_examples 'when actor' do
  it 'is valid' do
    expect(subject.valid?).to be(true)
  end
end

Impact on gitlab-org/gitlab

Enabling this 👮 rule on gitlab-org/gitlab results in 32169 files inspected, 26165 offenses detected.

Note that huge amount of offenses is technically not a problem anymore because .rubocop_todo/**/*.yml is used to track the offenses on per-cop basis. Added via gitlab-org/gitlab!72791 (merged) which predates gitlab-org/gitlab!25460 (comment 514552373).

Prior discussions

Scope

Introducing the related 👮 RSpec/ImplicitSubject (see gitlab-org/gitlab!25025 (closed)) is out of scope.

Edited by Peter Leitzen

Merge request reports