WIP: Add ignoring_interference_by_writer to everything
#866
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| it 'accepts (and not raise an error)' do | ||
| model = define_model_validating_uniqueness( | ||
| attribute_name: :name, | ||
| validation_options: { case_sensitive: false } |
There was a problem hiding this comment.
Put a comma after the last parameter of a multiline method call.
9 tasks
mcmire
force-pushed
the
ew-add-ignoring-interference-by-writer
branch
2 times, most recently
from
24a358e to
f6b62bb
Compare
| @@ -14,6 +14,8 @@ | |||
| config.infer_spec_type_from_file_location! | |||
| end | |||
|
|
|||
| config.example_status_persistence_file_path = "spec/examples.txt" | |||
There was a problem hiding this comment.
Prefer single-quoted strings when you don't need string interpolation or special symbols.
|
|
||
| config.infer_spec_type_from_file_location! | ||
| config.example_status_persistence_file_path = "spec/examples.txt" | ||
| config.alias_it_behaves_like_to(:it_supports, "it supports") |
There was a problem hiding this comment.
Prefer single-quoted strings when you don't need string interpolation or special symbols.
| def validation_matcher_scenario_args | ||
| { | ||
| matcher_name: :validate_uniqueness_of, | ||
| model_creator: :"active_record/uniqueness_matcher" |
There was a problem hiding this comment.
Put a comma after the last item of a multiline hash.
We need to use this in another part of the codebase: later, when we add ignoring_interference_by_writer to all matchers, we will need a way of overriding an attribute such that it produces another value that invalidates the test. Some tests require us to generate dummy values.
The main driver behind this commit is to provide a programmatic way to define models in tests. We already have ways of doing this, of course, with `define_model` and `define_active_model_class`, but these methods are very low-level, and in writing tests we have historically made our own methods inside of test files to define full and complete models. So we have this common pattern of defining a model with a validation, and that's repeated across many different files. What we would like to do, right now, is extract some commonly used assertions to a shared example group. These assertions need to define models inside of the tests, but the issue is that sometimes the models are ActiveRecord models, and sometimes they are ActiveModel models, and when the shared example group is used within a test file, we need a way to choose the strategy we'd like to use at runtime. Since the way we currently define models is via methods, we can't really provide a strategy very easily. Also, if we need to customize how those models are defined (say, the attribute needs to be a has-many association instead of a normal attribute) then the methods only go so far in providing us that level of customization before things get really complicated. So, to help us with this, this commit takes the simple pattern of model-plus-validation previously mentioned and places it in a class, then splits up that logic. So there are actually four main classes that we are introducing here: * CreateModel: for creating a model in an abstract way. You'll couple this with one of two lower-level strategies, which may also be used on their own: * ModelCreationStrategies::ActiveModel * ModelCreationStrategies::ActiveRecord * ActiveRecord::CreateTable: for creating a table that goes along with an ActiveRecord model. Note that `define_model` and `define_active_model_class` have not been changed so as not to completely break every single test, but they now simply call out to ModelCreationStrategies::ActiveModel and ModelCreationStrategies::ActiveRecord internally.
Also add a helper method, `build_scenario_for_validation_matcher`, to all unit tests. Our plan is to slowly transition all of the tests to use this helper method, and that we we can get rid of the umpteen different ways we are building records across all of the tests.
When used against an attribute that's an enum, `allow_value` will raise an AttributeValueChangedError. This commit prevents that from happening.
`allow_value` matcher is, of course, concerned with setting values on a particular attribute on a particular record, and then checking that the record is valid after doing so. That comes with a caveat: if the attribute is overridden in such a way so that the same value going into the attribute isn't the same value coming out of it, then `allow_value` will balk -- it'll say, "I can't do that because that changes how I work." That's all well and good, but what the attribute intentionally changes incoming values? ActiveRecord's typecasting behavior, for instance, would trigger such an exception. What if the developer needs a way to get around this? This is where `ignoring_interference_by_writer` comes into play. You can tack it on to the end of the matcher, and you're free to go on your way. So, prior to this commit you could already apply it to `allow_value`, but now in this commit it also works on any other matcher. But, one little thing: sometimes using this qualifier isn't going to work. Perhaps you or something else actually *is* overriding the attribute to change incoming values in a specific way, and perhaps the value that comes out makes the record fail validation, and there's nothing you can do about it. So in this case, even if you're using `ignoring_interference_by_writer`, we want to inform you about what the attribute is doing -- what the input and output was. And so we do.
mcmire
force-pushed
the
ew-add-ignoring-interference-by-writer
branch
from
e016615 to
e56a9cc
Compare
| def validation_matcher_scenario_args | ||
| super.deep_merge( | ||
| matcher_name: :validate_uniqueness_of, | ||
| model_creator: :"active_record/uniqueness_matcher" |
There was a problem hiding this comment.
Put a comma after the last parameter of a multiline method call.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is basically an answer to #799, #800, #819, and other issues, and also supercedes #820.
The idea here is to add
ignoring_interference_by_writerto all matchers (except forallow_value) -- but with a key twist, which is that matchers are still aware of when attributes change values, but it will only let the user know about them if the matcher in question fails. This way, the user (and I) still have a window in what is going on under the hood, but it won't disrupt what the user is trying to do normally.