-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[HttpKernel] Ensure the storage exists before purging it in ProfilerTest #11321
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
Conversation
penyaskito
commented
Jul 5, 2014
Q | A |
---|---|
Bug fix? | yes |
New feature? | no |
BC breaks? | no |
Deprecations? | no |
Tests pass? | yes |
Fixed tickets | #11319 |
License | MIT |
Doc PR | None |
@@ -79,8 +79,10 @@ protected function setUp() | |||
|
|||
protected function tearDown() | |||
{ | |||
$this->storage->purge(); | |||
$this->storage = null; | |||
if ($this->storage !== null) { |
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.
In Symfony code we usually opt for Yoda conditions, so this line should be:
if (null !== $this->storage) {
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.
Oops, fixed it! Thanks @javiereguiluz
Would it be possible to write a test for that? |
if (null !== $this->storage) { | ||
$this->storage->purge(); | ||
$this->storage = null; | ||
} | ||
|
||
@unlink($this->tmp); |
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.
can be skipped as well
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.
Added to the same if block, not sure if it should be a separate one.
@dawehner this is already a test. You don't write tests for tests. ^^ |
…ng it in ProfilerTest
👍 |
1 similar comment
👍 |
Thank you @penyaskito. |
… in ProfilerTest (penyaskito) This PR was merged into the 2.3 branch. Discussion ---------- [HttpKernel] Ensure the storage exists before purging it in ProfilerTest | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #11319 | License | MIT | Doc PR | None Commits ------- eb63270 bug #11319 [HttpKernel] Ensure the storage exists before purging it in ProfilerTest