RSpec aliases runtime matchers #956

sl4vr · 2020-07-07T18:25:58Z

This is an attempt of implementation of issue #333 and evolution of PR #921. Allowing to configure 3rd partly DSLs as valid RSpec syntax.

Configurable language elements:

Before submitting the PR make sure the following are checked:

Feature branch is up-to-date with master (if not - rebase it).
Squashed related commits together.
Added tests.
Updated documentation. (Add v2 update docs #1013)
Added an entry to the CHANGELOG.md if the new code introduces user-observable changes.
The build (bundle exec rake) passes (be sure to run this locally, since it may produce updated documentation that you will need to commit).

sl4vr · 2020-07-08T08:24:30Z

~~Looks like smoke tests fail, need to rewrite RuntimeMacros in a same manner like RuboCop's Macros are made.~~
Seems that smoke tests fail due to another reason, which should be fixed here: #955

lib/rubocop/rspec/language/node_pattern.rb

lib/rubocop/cop/rspec/named_subject.rb

pirj

Frankly, it looks great. All code review notes are minor nitpicks.

I'm concerned about two major things:

performance impact: dynamic way to define matchers may cost us
consistency: are we going to use this approach for all RSpec's DSL constructs

If there's no significant performance impact (e.g. on Discourse/GitLab specs suite runs), I'll be more than happy to merge this. We'll eventually catch up and rework other DSL elements to use this dynamic configuration.

lib/rubocop/rspec/example_group.rb

lib/rubocop/rspec/language/runtime_macros.rb

lib/rubocop/cop/rspec/described_class.rb

lib/rubocop/cop/rspec/empty_hook.rb

lib/rubocop/rspec/language.rb

lib/rubocop/cop/rspec/cop.rb

pirj · 2020-08-02T14:55:19Z

@sl4vr Do you need some help with this?
Highly appreciate if you could at least come up with a PoC, I can take it from there if you like, or we can share the work if you like.

pirj

Looks good! 👏
Minor notes, mostly on how to keep the existing cops intact.
Eager to rework the rest in the same way and start hacking on adding the missing DSL pieces.

lib/rubocop/cop/rspec/dialect.rb

lib/rubocop/cop/rspec/hook_argument.rb

lib/rubocop/cop/rspec/named_subject.rb

lib/rubocop/rspec/language.rb

sl4vr · 2020-08-07T17:38:14Z

NodePattern is bloated a bit with similar methods, I would wait for all RSpec language parts implementation and then do something like:

{
  regular_examples: ['Examples', 'Regular'],
  focused_examples: ['Examples', 'Focused'],
  skipped_examples: ['Examples', 'Skipped'],
  pending_examples: ['Examples', 'Pending'],
  hooks: ['Hooks'],
}.each do |name, config_keys|
  define_method "rspec_#{name}" do |keyword|
    send("#{name}_set").include?(keyword)
  end

  define_method "#{name}_set" do
    instance_variable_get("@#{name}_set") ||
      instance_variable_set(
        "@#{name}_set",
        Set.new(rspec_language_for(*config_keys))
      )
  end
end

Because it highly depends on config and may change yet.

pirj · 2020-11-02T17:51:44Z

@bquorning Both this PR's build and #1007 that I've just rebased onto this are green.

bquorning · 2020-11-02T17:52:27Z

But how come the specs passed when run with the entire suite? Usually it means there is some global state being changed somewhere.

pirj · 2020-11-02T18:13:15Z

There is indeed global state, in Language.config that is assigned when the first cop is initialized and starts the investigation.

bquorning

This PR looks so much better now. I think we are getting very close to merging.

lib/rubocop/rspec/language.rb

bquorning · 2020-11-02T20:33:48Z

lib/rubocop/rspec/language.rb


-      module ExampleGroups
-        GROUPS  = SelectorSet.new(%i[describe context feature example_group])


By the way, how come that we changed from ExampleGroups::GROUPS to ExampleGroups#regular and from Examples::EXAMPLES to Examples#regular?

Hm. I lost track of this.
To me, "regular" sounds better. But we at least have to mention this in the update_to_v2 document.

lib/rubocop/cop/rspec/let_setup.rb

bquorning · 2020-11-02T20:52:28Z

lib/rubocop/cop/rspec/empty_line_after_subject.rb

@@ -32,7 +32,7 @@ def on_block(node)

        def in_spec_block?(node)
          node.each_ancestor(:block).any? do |ancestor|
-            Examples::ALL.include?(ancestor.method_name)
+            Examples.all(ancestor.method_name)


This code reads a bit weird. It looks like there’s a question mark missing, i.e. Examples.all?(ancestor.method_name), but that’s also not good.

Probably not something we need to fix now, but just an observation.

Indeed. Those new helpers fit patterns more.
I can think of adding an include? alias for all to all of them, however, this will not provide a similar interface to Examples.regular.

Another way is to check if an argument has been passed (used in a pattern), or not. If not - just return a list, and this will make both Examples.all.include?(ancestor.method_name) and Examples.regular.include?(ancestor.method_name) possible.

Returning different types (boolean vs array) from the same method also isn’t great. Maybe we should call #all? from within out patterns? Also not really a great option :-/

I think we should just leave it for now.

lib/rubocop/cop/rspec/hooks_before_examples.rb

spec/rubocop/rspec/hook_spec.rb

bquorning

I… have run out of comments 😅

Thank you for the great work @pirj and @sl4vr. 🎉

Squash and merge at will.

pirj · 2020-11-03T10:29:58Z

@sl4vr Thanks a lot for implementing this. It is a huge amount of work, really, I now understand this after making subtle changes. It's time to claim your CoM trophy, but it's not all, you also get this symbolic golden RuboCop statuette:

@marcandre Thanks a lot for early-stage reviews, incredible suggestions and making changes to rubocop-ast that allowed this change to be made. You get a golden RuboCop statuette as well, and I guess it's not your first one!

marcandre · 2020-11-03T12:35:29Z

Thanks @pirj, you're too kind 😅
Congrats to @sl4vr, @bquorning and yourself for landing this 🎉

sl4vr · 2020-11-04T14:43:47Z

@pirj thank you for leading and finishing this PR 😅

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 - palkan#103

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

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 - palkan#103

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

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 - #103

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

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 - palkan/action_policy#103

sl4vr marked this pull request as draft July 7, 2020 18:26

sl4vr force-pushed the rspec-aliases_runtime-matchers branch 3 times, most recently from 2d63428 to f310907 Compare July 8, 2020 08:22

Darhazer reviewed Jul 8, 2020

View reviewed changes

lib/rubocop/rspec/language/node_pattern.rb Outdated Show resolved Hide resolved

sl4vr force-pushed the rspec-aliases_runtime-matchers branch 3 times, most recently from 62c3a15 to 346b685 Compare July 8, 2020 16:45

sl4vr closed this Jul 8, 2020

sl4vr force-pushed the rspec-aliases_runtime-matchers branch from 346b685 to 44e7d6e Compare July 8, 2020 16:49

sl4vr reopened this Jul 8, 2020

sl4vr force-pushed the rspec-aliases_runtime-matchers branch 3 times, most recently from 853e47b to bfa1475 Compare July 14, 2020 07:02

This was referenced Jul 15, 2020

WIP: Rspec aliases #921

Closed

[WIP] catch RSpec/ExpectInHook offenses in the setup block #894

Closed

pirj reviewed Jul 15, 2020

View reviewed changes

lib/rubocop/cop/rspec/named_subject.rb Outdated Show resolved Hide resolved

pirj reviewed Jul 15, 2020

View reviewed changes

pirj changed the title ~~WIP: Rspec aliases runtime matchers~~ RSpec aliases runtime matchers Jul 15, 2020

pirj mentioned this pull request Jul 15, 2020

Remove RSpec/EmptyExampleGroup's CustomIncludeMethods config option #969

Closed

1 task

Darhazer reviewed Jul 16, 2020

View reviewed changes

lib/rubocop/cop/rspec/cop.rb Outdated Show resolved Hide resolved

pirj mentioned this pull request Jul 21, 2020

RSpec/SharedContext: needs CustomIncludeMethods setting #975

Closed

pirj reviewed Aug 4, 2020

View reviewed changes

sl4vr force-pushed the rspec-aliases_runtime-matchers branch 4 times, most recently from 9590e21 to 89acc68 Compare August 6, 2020 06:58

bquorning reviewed Nov 2, 2020

View reviewed changes

bquorning approved these changes Nov 3, 2020

View reviewed changes

Implement dynamic matchers

e515d21

pirj force-pushed the rspec-aliases_runtime-matchers branch from d2d1355 to e515d21 Compare November 3, 2020 10:11

pirj merged commit 13cb62f into rubocop:release-2.0 Nov 3, 2020

This was referenced Nov 6, 2020

Add docs and provide a reference implementation on RSpec DSL aliases configuration in third-party gems #1077

Closed

Users should be able to globally configure example group and includes language #333

Closed

pirj mentioned this pull request Nov 8, 2020

Add RuboCop RSpec support palkan/action_policy#138

Merged

2 tasks

pirj mentioned this pull request Nov 9, 2020

Add RuboCop RSpec support test-prof/test-prof#199

Merged

2 tasks

erikkessler1 mentioned this pull request Nov 20, 2020

Upgrade to rubocop 1.0 and rubocop-rspec 2.0 salsify/salsify_rubocop#37

Merged

pirj mentioned this pull request May 11, 2021

RSpec/LeadingSubject is crashing if subject[] is used inside let #1150

Closed

pirj mentioned this pull request Oct 3, 2021

Fix inherit_mode for third-party gems #1192

Merged

5 tasks

pirj mentioned this pull request Jun 3, 2022

Make RSpec/FilePath support ActiveSupport inflections, if defined #1266

Closed

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RSpec aliases runtime matchers #956

RSpec aliases runtime matchers #956

sl4vr commented Jul 7, 2020 •

edited

Loading

sl4vr commented Jul 8, 2020 •

edited

Loading

pirj left a comment

pirj commented Aug 2, 2020

pirj left a comment

sl4vr commented Aug 7, 2020

pirj commented Nov 2, 2020

bquorning commented Nov 2, 2020

pirj commented Nov 2, 2020

bquorning left a comment

bquorning Nov 2, 2020

pirj Nov 2, 2020

bquorning Nov 2, 2020

pirj Nov 2, 2020

bquorning Nov 3, 2020

bquorning left a comment

pirj commented Nov 3, 2020

marcandre commented Nov 3, 2020

sl4vr commented Nov 4, 2020


		module ExampleGroups
		GROUPS = SelectorSet.new(%i[describe context feature example_group])

RSpec aliases runtime matchers #956

RSpec aliases runtime matchers #956

Conversation

sl4vr commented Jul 7, 2020 • edited Loading

sl4vr commented Jul 8, 2020 • edited Loading

pirj left a comment

Choose a reason for hiding this comment

pirj commented Aug 2, 2020

pirj left a comment

Choose a reason for hiding this comment

sl4vr commented Aug 7, 2020

pirj commented Nov 2, 2020

bquorning commented Nov 2, 2020

pirj commented Nov 2, 2020

bquorning left a comment

Choose a reason for hiding this comment

bquorning Nov 2, 2020

Choose a reason for hiding this comment

pirj Nov 2, 2020

Choose a reason for hiding this comment

bquorning Nov 2, 2020

Choose a reason for hiding this comment

pirj Nov 2, 2020

Choose a reason for hiding this comment

bquorning Nov 3, 2020

Choose a reason for hiding this comment

bquorning left a comment

Choose a reason for hiding this comment

pirj commented Nov 3, 2020

marcandre commented Nov 3, 2020

sl4vr commented Nov 4, 2020

sl4vr commented Jul 7, 2020 •

edited

Loading

sl4vr commented Jul 8, 2020 •

edited

Loading