Make RSpec describe_rule DRY

[kaka-ruto]

As is, the describe_rule dsl works with a single string rule

However, sometimes, a couple of rules behave in the same way, and you test them in the same manner.

For example, the following rules are tested in the same manner:

describe_rule :update? do
    succeed 'when user is the workspace owner' do
      let(:context) { { user: user } }
      let(:record) { build_stubbed :workspace, owner: user }
    end
     
    failed 'when user is not the workspace owner' do
      let(:context) { { user: user } }
      let(:record) { build_stubbed :workspace, owner: unauthorized_user }
    end
  end

describe_rule :show? do
    succeed 'when user is the workspace owner' do
      let(:context) { { user: user } }
      let(:record) { build_stubbed :workspace, owner: user }
    end
     
    failed 'when user is not the workspace owner' do
      let(:context) { { user: user } }
      let(:record) { build_stubbed :workspace, owner: unauthorized_user }
    end
  end

This can be refactored to allow more than one rules to be passed to the policy_rule, preferably in an array format. So the above code can be refactored to:

describe_rule [:update?, :show?] do
    succeed 'when user is the workspace owner' do
      let(:context) { { user: user } }
      let(:record) { build_stubbed :workspace, owner: user }
    end
     
    failed 'when user is not the workspace owner' do
      let(:context) { { user: user } }
      let(:record) { build_stubbed :workspace, owner: unauthorized_user }
    end
  end

Is this eligible for a new feature, an extension of the describe_rule dsl?

[palkan]

Hey @borenho!

Thanks for the proposal.

The idea looks useful in case you have very similar rules (if they 100% similar it's better to use rule aliases though).

On the other hand, auto-generated specs are much harder to debug and use for refactoring, especially when it's a gem specific implementation.

Can we use an Rspec built-in feature instead — shared example? Like this:

shared_examples_for "shared:update?" do
    succeed 'when user is the workspace owner' do
      let(:context) { { user: user } }
      let(:record) { build_stubbed :workspace, owner: user }
    end
     
    failed 'when user is not the workspace owner' do
      let(:context) { { user: user } }
      let(:record) { build_stubbed :workspace, owner: unauthorized_user }
    end
end

describe_rule :update? do
  include_examples "shared:update?"

  # update? specific examples go here
end

describe_rule :show? do
  include_examples "shared:update?"
  
  # show? specific example go here
end

A bit more verbose but we can add rule-specific examples to the shared ones. WDYT?

The only question here: does DSL work in shared examples? I'm not sure. Then making it possible to use DSL in shared examples and contexts would be a useful enhancement.

[dgarwood]

@palkan I have a similar use case and what you have is exactly what I'm doing.

The DSL works, but rubocop-rspec reports it as a place where 'shared_context' should be used instead of 'shared_examples' because there are no specs defined. I disabled the rule around the shared_example declaration because shared_context did break it.

[palkan]

The DSL works

Nice 👍 Then, we can close the issue, I think.

rubocop-rspec reports it as a place where 'shared_context' should be used instead of 'shared_examples' because there are no specs defined

Yeah, it doesn't understand custom example definition methods (succeed, failed). I will talk to rubocop-rspec maintainers, I think, we can add a list of example definition methods configurable.

[palkan]

It turned out, there is already a similar issue in rubocop-rspec: rubocop/rubocop-rspec#333

[dgarwood]

in case anyone runs into this in the future, this is what I had to do after updating rubocop to 1.21, rubocop-rspec ~> 2.0 and action_policy ~> 0.6.0:

I have some tests that use it_behaves_like in the action policy specs, and the config for Rubocop ended up looking like this:

inherit_gem:
  action_policy: config/rubocop-rspec.yml

...

RSpec:
  Language:
    Includes:
      inherit_mode:
        merge:
          - Examples
      Examples:
      # rules for Action Policy testing DSL
      - it_behaves_like

the inherit_mode is the important part, and can be found here: https://docs.rubocop.org/rubocop/configuration.html#merging-arrays-using-inherit_mode

@palkan not sure if this deserves a mention in the lint docs

[palkan]

@dgarwood Thanks for reporting. Looks like this (already fixed) issue: rubocop/rubocop-rspec#1126

I guess, we need to specify inherit_mode in our configuration. @pirj Hey, could you confirm please?

[pirj]

Sorry, I've missed that you are using rubocop 1.21.
It should not require setting inherit_mode in the project config or action_policy default config. If it does, then the bug was not completely fixed, as it was intended to be completely transparent for users.

[pirj]

For the following spec:

RSpec.describe do
  it_behaves_like 'a'
  it_behaves_like 'a'

  describe_rule :show? do
    succeed 'when post is published'
    succeed 'when post is published'
  end
end

and a clean user .rubocop.yml configuration like:

require:
  - rubocop-rspec

inherit_gem:
  action_policy: config/rubocop-rspec.yml

I would expect RSpec/RepeatedIncludeExample to be triggered by both the duplicated success and it_behaves_like.
However, currently, even with the fix, it doesn't merge succeed to Includes/Examples, but rather overwrites it. This results in the duplicate succeed detection, and missing out on the duplicated it_behaves_like.

There are three options:

1. Tell users to tweak inherit_mode in the user configuration, like you do, @dgarwood . It certainly does the job, and is a feasible workaround. However, burdening end-users of rubocop-rspec with boilerplate was not the original intent.
2. Submit a fix to action_policy with inherit_mode for what it adds. For this, I'd have to:

• send a follow-up fix for action_policy
• send a follow-up fix for test-prof
• update rubocop-rspec docs for libraries RSpec DSL config
• notify rspec-its, saharspec, cancancan, pundit about the above changes (see Add docs and provide a reference implementation on RSpec DSL aliases configuration in third-party gems rubocop/rubocop-rspec#1077)
• check if the above gems' maintainers managed to create their own config/default.ymls the right way and assist with fixing

3. Add inherit_mode on rubocop-rspec side

I've checked all three approaches, and they work equally fine, rubocop flags both duplications.

I'll send a PR to rubocop-rspec, adding a note to docs that this feature only works fine with the next minor version of it.

Please keep in mind to update to rubocop-rspec 2.5.1 or 2.6.0 whichever will be released next and include the fix.
You'll be free of necessity to tweak .rubocop.yml in your project.
Before rubocop-rspec is released on RubyGems, you can use it from git:

gem 'rubocop-rspec', github: 'rubocop/rubocop-rspec'

Sorry for the trouble (again).

[pirj]

rubocop/rubocop-rspec#1192

[pirj]

@dgarwood Fixed on rubocop-rspec side. Can you please check if it fixes your issue:

# Gemfile
gem 'rubocop-rspec', github: 'rubocop/rubocop-rspec'

I'm mostly interested that you don't have to include this part any more:

      inherit_mode:
        merge:
          - Examples