do not mock the validator in form type tests #11149
Conversation
|
Thanks to @clavat who made me aware of this overly complicated section that was not so helpful IMO. |
form/unit_testing.rst
Outdated
| // or if you also need to read constraints from annotations | ||
| $validator = Validation::createValidatorBuilder() | ||
| ->enableAnnotationMapping() | ||
| ->getValidator(); | ||
|
|
||
| return [ | ||
| new ValidatorExtension($this->validator), |
There was a problem hiding this comment.
| new ValidatorExtension($this->validator), | |
| new ValidatorExtension($validator), |
xabbuh
force-pushed
the
form-test-validator-extension
branch
from
575f8fb to
272e479
Compare
| $this->validator | ||
| ->method('getMetadataFor') | ||
| ->will($this->returnValue(new ClassMetadata(Form::class))); | ||
| $validator = Validation::createValidator(); |
There was a problem hiding this comment.
I would keep the current private property and do:
if (null === $this->validator) {
$this->validator = Validation::createValidator();
// ...
}Otherwise the setUp will be heavy , so also maybe a comment or an implementation of tearDownAfterClass.
There was a problem hiding this comment.
I don't really see for what you would need to access the validator. Previously this was just the case because you had to define in each tests individually which constraint violations should be raised.
There was a problem hiding this comment.
The property would not be meant to access it anymore but to prevent creating the validator for running each test in a class, performances wise.
We just need to create it once now that we don't need to control the mock anymore.
There was a problem hiding this comment.
That makes sense but is it really worth it? Wouldn't it just make the code harder to understand compared to the provided benefit?
There was a problem hiding this comment.
What about a PR to see the diff? You could advise to have that a logic in a trait. We could even provide such trait in core.
|
A lovely improvement! Thanks Christian and @clavat |
This PR was merged into the 3.4 branch. Discussion ---------- do not mock the validator in form type tests Commits ------- 272e479 do not mock the validator in form type tests
No description provided.