minor #4277 Improve exception expectations reliability (alexandre-dau…

…bois) This PR was merged into the 3.x branch. Discussion ---------- Improve exception expectations reliability This PR moves exception expectations the closest possible to the line intended to throw. It improves reliability of tests ensuring that the right code is throwing the expected exception and not something else (resulting in false-positive). This has been done many times across Symfony code base. Commits ------- 797e490 Improve exception expectations reliability
twigphp · Sep 4, 2024 · 83c5415 · 83c5415
2 parents fb2663e + 797e490
commit 83c5415
Show file tree

Hide file tree

Showing 13 changed files with 128 additions and 111 deletions.
diff --git a/tests/Cache/FilesystemTest.php b/tests/Cache/FilesystemTest.php
@@ -81,9 +81,6 @@ public function testWrite()
 
     public function testWriteFailMkdir()
     {
-        $this->expectException(\RuntimeException::class);
-        $this->expectExceptionMessage('Unable to create the cache directory');
-
         if (\defined('PHP_WINDOWS_VERSION_BUILD')) {
             $this->markTestSkipped('Read-only directories not possible on Windows.');
         }
@@ -97,14 +94,14 @@ public function testWriteFailMkdir()
         @mkdir($this->directory, 0555, true);
         $this->assertDirectoryExists($this->directory);
 
+        $this->expectException(\RuntimeException::class);
+        $this->expectExceptionMessage('Unable to create the cache directory');
+
         $this->cache->write($key, $content);
     }
 
     public function testWriteFailDirWritable()
     {
-        $this->expectException(\RuntimeException::class);
-        $this->expectExceptionMessage('Unable to write in the cache directory');
-
         if (\defined('PHP_WINDOWS_VERSION_BUILD')) {
             $this->markTestSkipped('Read-only directories not possible on Windows.');
         }
@@ -120,14 +117,14 @@ public function testWriteFailDirWritable()
         @mkdir($this->directory.'/cache', 0555);
         $this->assertDirectoryExists($this->directory.'/cache');
 
+        $this->expectException(\RuntimeException::class);
+        $this->expectExceptionMessage('Unable to write in the cache directory');
+
         $this->cache->write($key, $content);
     }
 
     public function testWriteFailWriteFile()
     {
-        $this->expectException(\RuntimeException::class);
-        $this->expectExceptionMessage('Failed to write cache file');
-
         $key = $this->directory.'/cache/cachefile.php';
         $content = $this->generateSource();
 
@@ -137,6 +134,9 @@ public function testWriteFailWriteFile()
         @mkdir($key, 0777, true);
         $this->assertDirectoryExists($key);
 
+        $this->expectException(\RuntimeException::class);
+        $this->expectExceptionMessage('Failed to write cache file');
+
         $this->cache->write($key, $content);
     }
 

diff --git a/tests/CustomExtensionTest.php b/tests/CustomExtensionTest.php
@@ -23,11 +23,12 @@ class CustomExtensionTest extends TestCase
      */
     public function testGetInvalidOperators(ExtensionInterface $extension, $expectedExceptionMessage)
     {
+        $env = new Environment(new ArrayLoader());
+        $env->addExtension($extension);
+
         $this->expectException(\InvalidArgumentException::class);
         $this->expectExceptionMessage($expectedExceptionMessage);
 
-        $env = new Environment(new ArrayLoader());
-        $env->addExtension($extension);
         $env->getUnaryOperators();
     }
 

diff --git a/tests/EnvironmentTest.php b/tests/EnvironmentTest.php
@@ -333,12 +333,12 @@ public function testAddMockExtension()
 
     public function testOverrideExtension()
     {
+        $twig = new Environment(new ArrayLoader());
+        $twig->addExtension(new EnvironmentTest_Extension());
+
         $this->expectException(\LogicException::class);
         $this->expectExceptionMessage('Unable to register extension "Twig\Tests\EnvironmentTest_Extension" as it is already registered.');
 
-        $twig = new Environment(new ArrayLoader());
-
-        $twig->addExtension(new EnvironmentTest_Extension());
         $twig->addExtension(new EnvironmentTest_Extension());
     }
 
