Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
Updated PHPUnit namespaces#21663
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
| * @author Nicolas Grekas <p@tchwork.com> | ||
| */ | ||
| class SymfonyTestsListenerextends\PHPUnit_Framework_BaseTestListener | ||
| class SymfonyTestsListenerextendsBaseTestListener |
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 think the bridge should not be updated to namespaces - only on master we should do that
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.
How about requiring a proper PHPUnit version? Actually the issue will be that PHPUnit < 5.7 does have another FC layer than 4.8. If we go that route we should enforce 4.8.35, 5.7 or 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.
there is already a conflict rule, should be enough, isnt'it?
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.
Yes, but we need to make it stricter. With my attempt in#21668 we might be able to make the bridge compatible with 4.8.35, 5.7 and 6.* It would be great if you could have a look, some tweaks are still needed.
nicolas-grekasFeb 19, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
What will using namespaces here provide at all on 2.8?
The bridge before 3.3 is still incompatible with phpunit 6 - using namespaces will only forbid its use with namespace-incompatible versions of phpunit.
#21668 is unrelated - it's only for master.
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.
Reverted as per your wish.
| namespaceSymfony\Bridge\PhpUnit; | ||
| useDoctrine\Common\Annotations\AnnotationRegistry; | ||
| usePHPUnit\Framework\TestCase; |
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.
should be reverted for the same reasons as the previous
peterrehm commentedFeb 20, 2017
Updated |
nicolas-grekas left a comment
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.
👍
nicolas-grekas commentedFeb 20, 2017
Thank you@peterrehm. |
This PR was squashed before being merged into the 2.8 branch (closes#21663).Discussion----------Updated PHPUnit namespaces| Q | A| ------------- | ---| Branch? | 2.8| Bug fix? | no| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR | -Follow Up of#21564Commits-------205ced4 Updated PHPUnit namespaces
nicolas-grekas commentedFeb 20, 2017
Same on 3.2 and master now? |
peterrehm commentedFeb 20, 2017
PR for 3.2 just provided. Once this is okay and merged I will look into master. |
This PR was merged into the 3.2 branch.Discussion----------Updated PHPUnit namespaces| Q | A| ------------- | ---| Branch? | 3.2| Bug fix? | no| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR | -Follow Up of#21663Commits-------c2e80e3 Updated PHPUnit namespaces
kobelobster commentedMar 6, 2017
One question@nicolas-grekas - When will this PR be merged and for which release is it planned? I saw that the 2.8.18 has been released today, so the next release will probably be in a month? Will this PR be also in the next release? Thanks! |
nicolas-grekas commentedMar 6, 2017
@tzfrs this is already released as part of 2.8.18 |
kobelobster commentedMar 6, 2017
Ah, I misunderstood the release page then Because it doesn't say here that this PR is included. Also, this PR is just closed and not merged, so I assumed it hasn't been integrated yet. |
nicolas-grekas commentedMar 6, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
That's because it's labelled as a "minor" - thus not listed in the changelog. If you look just above your previous comment, you'll see this has bee merged as commit13983d9 |
kobelobster commentedMar 6, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Ah okay thanks. However, I still get errors because of some code in the phpunit-bridge, but I saw you wrote here:#21694 (comment) that there is no plan of making 2.8 compatible with PhpUnit 6, right? So how would I use Symfony 2.8 with PhpUnit 6.0. Isn't it possible at this time and never will? |
xabbuh commentedMar 6, 2017
@tzfrs It's just the PhpUnitBridge for which you need to require the latest versions ( |
kobelobster commentedMar 6, 2017
Ah okay, thought I need it to have it at the same version as symfony. Ty. |
xabbuh commentedMar 6, 2017
The bridge is a bit special here. By the way, we also use the latest version of the bridge to run tests for Symfony on all branches. And that does work. ;) |
Follow Up of#21564