Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -925,6 +925,7 @@ RSpec/SpecFilePathFormat:
IgnoreMetadata:
type: routing
VersionAdded: '2.24'
VersionChanged: "<<next>>"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is unnecessary. Could you please revert?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, however I'd like to confirm that the revert would be correct, as the Pull Request template specifically states:

If you have modified an existing cop's configuration options:

- [ ] Set `VersionChanged: "<<next>>"` in `config/default.yml`.

and this PR does change an existing cops configuration options.

@ydah ydah Apr 4, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to my understanding, "modified an existing cop's configuration options" refers to instances such as when adding a new configuration option, changing a name, or for example, when adding autocorrect functionality and adding Autocorrect: true, we should add VersionChanged: "<<next>>".

Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/SpecFilePathFormat

RSpec/SpecFilePathSuffix:
Expand Down
10 changes: 9 additions & 1 deletion lib/rubocop/cop/rspec/spec_file_path_format.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ module RSpec
# # good
# whatever_spec.rb # describe MyClass, type: :routing do; end
#
# @example `IgnoreMetadata: {type=>[routing,models]}` (default)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it really configured like this in YAML?

# # good
# whatever_spec.rb # describe MyClass, type: :routing do; end
# whatever_spec.rb # describe MyClass, type: :models do; end

Copy link
Copy Markdown
Member

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?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Side note: how do we ignore any value for a key?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm hinting towards:

IgnoreMetadata:
  - prepare
  - type: model
  - type: routing
  - skip: [database]

Luckily enough, YAML allows to mix all that, and to cover all ignored cases unambiguously.

#
class SpecFilePathFormat < Base
include TopLevelGroup
include Namespace
Expand Down Expand Up @@ -73,7 +78,10 @@ def ensure_correct_file_path(send_node, class_name, arguments)
def ignore_metadata?(arguments)
arguments.any? do |argument|
metadata_key_value(argument).any? do |key, value|
ignore_metadata.values_at(key.to_s).include?(value.to_s)
ignore_values = ignore_metadata[key.to_s]
ignore_values = [ignore_values] unless ignore_values.is_a?(Array)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use Array() instead:

Suggested change
ignore_values = ignore_metadata[key.to_s]
ignore_values = [ignore_values] unless ignore_values.is_a?(Array)
ignore_values = Array(ignore_metadata[key.to_s])


ignore_values.include?(value.to_s)
end
end
end
Expand Down
31 changes: 31 additions & 0 deletions spec/rubocop/cop/rspec/spec_file_path_format_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -281,4 +281,35 @@ class Foo
RUBY
end
end

context \
'when configured with `IgnoreMetadata: { "foo" => ["bar", "baz"] }`' do
let(:cop_config) { { 'IgnoreMetadata' => { 'foo' => %w[bar baz] } } }

it 'registers no offense when including an ignored metadata value' do
expect_no_offenses(<<~RUBY, 'wrong_class_spec.rb')
describe MyClass, foo: :bar do; end
RUBY
end

it 'registers no offense when including another ignored metadata value' do
expect_no_offenses(<<~RUBY, 'wrong_class_spec.rb')
describe MyClass, foo: :baz do; end
RUBY
end

it 'registers an offense when the metadata is not to be ignored' do
expect_offense(<<~RUBY, 'wrong_class_spec.rb')
describe MyClass, foo: :quux do; end
^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Spec path should end with `my_class*_spec.rb`.
RUBY
end

it 'registers an offense when no metadata is present' do
expect_offense(<<~RUBY, 'wrong_class_spec.rb')
describe MyClass do; end
^^^^^^^^^^^^^^^^ Spec path should end with `my_class*_spec.rb`.
RUBY
end
end
end