jish / pre-commit

A slightly improved pre-commit hook for git
https://jish.github.io/pre-commit/
Other
796 stars 96 forks source link

Support detection of :focus within specs when using newer hash syntax #167

Closed pacso closed 10 years ago

pacso commented 10 years ago

With the new hash syntax you can apply :focus within a spec without using a hashrocket.

For example:

RSpec.describe Conversation, type: :model, focus: true do

Currently, this is not detected by the rspec_focus pre-commit plugin, and will be committed without error.

Please update the rspec_focus plugin to cover the newer hash syntax.

jish commented 10 years ago

Hi, I'm not too familiar with this RSpec option, could you give me an example of a "correct" focus option, and an example of an "incorrect" focus option?

Thanks! :)

pacso commented 10 years ago

Well, an "incorrect" focus option wont work ... so I don't think it's anything to worry about checking for.

Working options are as follows though.

# standard hash rocket syntax
describe SomeModel, :focus => true do 

# JSON style hash notation introduced in ruby 1.9
describe SomeModel, focus: true do 

# acceptable format if RSpec is configured to treat_symbols_as_metadata_keys_with_true_values
describe SomeModel, :focus do 

The first and last case are already covered by your existing check. The second example however is not and will get past the pre-commit hook checks. It's worth noting that inclusion of the ruby_symbol_hashrockets plugin will ensure everybody uses this undetected format.

pacso commented 10 years ago

Additionally, the various focus options can be included in multiple parts of a spec and be 'bad', but could be included within the code block of a spec and be 'good'.

For example, some 'bad' specs:

# focus on describe block
describe SomeModel, :focus => true do

# focus on context block
context 'user logged in', focus: true do

# focus on example block
it 'does something', :focus do

Yet, if you used focus within a spec, that should be considered 'good'. The following spec should not trigger any focus issues:

describe FocusModel do
  it 'should not trigger the pre-commit focus detector with old hash syntax' do
    expect(FocusModel.new(:focus => true)).to be_valid
  end

  it 'should not trigger the pre-commit focus detector with new hash syntax' do
    expect(FocusModel.new(focus: true)).to be_valid
  end

  it 'should not trigger the pre-commit focus detector with a has key' do
    focus = FocusModel.new
    expect(focus.value_for[:focus]).to eq 'undetected'
  end
end
pacso commented 10 years ago

Pull request https://github.com/jish/pre-commit/pull/169 resolves this issue

mpapis commented 10 years ago

closing in favor of #169