diogo
/
symfony
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

bug #26675 [HttpKernel] DumpDataCollector: do not flush when a dumper is provided (ogizanagi) 

This PR was merged into the 2.7 branch.

Discussion
----------

[HttpKernel] DumpDataCollector: do not flush when a dumper is provided

| Q             | A
| ------------- | ---
| Branch?       | 2.7 <!-- see below -->
| Bug fix?      | yes
| New feature?  | no <!-- don't forget to update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- 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 | 3db14045d4/src/Symfony/Component/HttpKernel/DataCollector/DumpDataCollector.php (L208-L209)   <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | N/A

This explains [the workaround I initially used](3db14045d4/src/Symfony/Component/HttpKernel/DataCollector/DumpDataCollector.php (L208-L209)) in the server dumper PR original code.

I might be wrong on the intent, but as soon as a dumper is provided (e.g by setting  `debug.dump_destination: php://stderr`), I think there is no need to set the `DumpDataCollector::$isCollected` flag to `false` as we explicitly ask for the dump to be output directly somewhere. So ne need to output again on `__destruct`.

Spotted by running tests on the `symfony/demo` with the server dumper enabled: dumps were output twice. Once on the server, once at the end of the tests.

But this can be easily seen as well by using `debug.dump_destination: php://stderr` on `test` env:

```diff
diff --git a/src/Controller/BlogController.php b/src/Controller/BlogController.php
index e3e30aa..bf9744e 100644
--- a/src/Controller/BlogController.php
+++ b/src/Controller/BlogController.php
@@ -50,6 +50,7 @@ class BlogController extends AbstractController
      */
     public function index(int $page, string $_format, PostRepository $posts): Response
     {
+        dump(get_class($posts));
         $latestPosts = $posts->findLatest($page);

         // Every template name also has two extensions that specify the format and
```

### Before

```sh
vendor/bin/simple-phpunit --filter=BlogControllerTest::testIndex
PHPUnit 6.5.7 by Sebastian Bergmann and contributors.

Testing Project Test Suite
BlogController.php on line 53:
"App\Repository\PostRepository"
.                                                                   1 / 1 (100%)

Time: 3.34 seconds, Memory: 44.25MB

OK (1 test, 1 assertion)
BlogController.php on line 53:
"App\Repository\PostRepository"
```

### After

```sh
vendor/bin/simple-phpunit --filter=BlogControllerTest::testIndex
PHPUnit 6.5.7 by Sebastian Bergmann and contributors.

Testing Project Test Suite
BlogController.php on line 53:
"App\Repository\PostRepository"
.                                                                   1 / 1 (100%)

Time: 731 ms, Memory: 28.00MB

OK (1 test, 1 assertion)
```

Commits
-------

11a0392516 [HttpKernel] DumpDataCollector: do not flush when a dumper is provided
This commit is contained in:
Fabien Potencier 2018-03-30 09:47:09 +02:00
parent 4243db5c87 11a0392516
commit 98ee8ab800
2 changed files with 22 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/Symfony/Component/HttpKernel/DataCollector/DumpDataCollector.php
View File

@ -67,7 +67,7 @@ class DumpDataCollector extends DataCollector implements DataDumperInterface
if ($this->stopwatch) {
$this->stopwatch->start('dump');
}
if ($this->isCollected) {
if ($this->isCollected && !$this->dumper) {
$this->isCollected = false;
}

21
src/Symfony/Component/HttpKernel/Tests/DataCollector/DumpDataCollectorTest.php
View File

@ -16,6 +16,7 @@ use Symfony\Component\HttpKernel\DataCollector\DumpDataCollector;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\VarDumper\Cloner\Data;
use Symfony\Component\VarDumper\Dumper\CliDumper;
/**
* @author Nicolas Grekas <p@tchwork.com>
@ -130,4 +131,24 @@ EOTXT;
$this->assertSame("\"DumpDataCollectorTest.php on line {$line}:\"\n456\n", ob_get_clean());
}
}
public function testFlushNothingWhenDataDumperIsProvided()
{
$data = new Data(array(array(456)));
$dumper = new CliDumper('php://output');
$collector = new DumpDataCollector(null, null, null, null, $dumper);
ob_start();
$collector->dump($data);
$line = __LINE__ - 1;
if (\PHP_VERSION_ID >= 50400) {
$this->assertSame("DumpDataCollectorTest.php on line {$line}:\n456\n", ob_get_clean());
} else {
$this->assertSame("\"DumpDataCollectorTest.php on line {$line}:\"\n456\n", ob_get_clean());
}
ob_start();
$collector->__destruct();
$this->assertEmpty(ob_get_clean());
}
}