@@ -370,11 +370,12 @@ public function testAddRuntimeLoader()
 
     public function testFailLoadTemplate()
     {
+        $template = 'testFailLoadTemplate.twig';
+        $twig = new Environment(new ArrayLoader([$template => false]));
+
         $this->expectException(RuntimeError::class);
         $this->expectExceptionMessage('Failed to load Twig template "testFailLoadTemplate.twig", index "112233": cache might be corrupted in "testFailLoadTemplate.twig".');
 
-        $template = 'testFailLoadTemplate.twig';
-        $twig = new Environment(new ArrayLoader([$template => false]));
         $twig->loadTemplate($twig->getTemplateClass($template), $template, 112233);
     }
 

diff --git a/tests/ExpressionParserTest.php b/tests/ExpressionParserTest.php
@@ -41,10 +41,10 @@ class ExpressionParserTest extends TestCase
      */
     public function testCanOnlyAssignToNames($template)
     {
-        $this->expectException(SyntaxError::class);
-
         $env = new Environment(new ArrayLoader(), ['cache' => false, 'autoescape' => false]);
         $parser = new Parser($env);
+
+        $this->expectException(SyntaxError::class);
         $parser->parse($env->tokenize(new Source($template, 'index')));
     }
 
@@ -84,10 +84,10 @@ public function testSequenceExpression($template, $expected)
      */
     public function testSequenceSyntaxError($template)
     {
-        $this->expectException(SyntaxError::class);
-
         $env = new Environment(new ArrayLoader(), ['cache' => false, 'autoescape' => false]);
         $parser = new Parser($env);
+
+        $this->expectException(SyntaxError::class);
         $parser->parse($env->tokenize(new Source($template, 'index')));
     }
 
@@ -210,12 +210,11 @@ public static function getTestsForSequence()
 
     public function testStringExpressionDoesNotConcatenateTwoConsecutiveStrings()
     {
-        $this->expectException(SyntaxError::class);
-
         $env = new Environment(new ArrayLoader(), ['cache' => false, 'autoescape' => false, 'optimizations' => 0]);
         $stream = $env->tokenize(new Source('{{ "a" "b" }}', 'index'));
         $parser = new Parser($env);
 
+        $this->expectException(SyntaxError::class);
         $parser->parse($stream);
     }
 
@@ -278,32 +277,30 @@ public static function getTestsForString()
 
     public function testAttributeCallDoesNotSupportNamedArguments()
     {
-        $this->expectException(SyntaxError::class);
-
         $env = new Environment(new ArrayLoader(), ['cache' => false, 'autoescape' => false]);
         $parser = new Parser($env);
 
+        $this->expectException(SyntaxError::class);
         $parser->parse($env->tokenize(new Source('{{ foo.bar(name="Foo") }}', 'index')));
     }
 
     public function testMacroCallDoesNotSupportNamedArguments()
     {
-        $this->expectException(SyntaxError::class);
-
         $env = new Environment(new ArrayLoader(), ['cache' => false, 'autoescape' => false]);
         $parser = new Parser($env);
 
+        $this->expectException(SyntaxError::class);
         $parser->parse($env->tokenize(new Source('{% from _self import foo %}{% macro foo() %}{% endmacro %}{{ foo(name="Foo") }}', 'index')));
     }
 
     public function testMacroDefinitionDoesNotSupportNonNameVariableName()
     {
-        $this->expectException(SyntaxError::class);
-        $this->expectExceptionMessage('An argument must be a name. Unexpected token "string" of value "a" ("name" expected) in "index" at line 1.');
-
         $env = new Environment(new ArrayLoader(), ['cache' => false, 'autoescape' => false]);
         $parser = new Parser($env);
 
+        $this->expectException(SyntaxError::class);
+        $this->expectExceptionMessage('An argument must be a name. Unexpected token "string" of value "a" ("name" expected) in "index" at line 1.');
+
         $parser->parse($env->tokenize(new Source('{% macro foo("a") %}{% endmacro %}', 'index')));
     }
 
