Add a cop to check for consistent method usage in feature specs. #439

rspeicher · 2017-08-05T18:24:32Z

No description provided.

rspeicher · 2017-08-05T18:37:47Z

I started down the path of making this ConfigurableEnforcedStyle, with rspec and capybara options.

If someone were to set capybara, we'd have to make sure to only check:

Files in spec/features
Files with type: :feature (and possibly just :feature?) metadata

I wasn't sure of the best way to handle those two conditions, so I'm open to feedback on making this configurable.

backus · 2017-08-05T19:39:42Z

lib/rubocop/cop/rspec/feature_methods.rb

+      class FeatureMethods < Cop
+        MSG = 'Use `%s` instead of `%s`.'.freeze
+
+        # https://git.io/v7rLu


If you hit y on your keyboard when visiting this link then github will give you a link to a sha that will never break. https://help.github.com/articles/getting-permanent-links-to-files/

Sure, updated.

backus

Nice!

bquorning · 2017-08-21T07:40:14Z

I’ve been trying to find out why capybara provides these aliased methods. The README just says that “Capybara also comes with a built in DSL for creating descriptive acceptance tests […] feature is in fact just an alias for describe ..., :type => :feature, background is an alias for before, scenario for it, and given/given! aliases for let/let!, respectively.” without any explanation for why this DSL should be preferable.

Also, I just want to note that these aliases are only available if Capybara (or rspec-rails) is loaded. Which puts this cop in the same category as RSpec::FactoryGirl::DynamicAttributeDefinedStatically as “not really an RSpec cop, but close”. Perhaps we should have an RSpec::Capybara module?

rspeicher · 2017-08-21T14:48:31Z

@bquorning I believe they're just syntactic sugar to make feature specs read more like Cucumber features.

Darhazer · 2017-08-21T15:14:23Z

Yup, this should be in RSpec::Capybara module. There are few other things we could implement for capybara (like #378 for example)

rspeicher · 2017-08-22T17:21:35Z

Moved the cop to a new Capybara namespace.

rspeicher · 2017-08-22T17:23:05Z

spec/project/default_config_spec.rb

      'factory_girl' => 'FactoryGirl'
    }
    glob = SpecHelper::ROOT.join('lib', 'rubocop', 'cop',
-                                 'rspec', '{,factory_girl/}*.rb')
+                                 'rspec', '{capybara/,,factory_girl/}*.rb')


Requiring these two changes might get messy quickly if you anticipate adding more namespaces, but I went with the simplest thing for now.

rspeicher · 2017-08-23T15:05:37Z

Can't see what the build failures are since they're private.

Darhazer · 2017-08-24T12:05:01Z

@rspeicher you can run rake on your local machine to check the build. I think you should add Capybara to the namespaces in ConfigFormatter and fix the build_config script to include capybara

-cops = '{*.rb,factory_girl/*.rb}'
+cops = '{*.rb,factory_girl/*.rb,capybara/*.rb}'

bquorning · 2017-08-24T12:43:33Z

Ahh, I think you need to add ,capybara/*.rb to bin/build_config for the confirm_config test to pass.

rspeicher · 2017-08-24T14:14:03Z

Back to green, thanks y'all!

rspeicher · 2017-08-22T17:42:38Z

config/default.yml

+Capybara/FeatureMethods:
+  Description: Checks for consistent method usage in feature specs.
+  Enabled: true
+  StyleGuide: http://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/Capybara/FeatureMethods


I think this link and the one below it are wrong -- the namespace should be RuboCop/Cop/RSpec/{Capybara,FactoryGirl}, no?

rspeicher · 2017-08-24T14:04:21Z

bin/build_config

@@ -8,7 +8,7 @@ require 'rubocop-rspec'
 require 'rubocop/rspec/description_extractor'
 require 'rubocop/rspec/config_formatter'

-cops = '{*.rb,factory_girl/*.rb}'
+cops = '{*.rb,capybara/*.rb,factory_girl/*.rb}'


This could be simplified to just **/*.rb.

[ci skip]

rspeicher · 2017-09-07T16:20:32Z

Rebased since 1.16.0 was released.

All cops need to be nested under `RuboCop/Cop/RSpec`, but cops nested under `Capybara` and `FactoryGirl` were missing the `RSpec` part.

rspeicher · 2017-09-07T16:35:58Z

lib/rubocop/rspec/config_formatter.rb

@@ -22,7 +22,7 @@ def unified_config
        cops.each_with_object(config.dup) do |cop, unified|
          unified[cop] = config.fetch(cop)
            .merge(descriptions.fetch(cop))
