Skip to content

Commit 8cf3938

Browse files
committed
TestCases test inspections in isolation
Before, all TestCases ran with all inspections enabled all the time. This could lead to false positives because tests don't see which inspection triggers errors.
1 parent c951ec0 commit 8cf3938

11 files changed

+154
-16
lines changed

src/Extension.php

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,25 @@
1414
use AlisQI\TwigQI\Inspection\UndeclaredVariableInMacro;
1515
use AlisQI\TwigQI\Inspection\InvalidTypes;
1616
use Twig\Extension\AbstractExtension;
17+
use Twig\NodeVisitor\NodeVisitorInterface;
1718

1819
class Extension extends AbstractExtension
1920
{
21+
/**
22+
* @var NodeVisitorInterface[]
23+
*/
24+
private array $inspections;
25+
26+
public function __construct(array|null $inspections = null) {
27+
$this->inspections = $inspections ?? $this->getDefaultInspections();
28+
}
29+
2030
public function getNodeVisitors(): array
31+
{
32+
return $this->inspections;
33+
}
34+
35+
private function getDefaultInspections(): array
2136
{
2237
return [
2338
new InvalidTypes(),

tests/AbstractTestCase.php

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,20 +4,54 @@
44

55
namespace AlisQI\TwigQI\Tests;
66

7-
use AlisQI\TwigQI\Extension;
87
use PHPUnit\Framework\TestCase;
98
use Twig\Environment;
9+
use Twig\Extension\ExtensionInterface;
1010
use Twig\Loader\ArrayLoader;
1111

1212
abstract class AbstractTestCase extends TestCase
1313
{
1414
protected Environment $env;
1515
protected array $errors;
1616

17+
/** @var array<string, string> */
18+
private static array $extensionPerTestCase = [];
19+
20+
/**
21+
* To test inspections in isolation, we need to create Twig Environments
22+
* with Extensions that load only one inspection (NodeVisitor) at a time.
23+
*
24+
* When Twig compiles templates to Template classes, their class name contains
25+
* the Environment's optionsHash to make them unique.
26+
*
27+
* _However_, that options hash includes only the Extensions' class names.
28+
* Adding the same Extension class but with different inspections (node visitors)
29+
* causes hash collision (for lack of a more precise term), meaning that templates
30+
* compiled with a different inspection get reused.
31+
*
32+
* Therefore, every TestCase must create a unique extension class.
33+
* The easiest way is to create anonymous classes that extend AlisQI\TwigQI\Extension.
34+
* We can't do that in this class because the name of the anonymous class is based on
35+
* the class that creates it, which again leads to collisions in the options hash.
36+
*/
37+
abstract protected function createUniqueExtensionClass(): ExtensionInterface;
38+
1739
public function setUp(): void
1840
{
1941
$this->env = new Environment(new ArrayLoader());
20-
$this->env->addExtension(new Extension());
42+
43+
$extension = $this->createUniqueExtensionClass();
44+
$this->env->addExtension($extension);
45+
46+
// Ensure Extension class is unique per TestCase
47+
$extensionClass = get_class($extension);
48+
if (!array_key_exists(static::class, self::$extensionPerTestCase)) {
49+
if (in_array($extensionClass, self::$extensionPerTestCase)) {
50+
throw new \RuntimeException('Duplicate extension class name');
51+
}
52+
53+
self::$extensionPerTestCase[static::class] = $extensionClass;
54+
}
2155

2256
$this->errors = [];
2357
set_error_handler(function (int $code, string $error) {

tests/BadArgumentCountInMacroCallTest.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,21 @@
44

55
namespace AlisQI\TwigQI\Tests;
66

7+
use AlisQI\TwigQI\Extension;
8+
use AlisQI\TwigQI\Inspection\BadArgumentCountInMacroCall;
79
use PHPUnit\Framework\Attributes\DataProvider;
10+
use Twig\Extension\ExtensionInterface;
811
use Twig\Loader\FilesystemLoader;
912

1013
class BadArgumentCountInMacroCallTest extends AbstractTestCase
1114
{
15+
protected function createUniqueExtensionClass(): ExtensionInterface
16+
{
17+
return new class([
18+
new BadArgumentCountInMacroCall()
19+
]) extends Extension {};
20+
}
21+
1222
public function test_itDoesNotWarnForMatchingArgumentNumber(): void
1323
{
1424
$this->env->createTemplate(<<<EOF

tests/InvalidConstantTest.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,20 @@
44

55
namespace AlisQI\TwigQI\Tests;
66

7+
use AlisQI\TwigQI\Extension;
8+
use AlisQI\TwigQI\Inspection\InvalidConstant;
79
use PHPUnit\Framework\Attributes\DataProvider;
10+
use Twig\Extension\ExtensionInterface;
811

912
class InvalidConstantTest extends AbstractTestCase
1013
{
14+
protected function createUniqueExtensionClass(): ExtensionInterface
15+
{
16+
return new class([
17+
new InvalidConstant(),
18+
]) extends Extension {};
19+
}
20+
1121
public static function getConstants(): array
1222
{
1323
return [

tests/InvalidDotOperationTest.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,20 @@
44

55
namespace AlisQI\TwigQI\Tests;
66

7+
use AlisQI\TwigQI\Extension;
8+
use AlisQI\TwigQI\Inspection\InvalidDotOperation;
79
use PHPUnit\Framework\Attributes\DataProvider;
10+
use Twig\Extension\ExtensionInterface;
811

912
class InvalidDotOperationTest extends AbstractTestCase
1013
{
14+
protected function createUniqueExtensionClass(): ExtensionInterface
15+
{
16+
return new class([
17+
new InvalidDotOperation()
18+
]) extends Extension {};
19+
}
20+
1121
public function test_itIgnoresAttributesOnExpressions(): void
1222
{
1323
$this->env->createTemplate(<<<EOF

tests/InvalidEnumCaseTest.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,20 @@
44

55
namespace AlisQI\TwigQI\Tests;
66

7+
use AlisQI\TwigQI\Extension;
8+
use AlisQI\TwigQI\Inspection\InvalidEnumCase;
79
use PHPUnit\Framework\Attributes\DataProvider;
10+
use Twig\Extension\ExtensionInterface;
811

912
class InvalidEnumCaseTest extends AbstractTestCase
1013
{
14+
protected function createUniqueExtensionClass(): ExtensionInterface
15+
{
16+
return new class([
17+
new InvalidEnumCase()
18+
]) extends Extension {};
19+
}
20+
1121
public static function getEnomCases(): array
1222
{
1323
$enum = '\\\\AlisQI\\\\TwigQI\\\\Tests\\\\Type\\\\Enom';

tests/InvalidTypesTest.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,20 @@
44

55
namespace AlisQI\TwigQI\Tests;
66

7+
use AlisQI\TwigQI\Extension;
8+
use AlisQI\TwigQI\Inspection\InvalidTypes;
79
use PHPUnit\Framework\Attributes\DataProvider;
10+
use Twig\Extension\ExtensionInterface;
811

912
class InvalidTypesTest extends AbstractTestCase
1013
{
14+
protected function createUniqueExtensionClass(): ExtensionInterface
15+
{
16+
return new class([
17+
new InvalidTypes()
18+
]) extends Extension {};
19+
}
20+
1121
public static function getTypes(): array
1222
{
1323
return [

tests/PositionalMacroArgumentAfterNamedTest.php

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,19 @@
44

55
namespace AlisQI\TwigQI\Tests;
66

7+
use AlisQI\TwigQI\Extension;
8+
use AlisQI\TwigQI\Inspection\PositionalMacroArgumentAfterNamed;
9+
use Twig\Extension\ExtensionInterface;
10+
711
class PositionalMacroArgumentAfterNamedTest extends AbstractTestCase
812
{
13+
protected function createUniqueExtensionClass(): ExtensionInterface
14+
{
15+
return new class([
16+
new PositionalMacroArgumentAfterNamed()
17+
]) extends Extension {};
18+
}
19+
920
public function test_itSupportsNamedArguments(): void
1021
{
1122
$this->env->createTemplate(<<<EOF

tests/RequiredMacroArgumentAfterOptionalTest.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,20 @@
44

55
namespace AlisQI\TwigQI\Tests;
66

7+
use AlisQI\TwigQI\Extension;
8+
use AlisQI\TwigQI\Inspection\RequiredMacroArgumentAfterOptional;
79
use PHPUnit\Framework\Attributes\DataProvider;
10+
use Twig\Extension\ExtensionInterface;
811

912
class RequiredMacroArgumentAfterOptionalTest extends AbstractTestCase
1013
{
14+
protected function createUniqueExtensionClass(): ExtensionInterface
15+
{
16+
return new class([
17+
new RequiredMacroArgumentAfterOptional()
18+
]) extends Extension {};
19+
}
20+
1121
public static function getValidOrderTests(): array
1222
{
1323
return [

tests/TypeAssertionsTest.php

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,22 @@
44

55
namespace AlisQI\TwigQI\Tests;
66

7-
use ArrayIterator;
8-
use DateTime;
9-
use Exception;
7+
use AlisQI\TwigQI\Assertion\WrapTypesInAssertedTypes;
8+
use AlisQI\TwigQI\Extension;
109
use PHPUnit\Framework\Attributes\DataProvider;
10+
use Twig\Extension\ExtensionInterface;
1111
use Twig\Markup;
1212
use Twig\Node\Node;
1313

1414
class TypeAssertionsTest extends AbstractTestCase
1515
{
16+
protected function createUniqueExtensionClass(): ExtensionInterface
17+
{
18+
return new class([
19+
new WrapTypesInAssertedTypes()
20+
]) extends Extension {};
21+
}
22+
1623
public static function getOptionalVariables(): array
1724
{
1825
return [
@@ -78,12 +85,12 @@ public static function getTypes(): array
7885
['string', '', true],
7986
['string', '0', true],
8087
['string', new Markup('hello', 'UTF-8'), true], // class Markup implements \Stringable
81-
['string', new Exception(), true], // Exception has __toString()
88+
['string', new \Exception(), true], // Exception has __toString()
8289

8390
['string', true, false],
8491
['string', 1, false],
8592
['string', [], false],
86-
['string', new DateTime(), false],
93+
['string', new \DateTime(), false],
8794

8895
['number', 0, true],
8996
['number', 1, true],
@@ -93,7 +100,7 @@ public static function getTypes(): array
93100
['number', '', false],
94101
['number', '0', false],
95102
['number', [], false],
96-
['number', new Exception(), false],
103+
['number', new \Exception(), false],
97104

98105
['boolean', true, true],
99106
['boolean', false, true],
@@ -102,23 +109,23 @@ public static function getTypes(): array
102109
['boolean', 'true', false],
103110
['boolean', '0', false],
104111
['boolean', [], false],
105-
['boolean', new Exception(), false],
112+
['boolean', new \Exception(), false],
106113

107-
['object', new Exception(), true],
114+
['object', new \Exception(), true],
108115

109116
['object', 'object', false],
110117
['object', true, false],
111118
['object', [], false],
112119

113120
['iterable', [], true],
114121
['iterable', [13, 37], true],
115-
['iterable', new ArrayIterator([13, 37]), true],
122+
['iterable', new \ArrayIterator([13, 37]), true],
116123
['iterable', ['foo' => 'bar'], true],
117124
['iterable', 'hello', false],
118125

119126
['iterable<string>', [], true],
120127
['iterable<string>', ['hello'], true],
121-
['iterable<string>', new ArrayIterator(['hello']), true],
128+
['iterable<string>', new \ArrayIterator(['hello']), true],
122129
['iterable<string>', [1337], false],
123130
['iterable<string>', 'hello', false],
124131

@@ -129,15 +136,15 @@ public static function getTypes(): array
129136

130137
['iterable<string, string>', [], true],
131138
['iterable<string, string>', ['foo' => 'bar'], true],
132-
['iterable<string, string>', new ArrayIterator(['foo' => 'bar']), true],
139+
['iterable<string, string>', new \ArrayIterator(['foo' => 'bar']), true],
133140
['iterable<string, string>', ['foo' => 1337], false],
134141
['iterable<string, string>', ['foo' => ['bar']], false],
135142
['iterable<string, string>', [13, 37], false],
136143

137144
['iterable<number, number>', [], true],
138145
['iterable<number, number>', [13, 37], true],
139146
['iterable<number, number>', [13 => 37], true],
140-
['iterable<number, number>', new ArrayIterator([13 => 37]), true],
147+
['iterable<number, number>', new \ArrayIterator([13 => 37]), true],
141148
['iterable<number, number>', ['13' => 37], true],
142149
['iterable<number, number>', ['leet' => 1337], false],
143150

@@ -150,11 +157,11 @@ public static function getTypes(): array
150157
['iterable<iterable<iterable<string, number>>>', [[[13, 37]]], false],
151158
['iterable<iterable<iterable<string, number>>>', [[['foo' => 'bar']]], false],
152159

153-
['\\\\Exception', new Exception(), true],
160+
['\\\\Exception', new \Exception(), true],
154161
['\\\\Exception', new Node(), false],
155162
['iterable<string, \\\\Twig\\\\Node\\\\Node>', ['node' => new Node()], true],
156163
['iterable<string, \\\\Twig\\\\Node\\\\Node[]>', ['nodes' => [new Node(), new Node()]], true],
157-
['iterable<string, \\\\Twig\\\\Node\\\\Node>', ['node' => new Exception()], false],
164+
['iterable<string, \\\\Twig\\\\Node\\\\Node>', ['node' => new \Exception()], false],
158165

159166
['\\\\Traversable', new Node(), true],
160167
['\\\\Traversable', true, false],

0 commit comments

Comments
 (0)