-
-
Notifications
You must be signed in to change notification settings - Fork 909
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
add true option for dependent matcher and test all options #631
Changes from 1 commit
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 |
---|---|---|
|
@@ -11,3 +11,4 @@ pkg | |
tags | ||
test/*/log/*.log | ||
tmp | ||
.ruby-version | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,11 +18,10 @@ def description | |
|
||
def matches?(subject) | ||
self.subject = ModelReflector.new(subject, name) | ||
|
||
if option_verifier.correct_for_string?(:dependent, dependent) | ||
if correct_for?(dependent) | ||
true | ||
else | ||
self.missing_option = "#{name} should have #{dependent} dependency" | ||
self.missing_option = missing_option_for(name, dependent) | ||
false | ||
end | ||
end | ||
|
@@ -34,6 +33,18 @@ def matches?(subject) | |
def option_verifier | ||
@option_verifier ||= OptionVerifier.new(subject) | ||
end | ||
|
||
def correct_for?(dependent=dependent) | ||
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. Why do you have default values here? Why not just 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. The original reason was that there is a |
||
case dependent | ||
when true, false then option_verifier.correct_for_boolean?(:dependent, dependent) | ||
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. Line is too long. [93/80] |
||
else option_verifier.correct_for_string?(:dependent, dependent) | ||
end | ||
end | ||
|
||
def missing_option_for(name=name, dependent=dependent) | ||
"#{name} should have #{dependent == true ? "a" : dependent} dependency" | ||
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. Line is too long. [83/80] |
||
end | ||
|
||
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. Extra empty line detected at body end. |
||
end | ||
end | ||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,16 @@ | ||
require 'unit_spec_helper' | ||
|
||
def each_dependent_option(rails_version) | ||
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. Three things:
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. Should be all set. |
||
case rails_version | ||
when /\A3/ | ||
[:destroy, :delete, :nullify, :restrict] | ||
when /\A4/ | ||
[:destroy, :delete, :nullify, :restrict_with_exception, :restrict_with_error] | ||
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. Line is too long. [81/80] |
||
end.each do |option| | ||
yield option | ||
end | ||
end | ||
|
||
describe Shoulda::Matchers::ActiveRecord::AssociationMatcher, type: :model do | ||
context 'belong_to' do | ||
it 'accepts a good association with the default foreign key' do | ||
|
@@ -645,9 +656,29 @@ def having_many_non_existent_class(model_name, assoc_name, options = {}) | |
expect(Person.new).not_to have_one(:detail) | ||
end | ||
|
||
it 'accepts an association with a valid :dependent option' do | ||
expect(having_one_detail(dependent: :destroy)). | ||
to have_one(:detail).dependent(:destroy) | ||
each_dependent_option(Rails.version) do |option| | ||
it 'accepts an association with a valid :dependent option' do | ||
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. Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. |
||
expect(having_one_detail(dependent: option)). | ||
to have_one(:detail).dependent(option) | ||
end | ||
end | ||
|
||
it 'accepts any dependent option if true' do | ||
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. Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. 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. Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. |
||
each_dependent_option(Rails.version) do |option| | ||
expect(having_one_detail(dependent: option)). | ||
to have_one(:detail).dependent(true) | ||
end | ||
end | ||
|
||
it 'rejects any dependent options if false' do | ||
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. Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. |
||
each_dependent_option(Rails.version) do |option| | ||
expect(having_one_detail(dependent: option)). | ||
to_not have_one(:detail).dependent(false) | ||
end | ||
end | ||
|
||
it 'accepts a nil dependent option if false' do | ||
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. Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. |
||
expect(having_one_detail).to have_one(:detail).dependent(false) | ||
end | ||
|
||
it 'rejects an association with a bad :dependent option' do | ||
|
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.
Can you remove this change? Thanks