Skip to content
This repository was archived by the owner on Nov 27, 2020. It is now read-only.

Make sure tests are run with full error reporting #875

Merged
merged 1 commit into from
Nov 14, 2015
Merged

Conversation

Tobion
Copy link
Contributor

@Tobion Tobion commented Oct 22, 2015

This is also done in all symfony components. It makes sure that tests report all errors and warnings including strict ones.
Often this is not the case otherwise (because it's not part of the php defaults). The other advantage is that it makes tests more equal in PHP 5 and PHP 7. PHP 7 removed strict errors and changed them to warnings or notices. So in PHP 7 those errors might cause failures but they do not cause failures in PHP 5 when strict is ignored.

This is also done in all symfony components. It makes sure that tests report all errors and warnings including strict ones.
Often this is not the case otherwise. The other advantage is that it makes tests more equal in PHP 5 and PHP 7. PHP  7 removed strict errors and changed them to warnings or notices. So in PHP 7 those errors might cause failures but they do not cause failures in PHP 5 when strict is ignored.
@Tobion
Copy link
Contributor Author

Tobion commented Oct 28, 2015

@nicolas-grekas what do you think?

@fabpot
Copy link
Member

fabpot commented Oct 28, 2015

👍

@Tobion
Copy link
Contributor Author

Tobion commented Oct 28, 2015

I wonder how this interacts with https://github.com/symfony/symfony-standard/blob/2.8/app/autoload.php#L6 that is part of the bootstrapping?

@Pierstoval
Copy link
Contributor

@Tobion I think this line should be specific to entry points (app.php, app_dev.php and console) and not the autoloader 😕

@Tobion
Copy link
Contributor Author

Tobion commented Nov 13, 2015

@nicolas-grekas do you agree with this PR? And shouldn't we add symfony/phpunit-bridge to the require-dev section as well?

@nicolas-grekas
Copy link
Member

LGTM Yes. Adjoint the bridge is a good idea although not strictly required since it looks like that no deprecations are triggered for now

@Tobion
Copy link
Contributor Author

Tobion commented Nov 13, 2015

I just tested it with phpunit and the php config is executed before AND after the bootstrap file. So the error_reporting is -1 when calling autoload and is reset to -1 after it by phpunit. But it is not set for each test. This is strange behavior by phpunit. But at the end it doesn't really matter to us. There is no difference if error reporting is -1 or -1 & ~E_USER_DEPRECATED if you use the phpunit bridge.

So this is good to be merged.

@Tobion
Copy link
Contributor Author

Tobion commented Nov 14, 2015

For the phpunit-bridge I created #884

@fabpot
Copy link
Member

fabpot commented Nov 14, 2015

Thank you @Tobion.

@fabpot fabpot merged commit e159c84 into 2.3 Nov 14, 2015
fabpot added a commit that referenced this pull request Nov 14, 2015
This PR was merged into the 2.3 branch.

Discussion
----------

Make sure tests are run with full error reporting

This is also done in all symfony components. It makes sure that tests report all errors and warnings including strict ones.
Often this is not the case otherwise (because it's not part of the php defaults). The other advantage is that it makes tests more equal in PHP 5 and PHP 7. PHP  7 removed strict errors and changed them to warnings or notices. So in PHP 7 those errors might cause failures but they do not cause failures in PHP 5 when strict is ignored.

Commits
-------

e159c84 Make sure tests are run with full error reporting
@Tobion Tobion deleted the Tobion-patch-1 branch November 14, 2015 16:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants