[Routing] apply deprecation triggers and fix tests #13361
Conversation
| @@ -118,7 +118,6 @@ public function testDumpForRouteWithDefaults() | |||
| public function testDumpWithSchemeRequirement() | |||
| { | |||
| $this->routeCollection->add('Test1', new Route('/testing', array(), array(), array(), '', array('ftp', 'https'))); | |||
| $this->routeCollection->add('Test2', new Route('/testing_bc', array(), array('_scheme' => 'https'))); // BC | |||
There was a problem hiding this comment.
All of these tests are useless because we already tested that logic inside the Route class that _scheme/_methods is backwards compatible and will result in the same route representation.
9a9485b to
6f1e26d
Compare
|
Ready |
|
@fabpot please also merge to master, so I can make a PR to remove the deprecated stuff. |
| @@ -38,26 +38,6 @@ public function testRedirectWhenNoSlash() | |||
| ); | |||
| } | |||
|
|
|||
| public function testSchemeRedirectBC() | |||
There was a problem hiding this comment.
Wouldn't it be better to prefix the test method's name with testLegacy instead of removing it?
There was a problem hiding this comment.
no, the test doesnt provide any value as I explained above
|
@Tobion there seems to be some remaining ones in the FrameworkBundle functional tests: https://travis-ci.org/symfony/symfony/jobs/46636638#L1759 |
|
@stof Yes the reason for this was hard to spot. But I found it and fixed it in a BC way. The problem was the container parameter resolving in the framework router that still accessed the deprecated requirements. |
|
👍 |
|
I still think that my suggestion in #13361 (comment) is legitimate. What do you think @fabpot ? |
… deprecated requirements while maintaining BC
92e7fa1 to
9af0ff2
Compare
…bion) This PR was merged into the 2.7 branch. Discussion ---------- [Routing] apply deprecation triggers and fix tests | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | [#12641, #12640, #12719] | License | MIT | Doc PR | Correctly add deprecation for _scheme and _method requirements and fix code to not trigger deprecations itself. And fix test so that they either explicitly test deprecated functionality or alternatively refactor tests to use new style. It also fixes the deprecation triggers for "pattern" which was not correctly done for YAML/XML loading. Commits ------- 9af0ff2 [FrameworkBundle] fix routing container param resolving to not access deprecated requirements while maintaining BC bd91867 [FrameworkBundle] remove superfluous test that is already covered in routing bc1c5c8 [Routing] apply deprecation triggers and fix tests
This was initially removed in symfony#13361 and accidentally put back in
This was initially removed in symfony#13361 and accidentally added again in symfony#11394.
This PR was merged into the 3.2 branch. Discussion ---------- [Routing] remove an unused routing fixture | 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 | This was initially removed in #13361 and accidentally added again in #11394. Commits ------- 6f67221 [Routing] remove an unused routing fixture
Correctly add deprecation for _scheme and _method requirements and fix code to not trigger deprecations itself. And fix test so that they either explicitly test deprecated functionality or alternatively refactor tests to use new style.
It also fixes the deprecation triggers for "pattern" which was not correctly done for YAML/XML loading.