-
Notifications
You must be signed in to change notification settings - Fork 137
Implemented test suite for widget rendering #275
Conversation
|
||
/** | ||
* Class BaseWidgetTest. | ||
*/ |
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.
@author
tag is missing
LGTM 👍 @greg0ire can you give us a final review please? |
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function setUp() |
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.
Should be protected
a141da5
to
2285fed
Compare
* | ||
* @author Christian Gripp <mail@core23.de> | ||
*/ | ||
abstract class BaseWidgetTest extends TypeTestCase |
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.
Please wait sonata-project/dev-kit#116 (comment) decision before merge. |
@core23 we have a decision, can you please check if this PR fullfill this? |
Don't know what's missing. |
__DIR__.'/../../../vendor/symfony/twig-bridge/Symfony/Bridge/Twig/Resources/views/Form', | ||
__DIR__.'/../../../vendor/symfony/twig-bridge/Resources/views/Form', | ||
__DIR__.'/../../../vendor/symfony/symfony/src/Symfony/Bridge/Twig/Resources/views/Form', | ||
__DIR__.'/../../../../../symfony/symfony/src/Symfony/Bridge/Twig/Resources/views/Form', |
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.
Please document each line, explaining in which situation each can happen. If you don't know, simply remove the line, and it will be added later with a comment if really needed. BTW, I don't think running tests from a Symfony project should be supported.
A test case should have documentation of how to use it. |
BTW, a test that meant to be publicly used should be tested as well. Or at lest implemented on this bundle for code coverage. |
Testception :) |
1608b2e
to
3fb5734
Compare
Added |
Added a new |
RTM? |
2f71a2c
to
ff3b770
Compare
Ping @soullivaneuh @greg0ire |
Can you give a final review @soullivaneuh @greg0ire |
$html = $this->renderWidget($choice->createView()); | ||
|
||
$this->assertContains( | ||
'<div id="choice"><input type="checkbox" id="choice_0" name="choice[]" value="0" /><label for="choice_0">[trans]some[/trans]</label><input type="checkbox" id="choice_1" name="choice[]" value="1" /><label for="choice_1">[trans]choices[/trans]</label></div>', |
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.
Use line breaks here please :
<?php
$this->cleanHtmlWhitespace(<<<HTML
<div id="choice">
<input type="checkbox" id="choice_0" name="choice[]" value="0" />
<label for="choice_0">[trans]some[/trans]</label>
<input type="checkbox" id="choice_1" name="choice[]" value="1" />
<label for="choice_1">[trans]choices[/trans]</label>
</div>
HTML)
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.
Fixed that
@core23 can you please check travis? |
All green :) |
LGTM |
Thank you @core23 👍 |
Changelog
Subject
Added a test suite for widget rendering. Basically it's a port of https://github.com/sonata-project/SonataAdminBundle/blob/3.x/Tests/Form/Widget/BaseWidgetTest.php to have a test suite for all bundles.
This can be used for sonata-project/SonataFormatterBundle#146