-
Notifications
You must be signed in to change notification settings - Fork 178
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
fix: PHP warnings triggered by the test suite #302
base: master
Are you sure you want to change the base?
fix: PHP warnings triggered by the test suite #302
Conversation
bd28e37
to
2270fae
Compare
2270fae
to
82caad1
Compare
433b083
to
8d6330e
Compare
@thePanz hey, I got it tests passes on GitHub action. Let me know whether this PR has any chance to be merged. |
8d6330e
to
e1e02b2
Compare
|
||
class_exists('Swift'); | ||
|
||
if (version_compare(Swift::VERSION, '6.0.0') >= 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This approach is a bit like https://github.com/FriendsOfSymfony1/symfony1/pull/237/files but way more elegant.
Question is, if we are able to support swiftmailer 6 including the doctrine spools, do we really need to support swiftmailer 5?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question, I guess not.
But it depends, on the ability of applications using swiftmailer 5 to migrate to swiftmailer 6.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This opens a path to add support for symfony/mailer
.
lib/config/autoload/swift.php
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do like this approach to support the files being loaded by composer.
@@ -8,6 +8,10 @@ | |||
* file that was distributed with this source code. | |||
*/ | |||
|
|||
if (!defined('SF_TEST_WITHOUT_COMPOSER')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
e1e02b2
to
b8b25b3
Compare
The test should fail when a warning PHP error occurred.
Depends on
How to execute tests?
To make test passes, I apply the following dependencies:
Checkout the "bundle": https://github.com/alquerci/symfony1/tree/fixtest-fail-on-php-errors--dev
Then execute:
What is done?
error_reporting
to catch from PHP warnings and higher errors.sfMailer
transport forSwitf
v6 isSwift_SmtpTransport
instead ofSwift_MailTransport
.Swift_MailTransport
as been removed.data/bin/symfony symfony:test --trace --update-autoloader
What about the CI?
I try to put a PHP error_reporting directive, but the execution environment looks like different than docker.
Next steps