-            .merge('StyleGuide' => STYLE_GUIDE_BASE_URL + cop)
+            .merge('StyleGuide' => STYLE_GUIDE_BASE_URL + cop.sub('RSpec/', ''))


Would you prefer an anchored regex here?

Darhazer · 2017-09-08T06:51:09Z

I think it's ready for merge. @backus final words?

backus · 2017-09-08T07:02:02Z

LGTM. @rspeicher or @Darhazer (whoever does the merge) can you squash down the commits?

rspeicher force-pushed the rs-feature_methods branch from 998bc6d to 1208b18 Compare August 5, 2017 18:25

rspeicher changed the title ~~WIP: Add a cop to check for consistent method usage in feature specs.~~ Add a cop to check for consistent method usage in feature specs. Aug 5, 2017

rspeicher force-pushed the rs-feature_methods branch from 1208b18 to a443b44 Compare August 5, 2017 18:32

backus reviewed Aug 5, 2017

View reviewed changes

rspeicher force-pushed the rs-feature_methods branch from a443b44 to 92d251c Compare August 7, 2017 16:10

backus approved these changes Aug 20, 2017

View reviewed changes

rspeicher force-pushed the rs-feature_methods branch from 92d251c to 80e3a73 Compare August 22, 2017 17:21

rspeicher commented Aug 22, 2017

View reviewed changes

rspeicher commented Aug 24, 2017

View reviewed changes

rspeicher added 4 commits September 7, 2017 12:15

Add a cop to check for consistent method usage in feature specs.

7ad2e46

Move RSpec/FeatureMethods to Capybara/FeatureMethods

630deba

Update the changelog to note RSpec/Capybara namespace

827d05c

[ci skip]

Add Capybara namespace to bin/build_config and ConfigFormatter

396c94f

rspeicher force-pushed the rs-feature_methods branch from aa8a130 to 00337e3 Compare September 7, 2017 16:18

Correct generated StyleGuide links for namespaced cops

a39ce8b

All cops need to be nested under `RuboCop/Cop/RSpec`, but cops nested under `Capybara` and `FactoryGirl` were missing the `RSpec` part.

rspeicher force-pushed the rs-feature_methods branch from 00337e3 to a39ce8b Compare September 7, 2017 16:33

rspeicher commented Sep 7, 2017

View reviewed changes

Darhazer approved these changes Sep 8, 2017

View reviewed changes

Darhazer merged commit 7fe393a into rubocop:master Sep 8, 2017

bquorning mentioned this pull request Oct 5, 2018

Remove unused modules #700

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a cop to check for consistent method usage in feature specs. #439

Add a cop to check for consistent method usage in feature specs. #439

rspeicher commented Aug 5, 2017

rspeicher commented Aug 5, 2017

backus Aug 5, 2017

rspeicher Aug 7, 2017

backus left a comment

bquorning commented Aug 21, 2017

rspeicher commented Aug 21, 2017

Darhazer commented Aug 21, 2017

rspeicher commented Aug 22, 2017 •

edited

Loading

rspeicher Aug 22, 2017

rspeicher commented Aug 23, 2017

Darhazer commented Aug 24, 2017

bquorning commented Aug 24, 2017

rspeicher commented Aug 24, 2017

rspeicher Aug 22, 2017

rspeicher Aug 24, 2017

rspeicher commented Sep 7, 2017

rspeicher Sep 7, 2017

Darhazer commented Sep 8, 2017

backus commented Sep 8, 2017

Add a cop to check for consistent method usage in feature specs. #439

Add a cop to check for consistent method usage in feature specs. #439

Conversation

rspeicher commented Aug 5, 2017

rspeicher commented Aug 5, 2017

backus Aug 5, 2017

Choose a reason for hiding this comment

rspeicher Aug 7, 2017

Choose a reason for hiding this comment

backus left a comment

Choose a reason for hiding this comment

bquorning commented Aug 21, 2017

rspeicher commented Aug 21, 2017

Darhazer commented Aug 21, 2017

rspeicher commented Aug 22, 2017 • edited Loading

rspeicher Aug 22, 2017

Choose a reason for hiding this comment

rspeicher commented Aug 23, 2017

Darhazer commented Aug 24, 2017

bquorning commented Aug 24, 2017

rspeicher commented Aug 24, 2017

rspeicher Aug 22, 2017

Choose a reason for hiding this comment

rspeicher Aug 24, 2017

Choose a reason for hiding this comment

rspeicher commented Sep 7, 2017

rspeicher Sep 7, 2017

Choose a reason for hiding this comment

Darhazer commented Sep 8, 2017

backus commented Sep 8, 2017

rspeicher commented Aug 22, 2017 •

edited

Loading