Skip to content
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

Allow array as value for type key in Configuration#include #2890

Closed
MatheusRich opened this issue Apr 23, 2021 · 5 comments
Closed

Allow array as value for type key in Configuration#include #2890

MatheusRich opened this issue Apr 23, 2021 · 5 comments

Comments

@MatheusRich
Copy link
Contributor

Subject of the issue

I want to be able to pass multiple types in the type config of the method RSpec::Core::Configuration#include. This would allow a cleaner configuration.

Your environment

  • Ruby version: ruby 2.7.2p137 (2020-10-01 revision 5445e04352) [x86_64-linux-musl]
  • rspec-core version: rspec-core 3.10.1

Steps to reproduce

This is the current way of including a config in multiple types:

RSpec.configure do |config|
  # ...

  config.include Capybara::RSpecMatchers, type: :request
  config.include Capybara::RSpecMatchers, type: :component
end

Expected behavior

Here's what I'd like to being able to do (We could add a key types if this change is not approved).

RSpec.configure do |config|
  # ...

  config.include Capybara::RSpecMatchers, type: [:request, :component]
end

Actual behavior

If an array is given, RSpec seems to ignore the option.

I'd be happy to provide a PR for this.

@pirj
Copy link
Member

pirj commented Apr 23, 2021

type is regular metadata. Introducing special handling for type doesn't seem to be justified, especially if it's for include only.
Changing how we handle metadata in the common case is a risk of breaking someone else's code, e.g.:

context 'with any of', numbers: [0, 20] do

To my taste, this would be a bit too magical, and would add more complexity, while we strive to simplify how it works. See

Please accept my apologies if I'm overreacting to a proposal for a trivial change. Can you please briefly explain what changes do you have in mind exactly?

Also, it should work fine this way:

config.include Capybara::RSpecMatchers, type: ->(type) { [:request, :component].include?(type) }
# or even
def in(*list)
  ->(value) { list.include(value) }
end

config.include Capybara::RSpecMatchers, type: in(:request, :component)

@MatheusRich
Copy link
Contributor Author

MatheusRich commented Apr 23, 2021

I have nothing specific in mind, I just wanted to get this conversation started. If it's easier to add, to me the proc/lambda is fine.

@JonRowe
Copy link
Member

JonRowe commented Apr 28, 2021

Can I suggest that this is already catered for by Ruby?

[:request, :component].each { |type| config.include Capybara::RSpecMatchers, type: type }

@MatheusRich
Copy link
Contributor Author

@JonRowe That definitely works, I just opened the issue to check if there's any other way to do it or if it is a common pattern so we could handle it at the library level.

Is there any other config that would benefit from an array/proc like we discussed above?

If not, i guess we can close this one.

@pirj
Copy link
Member

pirj commented Apr 28, 2021

any other config that would benefit from an array

Can't tell of the top of my head.

Proc and each approaches are there already.
Since it's already quite tricky and magical at times. Misconfiguration is sometimes hard to pinpoint.
I'd rather keep it as is if there's no major benefit.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants