Skip to content

[Form] Fix tests #30972

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 7, 2019
Merged

[Form] Fix tests #30972

merged 1 commit into from
Apr 7, 2019

Conversation

chalasr
Copy link
Member

@chalasr chalasr commented Apr 7, 2019

Q A
Branch? 3.4
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

@chalasr chalasr added this to the 3.4 milestone Apr 7, 2019
@chalasr chalasr requested a review from xabbuh April 7, 2019 11:45
xabbuh
xabbuh reviewed Apr 7, 2019
View reviewed changes
src/Symfony/Component/Form/Tests/Extension/Core/Type/FileTypeTest.php Outdated
@@ -296,7 +296,7 @@ public function uploadFileErrorCodes()
{
return [
'no error' => [UPLOAD_ERR_OK, null],
'upload_max_filesize ini directive' => [UPLOAD_ERR_INI_SIZE, 'The file is too large. Allowed maximum size is {{ limit }} {{ suffix }}.'],
'upload_max_filesize ini directive' => [UPLOAD_ERR_INI_SIZE, 'The file is too large. Allowed maximum size is 2 MiB.'],
Copy link
Member

Choose a reason for hiding this comment

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

I think we need to add an ini_set() call for upload_max_filesize to have a deterministic failure.

Copy link
Member Author

@chalasr chalasr Apr 7, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

We should be able to inject a mocked translator in FileType actually, but it reveals another issue: we can't set the translator since FormFactoryBuilder instantiates the CoreExtension() on its own, there is no way to pass a translator or any other CoreExtension dependency. Just had a brainstorm with @stof and @HeahDude, we might need another PR...

@chalasr
Copy link
Member Author

chalasr commented Apr 7, 2019 •
edited
Loading

Needs #30961

@chalasr
Copy link
Member Author

chalasr commented Apr 7, 2019

ready

@nicolas-grekas
Copy link
Member

Thank you @chalasr.

@nicolas-grekas nicolas-grekas merged commit 35418be into symfony:3.4 Apr 7, 2019
nicolas-grekas added a commit that referenced this pull request Apr 7, 2019
@nicolas-grekas
minor #30972 [Form] Fix tests (chalasr)
cb5a255 
This PR was merged into the 3.4 branch.

Discussion
----------

[Form] Fix tests

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

<!--
Write a short README entry for your feature/bugfix here (replace this comment block.)
This will help people understand your PR and can be used as a start of the Doc PR.
Additionally:
 - Bug fixes must be submitted against the lowest branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too).
 - Features and deprecations must be submitted against the master branch.
-->

Commits
-------

35418be [Form] Fix tests
@chalasr chalasr deleted the form-fix-tests branch April 7, 2019 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xabbuh xabbuh xabbuh left review comments

@webmozart webmozart webmozart approved these changes

Assignees
No one assigned
Labels
Form Status: Reviewed
Projects
None yet
Milestone
3.4
Development

Successfully merging this pull request may close these issues.
5 participants
@chalasr @nicolas-grekas @webmozart @xabbuh @carsonbot