[Tests] Streamline #52365

OskarStark · 2023-10-30T10:16:43Z

Q	A
Branch?	5.4
Bug fix?	no
New feature?	no
Deprecations?	no
Issues	--
License	MIT

Extracted from #52157

OskarStark · 2023-10-30T10:18:40Z

src/Symfony/Bundle/FrameworkBundle/Tests/Controller/TemplateControllerTest.php

+    public function testNoTwigTemplateActionMethod()
    {
+        $controller = new TemplateController();
+
        $this->expectException(\LogicException::class);
        $this->expectExceptionMessage('You cannot use the TemplateController if the Twig Bundle is not available.');
-        $controller = new TemplateController();

        $controller->templateAction('mytemplate')->getContent();
+    }
+
+    public function testNoTwigInvokeMethod()
+    {
+        $controller = new TemplateController();
+
+        $this->expectException(\LogicException::class);
+        $this->expectExceptionMessage('You cannot use the TemplateController if the Twig Bundle is not available.');
+
        $controller('mytemplate')->getContent();
    }


We should test both ways independently

OskarStark · 2023-10-30T10:19:04Z

src/Symfony/Bundle/SecurityBundle/Tests/Functional/SecurityRoutingIntegrationTest.php

@@ -125,8 +125,7 @@ public function testInvalidIpsInAccessControl()
        $this->expectException(\LogicException::class);
        $this->expectExceptionMessage('The given value "256.357.458.559" in the "security.access_control" config option is not a valid IP address.');

-        $client = $this->createClient(['test_case' => 'StandardFormLogin', 'root_config' => 'invalid_ip_access_control.yml']);
-        $client->request('GET', '/unprotected_resource');
+        $this->createClient(['test_case' => 'StandardFormLogin', 'root_config' => 'invalid_ip_access_control.yml']);


Exception is thrown while creating the Client, no need to call request()

OskarStark · 2023-10-30T10:19:21Z

src/Symfony/Bundle/WebProfilerBundle/Tests/Controller/ProfilerControllerTest.php

@@ -342,7 +342,7 @@ public function testPhpinfoActionWithProfilerDisabled()
        $twig = $this->createMock(Environment::class);

        $controller = new ProfilerController($urlGenerator, null, $twig, []);
-        $controller->phpinfoAction(Request::create('/_profiler/phpinfo'));
+        $controller->phpinfoAction();


phpInfoAction has no parameter

src/Symfony/Component/BrowserKit/Tests/AbstractBrowserTest.php

src/Symfony/Component/Cache/Tests/Adapter/RedisAdapterSentinelTest.php

src/Symfony/Component/Console/Tests/ApplicationTest.php

OskarStark · 2023-10-30T10:27:37Z

Ready to merge from my side

OskarStark · 2023-10-31T07:07:36Z

@nicolas-grekas as PRs get bigger and bigger, can you please merge/upmerge this one? Thanks

src/Symfony/Component/Console/Tests/Helper/TableTest.php

OskarStark · 2023-10-31T07:22:29Z

src/Symfony/Component/Console/Tests/Helper/TableTest.php

-        $table = new Table($output = $this->getOutputStream());
+        $table = new Table($this->getOutputStream());


Unused variable $output

OskarStark · 2023-10-31T07:27:31Z

src/Symfony/Component/Console/Tests/Helper/TableTest.php

        $table
            ->setHeaders(['ISBN', 'Title', 'Author', 'Price'])
            ->setRows([
                ['99921-58-10-7', [], 'Dante Alighieri', '9.95'],
            ]);

+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage('A cell must be a TableCell, a scalar or an object implementing "__toString()", "array" given.');
+
        $table->render();


@stof it seems, that this is no dead code, as the exception is raised when calling render() and not when calling setRows() 👍

then in the other PR, keep setRows in initialization

OskarStark · 2023-10-31T07:35:04Z

Fabbot looks unrelated

nicolas-grekas · 2023-10-31T07:58:46Z

Thank you @OskarStark.

…e expectation to avoid false positives (OskarStark) This PR was merged into the 6.4 branch. Discussion ---------- [Tests] Move `expectException` closer to the place of the expectation to avoid false positives | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | -- | License | MIT If you guys like this PR, I will fix the other places as well Should be merged after merge & upmerge of: * #52365 Commits ------- 6115ab0 [Tests] Move expectException closer to the place of the expectation to avoid false positives

This PR was squashed before being merged into the 7.1 branch. Discussion ---------- [Tests] Streamline | Q | A | ------------- | --- | Branch? | 7.1 | Bug fix? | no | New feature? | no | Deprecations? | no | Issues | -- | License | MIT Follows * #52157 * #52365 Commits ------- 33d71aa [Tests] Streamline

OskarStark requested a review from chalasr as a code owner October 30, 2023 10:16

carsonbot added the Status: Needs Review label Oct 30, 2023

carsonbot added this to the 5.4 milestone Oct 30, 2023

OskarStark requested a review from nicolas-grekas October 30, 2023 10:17

OskarStark commented Oct 30, 2023

View reviewed changes

src/Symfony/Component/BrowserKit/Tests/AbstractBrowserTest.php Outdated Show resolved Hide resolved

OskarStark commented Oct 30, 2023

View reviewed changes

src/Symfony/Component/Cache/Tests/Adapter/RedisAdapterSentinelTest.php Show resolved Hide resolved

OskarStark commented Oct 30, 2023

View reviewed changes

src/Symfony/Component/Console/Tests/ApplicationTest.php Outdated Show resolved Hide resolved

OskarStark commented Oct 30, 2023

View reviewed changes

src/Symfony/Component/Console/Tests/ApplicationTest.php Outdated Show resolved Hide resolved

stof approved these changes Oct 30, 2023

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Oct 30, 2023

OskarStark mentioned this pull request Oct 30, 2023

[Tests] Move expectException closer to the place of the expectation to avoid false positives #52157

Merged

yceruto approved these changes Oct 31, 2023

View reviewed changes

OskarStark force-pushed the fix/tests-4 branch from b98d3c4 to bf71742 Compare October 31, 2023 06:54

OskarStark requested a review from xabbuh as a code owner October 31, 2023 07:12

carsonbot added Status: Needs Review and removed Status: Reviewed labels Oct 31, 2023