-
Notifications
You must be signed in to change notification settings - Fork 154
BC break - Trying to configure method "getDefinitions" which cannot be configured #299
Comments
Please provide a minimal, self-contained, reproducing test case. |
I think this might be related to the fact that we call See https://github.com/symfony/symfony/blob/v2.3.39/src/Symfony/Bridge/Doctrine/Tests/Form/DoctrineOrmTypeGuesserTest.php for such a failing test:
|
@stof I do not have time to look at "real world" tests. Please provide a minimal, self-contained, reproducing test case. I need something that I can copy/paste from this issue into a file for running and debugging. The changes made in ec7832e are not supposed to disallow anything that used to work before. They were made to provide useful error messages when trying to configure methods that cannot be configured (see #296). |
As a hotfix, I have tagged b90749d as |
@sebastianbergmann thanks |
@stof While I am waiting for a minimal, self-contained, reproducing test case, I have briefly looked at https://github.com/symfony/symfony/blob/v2.3.39/src/Symfony/Bridge/Doctrine/Tests/Form/DoctrineOrmTypeGuesserTest.php. When I put That means that |
This PR was merged into the 2.3 branch. Discussion ---------- [ci] Tweak scripts | Q | A | ------------- | --- | Branch? | 2.3 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | sebastianbergmann/phpunit-mock-objects#299 | License | MIT | Doc PR | - Commits ------- 82c6bed [ci] Tweak scripts
Just to be clear: the behavior described in #299 (comment) is expected. I do not know why the |
this looks weird. |
actually, I know why you get false: it is an interface, not a class |
You're right, of course, and with |
I think I have found the root cause: the simple/naive In the case of https://github.com/symfony/symfony/blob/v2.3.39/src/Symfony/Bridge/Doctrine/Tests/Form/DoctrineOrmTypeGuesserTest.php, Yay! for PHP's case insensitivity. |
After 75d58e3 the tests in https://github.com/symfony/symfony/blob/v2.3.39/src/Symfony/Bridge/Doctrine/Tests/Form/DoctrineOrmTypeGuesserTest.php work for me. |
I now get the following errors:
As there are still |
I stopped debugging after looking at the first remaining error because |
For Symfony\Component\Templating\Tests\DelegatingEngineTest::testStreamRequiresStreamingEngine, the issue is a bug in the Symfony testsuite. We are indeed trying to configure a non-existent method. I think it may be great to have a different error message when the method does not exist at all rather than existing but not configurable (final or private). It would make it easier to understand the mistake. |
I agree but I am afraid that this would require significant work. I will probably do it eventually (unless someone beats me to it with a pull request) but no promises as to when. |
@sebastianbergmann a first easy step might be to add more explanations in the exception message about what |
Attempted in 957b4b2 |
This PR was merged into the 3.0 branch. Discussion ---------- fix controller tests | Q | A | ------------- | --- | Branch? | 3.0 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | items 9 to 12 from sebastianbergmann/phpunit-mock-objects#299 (comment) | License | MIT | Doc PR | Add missing public method stubs to `TestController` (all methods in the base `Controller` class from the FrameworkBundle are `protected` since Symfony 3.0). Commits ------- fb963d2 fix controller tests
@stof @nicolas-grekas Is the current state of |
OK for me, we'll fix Symfony's tests to not mock undefined methods afterwards. Thanks |
This PR was merged into the 3.0 branch. Discussion ---------- [3.0] fix mocking of some methods | Q | A | ------------- | --- | Branch? | 3.0 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | items 1 and 2 of sebastianbergmann/phpunit-mock-objects#299 (comment) | License | MIT | Doc PR | Commits ------- 07308e9 [3.0] fix mocking of some methods
This PR was merged into the 2.3 branch. Discussion ---------- [2.3] fix mocking of some methods | Q | A | ------------- | --- | Branch? | 2.3 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? |no | Tests pass? | yes | Fixed tickets | items 3, 4, 5, and 7 of sebastianbergmann/phpunit-mock-objects#299 (comment) | License | MIT | Doc PR | Commits ------- 542cf6b [2.3] fix mocking of some methods
Thank you for bringing this to my attention, @nicolas-grekas and @stof, and for helping to resolve it. |
Sidenote: with PHPUnit 5.3, due on April 1, these errors will be warnings:
|
This PR was merged into the 2.7 branch. Discussion ---------- [2.7] fix mocking of some methods | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | items 6 and 8 of sebastianbergmann/phpunit-mock-objects#299 (comment) | License | MIT | Doc PR | Commits ------- a45b93d [2.7] fix mocking of some methods
These methods do not actually exist on the mock object, which means we will get an error anyway if they are called. Recent versions of phpunit forbid configuring inexistant methods in mocks, which makes tests fail. See sebastianbergmann/phpunit-mock-objects#299
This PR was merged into the 2.3 branch. Discussion ---------- [phpunit] disable prophecy | Q | A | ------------- | --- | Branch? | 2.3 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Both Prophecy and Symfony require some phpdocumentor dependencies, but incompatible versions. Since we don't use Prophecy, let's disable it. phpunit-mock-objects is forced to v3.1.1 until we fix our test suite (see sebastianbergmann/phpunit-mock-objects#299). Commits ------- ae9bae7 [phpunit] disable prophecy
The symfony test suite does not work with ec7832e
with a phpunit that has been checked out and composer-installed from source
e.g.
phpunit src/Symfony/Component/HttpKernel/
displays:The text was updated successfully, but these errors were encountered: