Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add RuboCop RSpec support #199

Merged
merged 1 commit into from Nov 18, 2020
Merged

Conversation

pirj
Copy link
Contributor

@pirj pirj commented Nov 9, 2020

What is the purpose of this pull request?

Previously, RuboCop RSpec failed to detect RSpec aliases. A number of cops were failing false offences, and some cops were unable to lint, as they were skipping locally defined RSpec aliases taking them for arbitrary blocks and method calls.

See:

Sibling pull requests:

Gemfile:

# frozen_string_literal: true

source 'https://rubygems.org'

gem 'rubocop-rspec'
gem 'test-prof', path: '../../test-prof'

.rubocop.yml:

require:
  - rubocop-rspec

inherit_gem:
  test-prof: .rubocop-rspec-aliases.yml

spec/o_spec.rb:

# frozen_string_literal: true

RSpec.describe 'O' do
  let_it_be(:x) { 'the_one_and_only' }

  # I must come above any other memoized helper! (disclaimer: due to an arguable default linter setting)
  subject(:o) { 'everchanging' } # Not really everchanging with a frozen_string_literal

  it { expect(o).not_to eq("#{x}#{x}") }
end

Before (let_it_be is not detected as a memoized helper)

$ rubocop
Inspecting 2 files
..

2 files inspected, no offenses detected

After (let_it_be is properly detected)

$ rubocop
Inspecting 2 files
.C

Offenses:

spec/o_spec.rb:7:3: C: RSpec/LeadingSubject: Declare subject above any other let_it_be declarations.
  subject(:o) { 'everchanging' } # Not really everchanging with a frozen_string_literal
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

2 files inspected, 1 offense detected, 1 offense auto-correctable

What changes did you make? (overview)

  • added an RSpec alias config file that RuboCop is able to load and append to the default RuboCop RSpec alias configuration.
  • added docs on how to tell RuboCop to use those aliases

Is there anything you'd like reviewers to focus on?

Should I through in some test to make sure the detection works as expected?

I don't have a project that uses let_it_be/before_all handy at the moment.
Do you mind to checking this against a live project?

Checklist

  • [-] I've added tests for this change
  • I've added a Changelog entry
  • I've updated a documentation

@pirj
Copy link
Contributor Author

pirj commented Nov 9, 2020

Liche fail seems unrelated:

 docs/profilers/factory_prof.md
	ERROR	http://www.brendangregg.com/flamegraphs.html
Error: 		Dial tcp4 <nil>->50.63.8.6:80: i/o timeout
Error: Process completed with exit code 1.

Copy link
Collaborator

@palkan palkan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Same here: can we move the config to config/rubocop-rspec.yml?

@pirj
Copy link
Contributor Author

pirj commented Nov 17, 2020

Moved the config and tested again, works.

test-prof.gemspec Outdated Show resolved Hide resolved
Previously, RuboCop RSpec failed to detect RSpec aliases. A number of
cops were failing false offences, and some cops were unable to lint, as
they were skipping locally defined RSpec aliases taking them for
arbitrary blocks and method calls.

See:
 - rubocop/rubocop-rspec#1077
 - rubocop/rubocop-rspec#956

Sibling pull requests:
 - palkan/action_policy#138
@palkan palkan merged commit 8d6b0c7 into test-prof:master Nov 18, 2020
@palkan
Copy link
Collaborator

palkan commented Nov 18, 2020

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants