8000 [OptionsResolver] added test for allowed values and types that default to null by Tobion · Pull Request #13331 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[OptionsResolver] added test for allowed values and types that default to null #13331

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 26, 2015

Conversation

Tobion
Copy link
Contributor
@Tobion Tobion commented Jan 8, 2015
Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? no
Fixed tickets -
License MIT
Doc PR -

Added test to keep logic as experienced in #12809

@stof
Copy link
Member
stof commented Jan 8, 2015

The case experienced in #12809 is not specifically about null. It is about the default value being allowed or no (null just happens to be the default value in HWIOAuthBundle).

you will get the same BC break in 2.6 for any not allowed default value because of the bugfix (which was precisely about default values not being validated)

@Tobion
Copy link
Contributor Author
Tobion commented Jan 9, 2015

True, then it's even more important not to revert that. The tests I added here show exactly that both invalid values with setDefault() as well as with resolve() throw an exception.

@stof
Copy link
Member
stof commented Jan 9, 2015

but this is already tested (with the test using 42 as default value, that you edited in your PR

@Tobion
Copy link
Contributor Author
Tobion commented Jan 9, 2015

Yes but there has been no test where the invalid value is passed to resolve

@Tobion
Copy link
Contributor Author
Tobion commented Jan 13, 2015

ping

@Tobion
Copy link
Contributor Author
Tobion commented Jan 21, 2015

@stof you ok with the PR?

@stof
Copy link
Member
stof commented Jan 26, 2015

👍

@fabpot
Copy link
Member A3CB
fabpot commented Jan 26, 2015

Thank you @Tobion.

@fabpot fabpot merged commit 0b42cad into symfony:2.6 Jan 26, 2015
fabpot added a commit that referenced this pull request Jan 26, 2015
…s that default to null (Tobion)

This PR was merged into the 2.6 branch.

Discussion
----------

[OptionsResolver] added test for allowed values and types that default to null

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | no
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Added test to keep logic as experienced in #12809

Commits
-------

0b42cad [OptionsResolver] added test for allowed values and types that default to null
@Tobion Tobion deleted the test-for-12809 branch January 26, 2015 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0