Skip to content

Commit

Permalink
Fix build
Browse files Browse the repository at this point in the history
  • Loading branch information
@Seldaek
Seldaek committed Jul 4, 2013
1 parent 65c5257 commit 2a87244
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 50 deletions.
2 changes: 1 addition & 1 deletion Tests/Fixtures/Form/TestType.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,6 @@ public function setDefaultOptions(OptionsResolverInterface $resolver)

public function getName()
{
return 'test_type';
return '';
}
}
12 changes: 6 additions & 6 deletions Tests/Formatter/MarkdownFormatterTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,18 +81,18 @@ public function testFormat()
#### Parameters ####
test_type[a]:
a:
* type: string
* required: true
* description: A nice description
test_type[b]:
b:
* type: string
* required: false
test_type[c]:
c:
* type: boolean
* required: true
Expand All @@ -109,18 +109,18 @@ public function testFormat()
#### Parameters ####
test_type[a]:
a:
* type: string
* required: true
* description: A nice description
test_type[b]:
b:
* type: string
* required: false
test_type[c]:
c:
* type: boolean
* required: true
Expand Down
18 changes: 6 additions & 12 deletions Tests/Formatter/SimpleFormatterTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,22 +104,19 @@ public function testFormat()
'description' => 'create test',
'parameters' =>
array(
'test_type[a]' =>
array(
'a' => array(
'dataType' => 'string',
'required' => true,
'description' => 'A nice description',
'readonly' => false,
),
'test_type[b]' =>
array(
'b' => array(
'dataType' => 'string',
'required' => false,
'description' => '',
'readonly' => false,
),
'test_type[c]' =>
array(
'c' => array(
'dataType' => 'boolean',
'required' => true,
'description' => '',
Expand Down Expand Up @@ -147,22 +144,19 @@ public function testFormat()
'description' => 'create test',
'parameters' =>
array(
'test_type[a]' =>
array(
'a' => array(
'dataType' => 'string',
'required' => true,
'description' => 'A nice description',
'readonly' => false,
),
'test_type[b]' =>
array(
'b' => array(
'dataType' => 'string',
'required' => false,
'description' => '',
'readonly' => false,
),
'test_type[c]' =>
array(
'c' => array(
'dataType' => 'boolean',
'required' => true,
'description' => '',
Expand Down
69 changes: 38 additions & 31 deletions Tests/Parser/FormTypeParserTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,38 +32,45 @@ public function testParse($typeName, $expected)
public function dataTestParse()
{
return array(
array('Nelmio\ApiDocBundle\Tests\Fixtures\Form\TestType', array(
'a' => array(
'dataType' => 'string',
'required' => true,
'description' => 'A nice description',
'readonly' => false
),
'b' => array(
'dataType' => 'string',
'required' => true,
'description' => '',
'readonly' => false
),
'c' => array(
'dataType' => 'boolean',
'required' => true,
'description' => '',
'readonly' => false
))
array(
array('class' => 'Nelmio\ApiDocBundle\Tests\Fixtures\Form\TestType'),
array(
'a' => array(
'dataType' => 'string',
'required' => true,
'description' => 'A nice description',
'readonly' => false
),
'b' => array(
'dataType' => 'string',
'required' => true,
'description' => '',
'readonly' => false
),
'c' => array(
'dataType' => 'boolean',
'required' => true,
'description' => '',
'readonly' => false
)
)
),
array('Nelmio\ApiDocBundle\Tests\Fixtures\Form\CollectionType', array(
'a' => array(
'dataType' => 'array of strings',
'required' => true,
'description' => '',
'readonly' => false
), 'b' => array(
'dataType' => 'string',
'required' => true,
'description' => '',
'readonly' => false
))
array(
array('class' => 'Nelmio\ApiDocBundle\Tests\Fixtures\Form\CollectionType'),
array(
'collection_type[a]' => array(
'dataType' => 'array of strings',
'required' => true,
'description' => '',
'readonly' => false
),
'collection_type[b]' => array(
'dataType' => 'string',
'required' => true,
'description' => '',
'readonly' => false
)
)
),
);
}
Expand Down

8 comments on commit 2a87244

@willdurand
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again? It looks like we change this stuff every month..

@Seldaek
Copy link
Member Author

@Seldaek Seldaek commented on 2a87244 Jul 4, 2013 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@willdurand
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't know, but we already merged 2 or 3 PRs related to this stuff :(

@Seldaek
Copy link
Member Author

@Seldaek Seldaek commented on 2a87244 Jul 4, 2013 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@willdurand
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Roger!

@stof
Copy link
Contributor

@stof stof commented on 2a87244 Jul 5, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is testing using invalid data.

The big issue is that this bundle makes a wrong assumption about the Form component. It is impossible to build a form knowing only the type. you need 2 things: the type and the root name. The bundle switched from using an empty name to using the name of the type as form name (which is what is done by default when you use the createForm shortcut).

The test is flawed as it uses an empty type name, which is wrong (type names need to be unique in the whole project so you cannot use an empty type name to expect an empty form name).

@Seldaek
Copy link
Member Author

@Seldaek Seldaek commented on 2a87244 Jul 8, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stof is the Type name really required? I thought it was only a recommendation to avoid conflicts if you use multiple forms in one same action, but as far as I know it doesn't prevent anything from working.

@stof
Copy link
Contributor

@stof stof commented on 2a87244 Jul 8, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is required as it is the identifier of the type in the FormRegistry. the type name is not the same than the form name (even if the createForm shortcut uses the type name as name of the form)

Please sign in to comment.