-
-
Notifications
You must be signed in to change notification settings - Fork 278
RSpec/SpecFilePathFormat allow multiple values for same key in IgnoreMetadata #2076
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
base: master
Are you sure you want to change the base?
RSpec/SpecFilePathFormat allow multiple values for same key in IgnoreMetadata #2076
Conversation
…reMetadata values to ignore for each key
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should change the default config to use an array:
IgnoreMetadata:
- type: routing
+ type:
+ - routing
VersionAdded: '2.24'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left some feedback on the previous PR.
Can you please check?
I’ll move my comments to this PR when I het back to my computer.
I think the current approach will lead to ambiguous configuration, and we should do this differently. I haven’t heard counter arguments, so I’m setting a block until we figure out how to proceed. |
# @example `IgnoreMetadata: {type=>[routing,models]}` (default) | ||
# # good | ||
# whatever_spec.rb # describe MyClass, type: :routing do; end | ||
# whatever_spec.rb # describe MyClass, type: :models do; end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if the metadata's value IS an array?
RSpec.describe Airplane, prepare: [:fuel] do
How do we define IgnoreMetadata in YAML?
# @example `IgnoreMetadata: {type=>[routing,models]}` (default) | ||
# # good | ||
# whatever_spec.rb # describe MyClass, type: :routing do; end | ||
# whatever_spec.rb # describe MyClass, type: :models do; end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I propose:
IgnoreMetadata:
- prepare # ignore by presence of a key
- type: model # ignore one value
- type: routing # and another, too
- skip: [database] # ignore when the value is an array of specific values
Luckily enough, YAML allows to mix all that, and to cover all ignored cases unambiguously.
Retry of PR #2070 - I seem to have fat-fingered the rebase to master in the previous PR. So here is a fresh PR based off of the current master with all the change requests of the original included.
Description of original PR:
RSpec/SpecFilePathFormat allows ignoring certain values in rspec metadata through the IgnoreMetadata option, as given in the documentation:
However the current implementation does not seem to allow supplying multiple values for the same key - e.g. if you have some specs marked
type: controller
, others astype: :model
etc, only one ofcontroller
,model
, ... can be supplied to be ignored.This is probably due to an oversight (or me not being able to figure out how to properly supply an array of values to be ignored).
The PR allows for either an array or a single value to be given for a key in the IgnoreMetadata option.
While it would also be desirable to give a
true
value to ignore all metadata, in the format thatIgnoreMethods
use, this should probably be part of a different PR.