[DependencyInjection] Fix regression in ordering service locators by priority #57345
Conversation
|
Hey! I see that this is your first PR. That is great! Welcome! Symfony has a contribution guide which I suggest you to read. In short:
Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change. When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor! I am going to sit back now and wait for the reviews. Cheers! Carsonbot |
c9c25c2 to
2b098d1
Compare
|
I tried to rebase onto 7.0 but there is a conflict - if you want me to rebase anyway or open a separate PR for 7.0 let me know. |
|
I had a quick look at the git history: #42532 removed this ksort, but it's still in 5.4? |
|
The break was introduced in 7.0 in #50578. Refactoring was done which somehow reintroduced the ksort, which broke the tests, and the test cases were altered to the ksorted order instead of finding/fixing the source of the break. The test coverage was incorrectly changed in cc4ef49#diff-2d095e6b8882e9c562ea8541b1c0c995b8fd438946e73b4bd4a74f9895a47366L215-R215 but the services should be returned in the same order they were added, and not be sorted. Similar changes to IntegrationTest also show this regression where the services are alphabetically sorted but should not be: cc4ef49#diff-4440c0daf07aa75a97e4cf9f147391d566ebaa04fe772bdf1c896f92f6292f38 |
…g (seriquynh) This PR was merged into the 5.4 branch. Discussion ---------- [Console] Test convert CompletionInput into string | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | no | New feature? | no | Deprecations? | no | Issues | N/A | License | MIT CompletionInput can be converted into string but there are no tests for it. So, I a simple test here based on it's current behavior. Commits ------- 0a7477d Test convert CompletionInput into string
…rekas) This PR was merged into the 5.4 branch. Discussion ---------- [Filesystem][Mime] Fix transient tests | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | no | New feature? | no | Deprecations? | no | Issues | - | License | MIT HttpClient already uses this port so that when running concurrently, we get 404s for the logo. Commits ------- c3bf9a6 [Filesystem][Mime] Fix transient tests
* 5.4: [Filesystem][Mime] Fix transient tests Test convert CompletionInput into string
* 6.4: [Filesystem][Mime] Fix transient tests Test convert CompletionInput into string
2b098d1 to
d4b812c
Compare
|
Thank you @longwave. |
Fixes a regression in Symfony 7.0 where service locators are no longer ordered by priority.