Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[DependencyInjection] properly fix tests on PHP 5#29792
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
xabbuh commentedJan 5, 2019
| Q | A |
|---|---|
| Branch? | 3.4 |
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | |
| License | MIT |
| Doc PR |
chalasr commentedJan 5, 2019
Thank you@xabbuh. |
This PR was merged into the 3.4 branch.Discussion----------[DependencyInjection] properly fix tests on PHP 5| Q | A| ------------- | ---| Branch? | 3.4| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets || License | MIT| Doc PR |Commits-------c8ced3a properly fix tests on PHP 5
przemyslaw-bogusz commentedJan 21, 2019
While running Travis CI tests with PHP 5.5 (build 67951.2) for PR#29935 I got:
I thinkFactoryReturnTypePassTest on branch 3.4 should check PHP version and skip the test if necessary, same as it does with hhvm. If that's true, I can make the relevant PR. |
xabbuh commentedJan 21, 2019
@przemyslaw-bogusz Can you link to part of the job you refer to (you can click on a line number on a left in the Travis CI output)? I do not find anything like that. |
przemyslaw-bogusz commentedJan 21, 2019
It seems that the test ran again just a few minutes ago and now the error is gone. Next time I will make a screenshot. Sorry for disturbing you. |
xabbuh commentedJan 21, 2019
Nothing to worry about. :) Just by reading the code I also don't see where the test would be skipped on PHP 5.5. But we seem to miss something as the test apparently passes right now. :) |
przemyslaw-bogusz commentedJan 21, 2019
As I understand this PR was about fixing tests due to the fact, that Symfony 3.4 can run on PHP 5, which means that tests from branch 3.4 should take into account, that return type declarations were introduced in PHP 7. I thought that was why the test failed, as it hinted at symfony/src/Symfony/Component/DependencyInjection/Tests/Fixtures/FactoryDummy.php Lines 16 to 18 indb6784b
But now, since suddenly the Travis CI test passes, I'm confused. |
przemyslaw-bogusz commentedJan 21, 2019
@xabbuh I wear glasses so that's the main problem. The failed Travis CI test is in#29944. Here's link to the test: |
xabbuh commentedJan 21, 2019
@przemyslaw-bogusz looks reasonable to me, could you do the PR? |
przemyslaw-bogusz commentedJan 21, 2019
@xabbuh I will do the PR |
przemyslaw-bogusz commentedJan 21, 2019
@xabbuh Could you please enlighten with one more thing? Since the problem resolves around using return type declarations in PHP 5 test environment, for the CI test to fail, it's enough thatFactoryReturnTypePassTest has a |