@@ -312,12 +309,12 @@ public function testMacroDefinitionDoesNotSupportNonNameVariableName()
      */
     public function testMacroDefinitionDoesNotSupportNonConstantDefaultValues($template)
     {
-        $this->expectException(SyntaxError::class);
-        $this->expectExceptionMessage('A default value for an argument must be a constant (a boolean, a string, a number, a sequence, or a mapping) in "index" at line 1');
-
         $env = new Environment(new ArrayLoader(), ['cache' => false, 'autoescape' => false]);
         $parser = new Parser($env);
 
+        $this->expectException(SyntaxError::class);
+        $this->expectExceptionMessage('A default value for an argument must be a constant (a boolean, a string, a number, a sequence, or a mapping) in "index" at line 1');
+
         $parser->parse($env->tokenize(new Source($template, 'index')));
     }
 
@@ -359,67 +356,68 @@ public static function getMacroDefinitionSupportsConstantDefaultValues()
 
     public function testUnknownFunction()
     {
-        $this->expectException(SyntaxError::class);
-        $this->expectExceptionMessage('Unknown "cycl" function. Did you mean "cycle" in "index" at line 1?');
-
         $env = new Environment(new ArrayLoader(), ['cache' => false, 'autoescape' => false]);
         $parser = new Parser($env);
 
+        $this->expectException(SyntaxError::class);
+        $this->expectExceptionMessage('Unknown "cycl" function. Did you mean "cycle" in "index" at line 1?');
+
         $parser->parse($env->tokenize(new Source('{{ cycl() }}', 'index')));
     }
 
     public function testUnknownFunctionWithoutSuggestions()
     {
-        $this->expectException(SyntaxError::class);
-        $this->expectExceptionMessage('Unknown "foobar" function in "index" at line 1.');
-
         $env = new Environment(new ArrayLoader(), ['cache' => false, 'autoescape' => false]);
         $parser = new Parser($env);
 
+        $this->expectException(SyntaxError::class);
+        $this->expectExceptionMessage('Unknown "foobar" function in "index" at line 1.');
+
         $parser->parse($env->tokenize(new Source('{{ foobar() }}', 'index')));
     }
 
     public function testUnknownFilter()
     {
-        $this->expectException(SyntaxError::class);
-        $this->expectExceptionMessage('Unknown "lowe" filter. Did you mean "lower" in "index" at line 1?');
-
         $env = new Environment(new ArrayLoader(), ['cache' => false, 'autoescape' => false]);
         $parser = new Parser($env);
 
+        $this->expectException(SyntaxError::class);
+        $this->expectExceptionMessage('Unknown "lowe" filter. Did you mean "lower" in "index" at line 1?');
+
         $parser->parse($env->tokenize(new Source('{{ 1|lowe }}', 'index')));
     }
 
     public function testUnknownFilterWithoutSuggestions()
     {
-        $this->expectException(SyntaxError::class);
-        $this->expectExceptionMessage('Unknown "foobar" filter in "index" at line 1.');
-
         $env = new Environment(new ArrayLoader(), ['cache' => false, 'autoescape' => false]);
         $parser = new Parser($env);
 
+        $this->expectException(SyntaxError::class);
+        $this->expectExceptionMessage('Unknown "foobar" filter in "index" at line 1.');
+
         $parser->parse($env->tokenize(new Source('{{ 1|foobar }}', 'index')));
     }
 
     public function testUnknownTest()
     {
-        $this->expectException(SyntaxError::class);
-        $this->expectExceptionMessage('Unknown "nul" test. Did you mean "null" in "index" at line 1');
-
         $env = new Environment(new ArrayLoader(), ['cache' => false, 'autoescape' => false]);
         $parser = new Parser($env);
         $stream = $env->tokenize(new Source('{{ 1 is nul }}', 'index'));
+
+        $this->expectException(SyntaxError::class);
+        $this->expectExceptionMessage('Unknown "nul" test. Did you mean "null" in "index" at line 1');
+
         $parser->parse($stream);
     }
 
     public function testUnknownTestWithoutSuggestions()
     {
-        $this->expectException(SyntaxError::class);
-        $this->expectExceptionMessage('Unknown "foobar" test in "index" at line 1.');
-
         $env = new Environment(new ArrayLoader(), ['cache' => false, 'autoescape' => false]);
         $parser = new Parser($env);
 
+        $this->expectException(SyntaxError::class);
+        $this->expectExceptionMessage('Unknown "foobar" test in "index" at line 1.');
+
         $parser->parse($env->tokenize(new Source('{{ 1 is foobar }}', 'index')));
     }
 

