Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[DX] [Form] Add helper method to register form extensions during unit testing#21780
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
279b0ef to082c642Compare| $this->dispatcher =$this->getMockBuilder('Symfony\Component\EventDispatcher\EventDispatcherInterface')->getMock(); | ||
| $this->builder =newFormBuilder(null,null,$this->dispatcher,$this->factory); | ||
| $this->validator =$this->getMockBuilder('Symfony\Component\Validator\Validator\ValidatorInterface')->getMock(); |
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.
This will break existing applications that use theTypeTestCase class, but do not have the Validator component installed.
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.
Wrapped the extension in a check to make sure ValidatorInterface exists
082c642 to4986008Compare| $this->builder =newFormBuilder(null,null,$this->dispatcher,$this->factory); | ||
| } | ||
| protectedfunctiongetTypedExtensions() |
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.
If you want this to be called automatically based on the parentFormIntegrationTestCase class, you will have to rename this method togetTypeExtensions().
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.
Thanks, it was supposed to begetTypeExtensions, I don't know how that d sneaked in there
4986008 to3481246CompareHeahDude commentedMar 3, 2017
Hi@pierredup, thank you for proposing this. In fact there is already thisTypeTestCase form the validator extension that can be extended, it does extend thebase TypeTestCase with the change you suggest. I've openedsymfony/symfony-docs#7566 so we don't forget to document it, we may close here though. |
pierredup commentedMar 4, 2017
@HeahDude I initially did try using that, but I got the error This is because the This still leaves the issue that it's not easy to register your own type extensions. You can pass it as the second argument to the |
| if (interface_exists(ValidatorInterface::class)) { | ||
| $validator =$this->getMockBuilder(ValidatorInterface::class)->getMock(); | ||
| $validator->expects($this->any())->method('validate')->will($this->returnValue(array())); |
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.
Then only this line should be added in the proper validator extension base type, we don't need to add the extension overhead to all base tests.
| returnarray(); | ||
| } | ||
| protectedfunctiongetTypeExtensions() |
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.
If this line were too be added in favor of:
useSymfony\Component\Form\PreloadedExtension;// ...protectedfunctiongetExtensions(){returnarray(newPreloadedExtension(array(newSomeType($this->getDependencyMock())),array(newSomeTypeExtension(newSomeService()),array(newSomeGuesser($this->getDependencyMock2()),newSomeOtherGuesser()), ), ));}
We should also add two other protected functions for types and guessers, and addaddTypeGuessers andaddTypes call above as well to be consistent.
I would not vote against it, but I would vote for a better doc.
pierredup commentedMar 6, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I created a separate PR (#21886) to address the validate method issue, which can go as a bug fix, and updated this PR as a new feature to include extra helper methods to register form extensions for unit testing for DX |
xabbuh commentedMar 6, 2017
👍 Status: Reviewed |
fabpot commentedMar 6, 2017
Can you add a note in the CHANGELOG? |
fabpot commentedMar 6, 2017
Thank you@pierredup. |
fabpot commentedMar 6, 2017
see638bdc8 |
| returnarray(); | ||
| } | ||
| protectedfunctiongetTypedExtensions() |
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.
the method should IMO be namedgetTypeExtensions()
…or unit testing (pierredup)This PR was squashed before being merged into the master branch (closes#7578).Discussion----------Add docs to register custom form extensions and types for unit testingDocs forsymfony/symfony#21780 to register custom extensions and typesCommits-------45d6ed8 Add docs to register custom form extensions and types for unit testing
…) (xabbuh)This PR was merged into the 3.3-dev branch.Discussion----------[Form] rename getTypedExtensions() to getTypeExtensions()| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#21780 (comment)| License | MIT| Doc PR |Commits-------099f626 rename getTypedExtensions() to getTypeExtensions()
Uh oh!
There was an error while loading.Please reload this page.
Add helper method to register form extensions during unit testing, so it's easier to register custom form type extensions, form types, or type guessers