Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

On SMv2 only canonicalized aliases make sense #69

Merged
merged 1 commit into from
May 3, 2016
Merged

On SMv2 only canonicalized aliases make sense #69

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions src/FormElementManager/FormElementManagerV2Polyfill.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,7 @@ class FormElementManagerV2Polyfill extends AbstractPluginManager
'image' => Element\Image::class,
'month' => Element\Month::class,
'monthselect' => Element\MonthSelect::class,
'MonthSelect' => Element\MonthSelect::class,
'multiCheckbox' => Element\MultiCheckbox::class,
'multiCheckBox' => Element\MultiCheckbox::class,
'multicheckbox' => Element\MultiCheckbox::class,
'number' => Element\Number::class,
'password' => Element\Password::class,
'radio' => Element\Radio::class,
Expand Down
17 changes: 17 additions & 0 deletions test/FormElementManagerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,4 +221,21 @@ public function testAddingInvokableCreatesAliasAndMapsClassToElementFactory()
$this->assertEquals(ElementFactory::class, $factories['zendtestformtestassetelementwithfilter']);
}
}

public function testAllAliasesShouldBeCanonicalized()
{
if (method_exists($this->manager, 'configure')) {
$this->markTestSkipped('Check canonicalized makes sense only on v2');
}

$r = new ReflectionProperty($this->manager, 'aliases');
$r->setAccessible(true);
$aliases = $r->getValue($this->manager);

foreach ($aliases as $name => $alias) {
$this->manager->get($name . ' ');
Copy link
Contributor

@zluiten zluiten May 2, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Quite expensive no? An assertion on lower casing should be sufficient I think. $this->assertSame(strtolower($name), $name);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found the bug because I was trying to get the element with the camel-cased key.
As such, the test should reflect the use case and treat the code in the best black-box way possible.
Checking the case of keys would be a total white-box test with no insight of why we are testing that way.

$this->manager->get(strtoupper($name));
$this->manager->get($name);
}
}
}