diff --git a/tests/Extension/SandboxTest.php b/tests/Extension/SandboxTest.php
@@ -14,6 +14,7 @@
 use PHPUnit\Framework\TestCase;
 use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
 use Twig\Environment;
+use Twig\Error\RuntimeError;
 use Twig\Error\SyntaxError;
 use Twig\Extension\SandboxExtension;
 use Twig\Extension\StringLoaderExtension;
@@ -72,10 +73,11 @@ protected function setUp(): void
      */
     public function testSandboxForCoreTags(string $tag, string $template)
     {
+        $twig = $this->getEnvironment(true, [], self::$templates, []);
+
         $this->expectException(SecurityError::class);
         $this->expectExceptionMessageMatches(sprintf('/Tag "%s" is not allowed in "index \(string template .+?\)" at line 1/', $tag));
 
-        $twig = $this->getEnvironment(true, [], self::$templates, []);
         $twig->createTemplate($template, 'index')->render([]);
     }
 
@@ -124,10 +126,11 @@ public static function getSandboxedForExtendsAndUseTagsTests()
 
     public function testSandboxWithInheritance()
     {
+        $twig = $this->getEnvironment(true, [], self::$templates, ['extends', 'block']);
+
         $this->expectException(SecurityError::class);
         $this->expectExceptionMessage('Filter "json_encode" is not allowed in "1_child" at line 3.');
 
-        $twig = $this->getEnvironment(true, [], self::$templates, ['extends', 'block']);
         $twig->load('1_child')->render([]);
     }
 
@@ -441,14 +444,14 @@ public function testSandboxDisabledAfterIncludeFunctionError()
 
     public function testSandboxWithNoClosureFilter()
     {
-        $this->expectException('\Twig\Error\RuntimeError');
-        $this->expectExceptionMessage('The callable passed to the "filter" filter must be a Closure in sandbox mode in "index" at line 1.');
-
         $twig = $this->getEnvironment(true, ['autoescape' => 'html'], ['index' => <<<EOF
 {{ ["foo", "bar", ""]|filter("trim")|join(", ") }}
 EOF
         ], [], ['escape', 'filter', 'join']);
 
+        $this->expectException(RuntimeError::class);
+        $this->expectExceptionMessage('The callable passed to the "filter" filter must be a Closure in sandbox mode in "index" at line 1.');
+
         $twig->load('index')->render([]);
     }
 

diff --git a/tests/LexerTest.php b/tests/LexerTest.php
@@ -319,12 +319,12 @@ public function testStringWithHash()
 
     public function testStringWithUnterminatedInterpolation()
     {
+        $template = '{{ "bar #{x" }}';
+        $lexer = new Lexer(new Environment(new ArrayLoader()));
+
         $this->expectException(SyntaxError::class);
         $this->expectExceptionMessage('Unclosed """');
 
-        $template = '{{ "bar #{x" }}';
-
-        $lexer = new Lexer(new Environment(new ArrayLoader()));
         $lexer->tokenize(new Source($template, 'index'));
     }
 
@@ -388,9 +388,6 @@ public function testOperatorEndingWithALetterAtTheEndOfALine()
 
     public function testUnterminatedVariable()
     {
-        $this->expectException(SyntaxError::class);
-        $this->expectExceptionMessage('Unclosed "variable" in "index" at line 3');
-
         $template = '
 
 {{
@@ -401,14 +398,14 @@ public function testUnterminatedVariable()
 ';
 
         $lexer = new Lexer(new Environment(new ArrayLoader()));
+
+        $this->expectException(SyntaxError::class);
+        $this->expectExceptionMessage('Unclosed "variable" in "index" at line 3');
         $lexer->tokenize(new Source($template, 'index'));
     }
 
     public function testUnterminatedBlock()
     {
-        $this->expectException(SyntaxError::class);
-        $this->expectExceptionMessage('Unclosed "block" in "index" at line 3');
-
         $template = '
 
 {%
@@ -419,6 +416,10 @@ public function testUnterminatedBlock()
 ';
 
         $lexer = new Lexer(new Environment(new ArrayLoader()));
+
+        $this->expectException(SyntaxError::class);
+        $this->expectExceptionMessage('Unclosed "block" in "index" at line 3');
+
         $lexer->tokenize(new Source($template, 'index'));
     }