-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[FrameworkBundle] WebTestCase KernelBrowser::getContainer null return type #31202
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FrameworkBundle] WebTestCase KernelBrowser::getContainer null return type #31202
Conversation
3d11b16
to
b7e5040
Compare
b7e5040
to
6e010c3
Compare
Wouldn't it make sense to move that deprecation into the kernel? 🤔 |
6e010c3
to
4bc8b7a
Compare
PR Updated. |
5e143c1
to
54428ce
Compare
There are alot of deprecation with this being deprecated directly in the Kernel, I see 2 way of fixing this:
WDYT ? |
is there a legit case that relies on a nulled container? We should check before shipping :) in general i think tests should be fixed yes, unless it tests a behavior that becomes irrelevant in 5.0; then it's a legacy test. |
The deprecations seem to be triggered by the |
d6e58b3
to
bd6c8b0
Compare
Status: Needs Review |
a522f61
to
1b67876
Compare
Test fixed ! |
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.
with minor comment
7a2b34e
to
50eda4b
Compare
PR Rebased |
50eda4b
to
e169e1a
Compare
PR rebased, ready. |
Thank you @Simperfit. |
…ner null return type (Simperfit) This PR was merged into the 4.4 branch. Discussion ---------- [FrameworkBundle] WebTestCase KernelBrowser::getContainer null return type | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | no <!-- don't forget to update src/**/CHANGELOG.md files --> | BC breaks? | no <!-- see https://symfony.com/bc --> | Deprecations? | yes <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tests pass? | yes <!-- please add some, will be required by reviewers --> | Fixed tickets | #25920 <!-- #-prefixed issue number(s), if any --> | License | MIT | Doc PR | <!-- required for new features --> <!-- Write a short README entry for your feature/bugfix here (replace this comment block.) This will help people understand your PR and can be used as a start of the Doc PR. Additionally: - Bug fixes must be submitted against the lowest branch where they apply (lowest branches are regularly merged to upper ones so they get the fixes too). - Features and deprecations must be submitted against the master branch. --> As @stof suggested in the #25920 I started deprecating the behaviour of returning null when the container is non booted / null. If this is not wanted we should close both the PR and the issue ;). Commits ------- e169e1a [FrameworkBundle] WebTestCase KernelBrowser::getContainer null return type
…eal one instead (nicolas-grekas) This PR was merged into the 4.4 branch. Discussion ---------- [FrameworkBundle] Don't reset the test container but the real one instead | Q | A | ------------- | --- | Branch? | 4.4 for features / 3.4 or 4.3 for bug fixes <!-- see below --> | Bug fix? | yes/no | New feature? | yes/no <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | yes/no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tickets | Fix #... <!-- prefix each issue number with "Fix #", if any --> | License | MIT | Doc PR | - After #31202 and #32056, the tearDown method keeps throwing deprecation notices about "Getting the container from a non-booted kernel". The reason is that resetting the test-container calls `$kernel->getContainer()` while the kernel has been shut down. This fixes it and a few other glitches found meanwhile. Commits ------- 8e16143 [FrameworkBundle] Dont reset the test container but the real one instead
Pretty nasty BC break between 4.3 and 4.4, took me 2 hours to figure out 😫 This is the fix if anyone needs it: https://github.com/Symplify/Symplify/pull/1681/files |
Since 5.0 the parent implementation throws an exception anyway making the overwrites obsolete, see symfony#31202
Since 5.0 the parent implementation throws an exception anyway making the overwrites obsolete, see symfony#31202
…s (Tobion) This PR was merged into the 5.0 branch. Discussion ---------- [FrameworkBundle] remove getContainer overwrites in tests | Q | A | ------------- | --- | Branch? | 5.0 | Bug fix? | no | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tickets | | Doc PR | Since 5.0 the parent implementation throws an exception anyway making the overwrites obsolete, see #31202 Commits ------- 5ef9390 remove getContainer overwrites in tests
Since 5.0 the parent implementation throws an exception anyway making the overwrites obsolete, see symfony/symfony#31202
Since 5.0 the parent implementation throws an exception anyway making the overwrites obsolete, see symfony/symfony#31202
Since 5.0 the parent implementation throws an exception anyway making the overwrites obsolete, see symfony#31202
Since 5.0 the parent implementation throws an exception anyway making the overwrites obsolete, see symfony#31202
As @stof suggested in the #25920 I started deprecating the behaviour of returning null when the container is non booted / null.
If this is not wanted we should close both the PR and the issue ;).