-
Notifications
You must be signed in to change notification settings - Fork 25
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
Expand functionality: limit group for adapter #41
base: master
Are you sure you want to change the base?
Changes from 4 commits
979d3aa
9e588b5
88c3c3f
99dc00e
326c86d
4f251c1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -96,10 +96,23 @@ def temporary_tags | |
# | ||
# @param adapter_names [Array<Symbol>] Names of adapters to use | ||
def adapter(*adapter_names, group: @group) | ||
raise ConfigurationError, "Adapter limitation can't be defined outside of group" unless group | ||
raise ConfigurationError, "Adapter limitation can't be defined without adapter_names" if adapter_names.empty? | ||
|
||
@adapter_names = adapter_names | ||
if group | ||
include_group(group) | ||
else | ||
return yield if block_given? | ||
|
||
raise ConfigurationError, "Should be block passed with call .include_group for example" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That error message isn't very helpful because it doesn't tell what is wrong with this call to the Should be something like following, I think:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated the message. |
||
end | ||
end | ||
|
||
def include_group(group) | ||
raise ConfigurationError, "Adapter limitation can't be defined without of group name" unless group | ||
|
||
Yabeda.groups[group] ||= Yabeda::Group.new(group) | ||
Yabeda.groups[group].adapter(*adapter_names) | ||
Yabeda.groups[group].adapter(*@adapter_names) | ||
end | ||
|
||
private | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,8 +12,6 @@ | |
# Disable RSpec exposing methods globally on `Module` and `main` | ||
config.disable_monkey_patching! | ||
|
||
config.order = :random | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is not good when test suite result depends on order in which tests are executed. These dependencies need to be detected and removed (maybe something need to be added to the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree, it just solved my problem. Reverted it. |
||
|
||
config.expect_with :rspec do |c| | ||
c.syntax = :expect | ||
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'm a bit worried that this instance variable is being kept defined after
adapter
method has ended. While it should be harmless now, it can possibly create problems in the future. Let's clear it in theensure
block maybe?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.
Done.