Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[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

Merged
chalasr merged 1 commit intosymfony:3.4fromxabbuh:pr-29697
Jan 5, 2019

Conversation

@xabbuh
Copy link
Member

QA
Branch?3.4
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets
LicenseMIT
Doc PR

@chalasr
Copy link
Member

Thank you@xabbuh.

@chalasrchalasr merged commitc8ced3a intosymfony:3.4Jan 5, 2019
chalasr pushed a commit that referenced this pull requestJan 5, 2019
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
@xabbuhxabbuh deleted the pr-29697 branchJanuary 5, 2019 16:36
@przemyslaw-bogusz
Copy link
Contributor

While running Travis CI tests with PHP 5.5 (build 67951.2) for PR#29935 I got:

PHP Parse error: syntax error, unexpected ':', expecting ';' or '{' in /home/travis/build/symfony/symfony/src/Symfony/Component/DependencyInjection/Tests/Fixtures/FactoryDummy.php on line 16
Parse error: syntax error, unexpected ':', expecting ';' or '{' in /home/travis/build/symfony/symfony/src/Symfony/Component/DependencyInjection/Tests/Fixtures/FactoryDummy.php on line 16

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
Copy link
MemberAuthor

@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
Copy link
Contributor

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
Copy link
MemberAuthor

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
Copy link
Contributor

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: from the return type declaration.

publicstaticfunctioncreateFactory():FactoryDummy
{
}

But now, since suddenly the Travis CI test passes, I'm confused.

@przemyslaw-bogusz
Copy link
Contributor

@xabbuh I wear glasses so that's the main problem. The failed Travis CI test is in#29944. Here's link to the test:
https://travis-ci.org/symfony/symfony/jobs/482202091

@xabbuh
Copy link
MemberAuthor

@przemyslaw-bogusz looks reasonable to me, could you do the PR?

@przemyslaw-bogusz
Copy link
Contributor

@xabbuh I will do the PR

xabbuh reacted with thumbs up emoji

@przemyslaw-bogusz
Copy link
Contributor

@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 ause Symfony\Component\DependencyInjection\Tests\Fixtures\FactoryDummy;, which loads a class with a syntax error. Why Travis CI tests with PHP 5 for other PRs pass? Does it mean that not all PHPUnit tests are run on each PR?

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@chalasrchalasrchalasr approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

3.4

Development

Successfully merging this pull request may close these issues.

4 participants

@xabbuh@chalasr@przemyslaw-bogusz@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp