rubocop-rspec icon indicating copy to clipboard operation
rubocop-rspec copied to clipboard

RSpec/FilePath and metadata

Open schwern opened this issue 4 years ago • 4 comments

We have some spec files which are isolated by their metadata. For example, specs for services which touch real APIs using test accounts. We want them to be completely isolated from other tests.

# spec/some_service/live_spec.rb

describe SomeService, :live do
  # tests which contact real services
end

This is in spec/some_service/live_spec.rb. RSpec/FilePath does not like this and I don't see any way to configure it to be satisfied. If we put it into spec/some_service_spec.rb then RSpec/MultipleDescribes is unhappy. I do not want to nest it inside the other describe SomeService for risk of the configuration of the live tests polluting the mock tests, or vice-versa.

Is there a preferred way of handling this?

  • rubocop-rspec 2.2.0
  • rubocop 1.9.1
  • ruby 2.7.2

schwern avatar Mar 30 '21 22:03 schwern

Do you use this metadata in some way, or is it there for purely informational purpose?

An option to trick the cop would be to:

describe SomeService, 'live', :live do

There is certainly some duplication, but at the same time documentation formatter output makes it clear what this example group is about.

Do you have some change to the cop in mind?

pirj avatar Mar 30 '21 22:03 pirj

Do you use this metadata in some way, or is it there for purely informational purpose?

By default, live tests are skipped. They are run as a release test.

RSpec.configure do |config|
  config.filter_run_excluding live: true unless ENV["TEST_ALL"]
end

I would prefer not to add a fake method to the describe. That would be bad style which defeats the point.

Perhaps the cop could be configured to treat metadata like methods? So spec/some_service/live_spec.rb is acceptable for both describe SomeService, :live and describe SomeService, '#live'. I'm also open to changing how we handle these tests.

schwern avatar Mar 30 '21 22:03 schwern

I'm puzzled to find a better option off the top of my head. On the other head I'm not certain if this is a widespread naming scheme, and in this case, I'm not convinced to introduce such an option.

pirj avatar Mar 30 '21 23:03 pirj

Frankly, I find it puzzling why the following passes:

    expect_no_offenses(<<-RUBY, 'foo_bar_spec.rb')
      describe Foo do; end
    RUBY

while this fails:

    expect_no_offenses(<<-RUBY, 'foo/bar_spec.rb')
      describe Foo do; end
    RUBY

Was it a deliberate design decision?

What do you think of relaxing the cop to allow a more flexible directory structure, @bquorning, @Darhazer ?

The ticket in question is just one case, I hope I could hint at the other with the above examples.

real-world-rspec highlights:

57497 files inspected, 1999 offenses detected

/Users/pirj/source/real-world-rspec/spree/core/spec/models/spree/order/updating_spec.rb:3:1: C: RSpec/FilePath: Spec path should end with spree/order*_spec.rb.
describe Spree::Order, type: :model do
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

solidus/core/spec/models/spree/order/state_machine_spec.rb:5:1: C: RSpec/FilePath: Spec path should end with spree/order*_spec.rb.
RSpec.describe Spree::Order, type: :model do
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

rspec-core/spec/rspec/core_spec.rb:3:1: C: RSpec/FilePath: Spec path should end with rspec*_spec.rb.
RSpec.describe RSpec do
^^^^^^^^^^^^^^^^^^^^

refinerycms/core/spec/lib/refinery/core_spec.rb:3:1: C: RSpec/FilePath: Spec path should end with refinery*_spec.rb.
describe Refinery do
^^^^^^^^^^^^^^^^^

gitlabhq/spec/services/ci/create_pipeline_service/tags_spec.rb:4:1: C: RSpec/FilePath: Spec path should end with ci/create_pipeline_service*_spe
c.rb.
RSpec.describe Ci::CreatePipelineService do
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

canvas-lms/spec/models/content_migration/course_copy_pace_plans_spec.rb:22:1: C: RSpec/FilePath: Spec path should end with content_migration*_sp
ec.rb.
describe ContentMigration do
^^^^^^^^^^^^^^^^^^^^^^^^^

pirj avatar Jun 16 '22 23:06 pirj