8000 [Form] fixed DateTimeType tests using "date_widget"="choice". by adrienlucas · Pull Request #17248 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[Form] fixed DateTimeType tests using "date_widget"="choice". #17248

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

Closed

Conversation

adrienlucas
Copy link
Contributor
Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets no
License MIT
Doc PR no

As said in the documentation, the default years available in the DateType go from five year before to five year after the current year. So now that it's 2016 all around, the tests which use the "date_widget"="choice" without defining a "years" option and tries to submit a date in 2010 are failing because 2010 is not in the default choice list.

This PR fixes the issue by setting the "years" option to "2010" for each test that need it. It also remove unnecessary (and confusing) dummy data from some tests which were suffering the same issue but silently.

Edit : The PHP 5.6 build on travis seems to fetch and run some old version of the tests. So it still fail on this build but not on the others anymore.

@adrienlucas adrienlucas force-pushed the fix-form-datetime-test branch from 3a8fc99 to fffc6fa Compare January 3, 2016 21:37
@xabbuh
Copy link
Member
xabbuh commented Jan 4, 2016

👍 (should be merged into the 2.3 branch though)

Status: Reviewed

'time_widget' => 'choice',
'input' => 'datetime',
'with_minutes' => false,
));

$form->setData(new \DateTime('2010-06-02 03:04:05 UTC'));
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you make this change? Doesn't it change the test?

Copy link
Contributor

Choose a reason for hiding this comment

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

Nvm, it's clear after re-reading your PR :) 👍

@stof
Copy link
Member
stof commented Jan 4, 2016

👍

@Tobion
Copy link
Contributor
Tobion commented Jan 4, 2016
8000

Good catch, thanks @adrienlucas.

Tobion added a commit that referenced this pull request Jan 4, 2016
…ice". (Adrien LUCAS)

This PR was submitted for the master branch but it was merged into the 2.3 branch instead (closes #17248).

Discussion
----------

[Form] fixed DateTimeType tests using "date_widget"="choice".

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

As said in the [documentation](http://symfony.com/doc/current/reference/forms/types/datetime.html#years), the default years available in the ``DateType`` go from five year before to five year after the current year. So now that it's 2016 all around, the tests which use the ``"date_widget"="choice"`` without defining a ``"years"`` option and tries to submit a date in 2010 are failing because 2010 is not in the default choice list.

This PR fixes the issue by setting the ``"years"`` option to ``"2010"`` for each test that need it. It also remove unnecessary (and confusing) dummy data from some tests which were suffering the same issue but silently.

Edit : The PHP 5.6 build on travis seems to fetch and run some old version of the tests. So it still fail on this build but not on the others anymore.

Commits
-------

be20e89 Fix Form's DateTimeType tests.
@Tobion Tobion closed this Jan 4, 2016
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.

6 participants
0