Skip to content
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

Added more tests for WebProfilerBundle #31522

Merged
merged 1 commit into from Jan 9, 2020

Conversation

@javiereguiluz
Copy link
Member

javiereguiluz commented May 17, 2019

Q A
Branch? 3.4
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

Thanks to @jpauli Code Coverage info about Symfony (http://cov.jpauli.tech/) I found that WebProfiler's controllers are pretty badly covered:

image

This PR focuses on testing the main controller class:

image

@fabpot

This comment has been minimized.

Copy link
Member

fabpot commented May 18, 2019

@javiereguiluz Can you have a look at the tests, they seem broken.

@fabpot

This comment has been minimized.

Copy link
Member

fabpot commented Jul 3, 2019

@javiereguiluz Can you have a look at this PR? We cannot merge it as the tests are broken.

@javiereguiluz javiereguiluz force-pushed the javiereguiluz:tests_webprofiler branch from 3c7ac21 to e218edc Jul 8, 2019
@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Jul 18, 2019

Can you please rebase, squash, fix tests?

@javiereguiluz javiereguiluz force-pushed the javiereguiluz:tests_webprofiler branch from 4ddbc08 to 0140e3e Jul 18, 2019
@javiereguiluz

This comment has been minimized.

Copy link
Member Author

javiereguiluz commented Jul 18, 2019

I've rebased and squashed ... but I need help solving a failing test: https://travis-ci.org/symfony/symfony/jobs/560385179#L2602 Thanks.

@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Sep 26, 2019

Can you please rebase so that we can see fresh tests?

@javiereguiluz javiereguiluz force-pushed the javiereguiluz:tests_webprofiler branch from 0140e3e to e7dee4b Sep 27, 2019
@javiereguiluz

This comment has been minimized.

Copy link
Member Author

javiereguiluz commented Sep 27, 2019

I've rebased and fixed everything I could, but now I see this error:

  1. Symfony\Bundle\WebProfilerBundle\Tests\Controller\ProfilerControllerTest::testPanelActionWithWrongPanel
    TypeError: Argument 2 passed to Symfony\Bundle\TwigBundle\Controller\ExceptionController::showAction() must be an instance of Symfony\Component\Debug\Exception\FlattenException, instance of Symfony\Component\ErrorRenderer\Exception\FlattenException given, called in /home/travis/build/symfony/symfony/src/Symfony/Bundle/WebProfilerBundle/vendor/symfony/http-kernel/HttpKernel.php on line 146

https://travis-ci.org/symfony/symfony/jobs/590304523#L3318

@nicolas-grekas nicolas-grekas force-pushed the javiereguiluz:tests_webprofiler branch 2 times, most recently from ab326e3 to 55316ab Jan 9, 2020
@nicolas-grekas nicolas-grekas force-pushed the javiereguiluz:tests_webprofiler branch 2 times, most recently from 9884bb2 to 55309da Jan 9, 2020
@nicolas-grekas nicolas-grekas force-pushed the javiereguiluz:tests_webprofiler branch from 55309da to 2f7a820 Jan 9, 2020
@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Jan 9, 2020

Thank you @javiereguiluz.

nicolas-grekas added a commit that referenced this pull request Jan 9, 2020
This PR was merged into the 3.4 branch.

Discussion
----------

Added more tests for WebProfilerBundle

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | -   <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | -

Thanks to @jpauli Code Coverage info about Symfony (http://cov.jpauli.tech/) I found that WebProfiler's controllers are pretty badly covered:

![image](https://user-images.githubusercontent.com/73419/57919817-ec390500-7899-11e9-81b7-763a0b35d0ec.png)

This PR focuses on testing the main controller class:

![image](https://user-images.githubusercontent.com/73419/57919877-04108900-789a-11e9-8a93-3466b672d873.png)

Commits
-------

2f7a820 Added more tests for WebProfilerBundle
@nicolas-grekas nicolas-grekas merged commit 2f7a820 into symfony:3.4 Jan 9, 2020
2 of 3 checks passed
2 of 3 checks passed
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details
$crawler = $client->request('GET', '/_profiler/latest?panel=router');

$this->assertSame('_', $crawler->filter('.metrics .metric .value')->eq(0)->text());
$this->assertSame('12', $crawler->filter('.metrics .metric .value')->eq(1)->text());

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Jan 9, 2020

Member

I removed this assertion when merging in 4.4 because it doesn't pass and the patch is painful enough to merge up.
Feel free to submit it again on 4.4 with the proper assertion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.