Skip to content

Commit 78aa8df

Browse files
acoultonacoulton
authored
test: Explicitly cover departures from expected cucumber parsing (#392)
Previously, we were just skipping testing gherkin examples where our parsing differed from Cucumber. In some cases, these contain syntax that is not represented in our own test data. This created a risk that, as we introduce the new compatibility mode, we accidentally change the behaviour of the parser in legacy mode. Going forward, we will explicitly assert that our parsing in these cases matches our existing behaviour. I have implemented this by capturing and dumping the result to YAML, rather than by manually representing in e.g. ndjson and loading this to compare as a FeatureNode. This is because our existing test logic, loaders and comparators are intentionally "loose" in certain places where we expect the serialised expectation to differ from the runtime value (some string padding, exception messages, etc) whereas we want to be absolutely explicit about the content of our nodes. Serialising to YAML and comparing as a string will also make it easy to update the expectations (by running with RE_RECORD_EXPECTATIONS=1) when e.g. we add new properties to our Node classes, or intentionally change exception messages. The expectations in this branch are based off the 4.14.0 release, before we introduced any of the new compatibility mode parser changes, to ensure that the expectations reflect the behaviour of the current production release.
1 parent 44a978c commit 78aa8df

29 files changed

+862
-6
lines changed

‎src/Filesystem.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,21 @@ public static function readFile(string $fileName): string
4343
return $result;
4444
}
4545

46+
public static function writeFile(string $fileName, string $content): void
47+
{
48+
self::ensureDirectoryExists(dirname($fileName));
49+
try {
50+
$result = self::callSafely(static fn () => file_put_contents($fileName, $content));
51+
} catch (ErrorException $e) {
52+
throw new FilesystemException(
53+
sprintf('File "%s" cannot be written: %s', $fileName, $e->getMessage()),
54+
previous: $e,
55+
);
56+
}
57+
58+
assert($result !== false, 'file_put_contents() should not return false without emitting a PHP warning');
59+
}
60+
4661
/**
4762
* @return array<mixed>
4863
*

‎tests/Cucumber/CompatibilityTest.php‎

Lines changed: 96 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,12 @@
1818
use Behat\Gherkin\Parser;
1919
use FilesystemIterator;
2020
use PHPUnit\Framework\Attributes\DataProvider;
21+
use PHPUnit\Framework\ExpectationFailedException;
2122
use PHPUnit\Framework\TestCase;
2223
use RuntimeException;
2324
use SebastianBergmann\Comparator\Factory;
2425
use SplFileInfo;
26+
use UnexpectedValueException;
2527

2628
/**
2729
* Tests the parser against the upstream cucumber/gherkin test data.
@@ -35,11 +37,13 @@ class CompatibilityTest extends TestCase
3537
{
3638
private const GHERKIN_TESTDATA_PATH = __DIR__ . '/../../vendor/cucumber/gherkin-monorepo/testdata';
3739
private const EXTRA_TESTDATA_PATH = __DIR__ . '/extra_testdata';
40+
private const PROJECT_BASE_PATH = __DIR__ . '/../../';
41+
private const EXPECTED_VARIANTS_PATH = __DIR__ . '/expected_variants';
3842

3943
/**
4044
* @phpstan-var TKnownIncompatibilityMap
4145
*/
42-
private array $notParsingCorrectly = [
46+
private static array $notParsingCorrectly = [
4347
'legacy' => [
4448
'complex_background.feature' => 'Rule keyword not supported',
4549
'docstrings.feature' => 'Escaped delimiters in docstrings are not unescaped',
@@ -74,7 +78,7 @@ class CompatibilityTest extends TestCase
7478
/**
7579
* @phpstan-var TKnownIncompatibilityMap
7680
*/
77-
private array $parsedButShouldNotBe = [
81+
private static array $parsedButShouldNotBe = [
7882
'legacy' => [
7983
'invalid_language.feature' => 'Invalid language is silently ignored',
8084
],
@@ -132,8 +136,8 @@ protected function setUp(): void
132136
#[DataProvider('goodCucumberFeatures')]
133137
public function testFeaturesParseTheSameAsCucumber(GherkinCompatibilityMode $mode, SplFileInfo $file): void
134138
{
135-
if (isset($this->notParsingCorrectly[$mode->value][$file->getFilename()])) {
136-
$this->markTestIncomplete($this->notParsingCorrectly[$mode->value][$file->getFilename()]);
139+
if (isset(self::$notParsingCorrectly[$mode->value][$file->getFilename()])) {
140+
$this->markTestIncomplete(self::$notParsingCorrectly[$mode->value][$file->getFilename()]);
137141
}
138142

139143
assert(self::$featureNodeComparator instanceof FeatureNodeComparator);
@@ -158,8 +162,8 @@ public function testFeaturesParseTheSameAsCucumber(GherkinCompatibilityMode $mod
158162
#[DataProvider('badCucumberFeatures')]
159163
public function testBadFeaturesDoNotParse(GherkinCompatibilityMode $mode, SplFileInfo $file): void
160164
{
161-
if (isset($this->parsedButShouldNotBe[$mode->value][$file->getFilename()])) {
162-
$this->markTestIncomplete($this->parsedButShouldNotBe[$mode->value][$file->getFilename()]);
165+
if (isset(self::$parsedButShouldNotBe[$mode->value][$file->getFilename()])) {
166+
$this->markTestIncomplete(self::$parsedButShouldNotBe[$mode->value][$file->getFilename()]);
163167
}
164168

165169
$gherkinFile = $file->getPathname();
@@ -177,6 +181,70 @@ public function testBadFeaturesDoNotParse(GherkinCompatibilityMode $mode, SplFil
177181
$this->parser->parseFile($gherkinFile);
178182
}
179183

184+
#[DataProvider('skippedCucumberFeatures')]
185+
public function testSkippedExamplesParseAsExpected(GherkinCompatibilityMode $mode, SplFileInfo $file): void
186+
{
187+
$dumper = new ParserResultDumper(Filesystem::getRealPath(self::PROJECT_BASE_PATH));
188+
189+
$this->parser->setGherkinCompatibilityMode($mode);
190+
191+
try {
192+
$gherkinFile = Filesystem::getRealPath($file->getPathname());
193+
$result = $this->parser->parse(Filesystem::readFile($gherkinFile), $gherkinFile);
194+
} catch (ParserException $e) {
195+
$result = $e;
196+
}
197+
198+
$dumpedResult = $dumper->dump($result);
199+
200+
$expectationFile = $this->getExpectedVariantFilename($mode, $file);
201+
try {
202+
$this->assertStringEqualsFile($expectationFile, $dumpedResult);
203+
} catch (ExpectationFailedException $e) {
204+
if (getenv('RE_RECORD_EXPECTATIONS')) {
205+
Filesystem::writeFile($expectationFile, $dumpedResult);
206+
}
207+
throw $e;
208+
}
209+
}
210+
211+
public function testNoRedundantVariantsFiles(): void
212+
{
213+
// This is a sanity check to ensure that variant expectation files are deleted once they are no longer required.
214+
$usedFiles = [];
215+
foreach (self::skippedCucumberFeatures() as ['mode' => $mode, 'file' => $file]) {
216+
$usedFiles[] = $this->getExpectedVariantFilename($mode, $file);
217+
}
218+
219+
$redundantFiles = array_diff(
220+
Filesystem::findFilesRecursively(self::EXPECTED_VARIANTS_PATH, '*.expected.yaml'),
221+
$usedFiles
222+
);
223+
if ($redundantFiles === []) {
224+
$this->addToAssertionCount(1);
225+
226+
return;
227+
}
228+
229+
$fileList = implode(
230+
"\n - ",
231+
array_map(
232+
fn ($f) => preg_replace(
233+
'/^' . preg_quote(self::EXPECTED_VARIANTS_PATH . '/', '/') . '/',
234+
'',
235+
$f
236+
),
237+
$redundantFiles
238+
)
239+
);
240+
241+
throw new UnexpectedValueException(sprintf(
242+
"Found redundant expectation files in %s that were not used by any skipped feature:\n - %s",
243+
self::EXPECTED_VARIANTS_PATH,
244+
$fileList
245+
));
246+
}
247+
180248
/**
181249
* @phpstan-return iterable<string, TCucumberParsingTestCase>
182250
*/
@@ -195,6 +263,23 @@ public static function badCucumberFeatures(): iterable
195263
yield from self::getCucumberFeatures(self::EXTRA_TESTDATA_PATH . '/bad');
196264
}
197265

266+
/**
267+
* @return iterable<string, TCucumberParsingTestCase>
268+
*/
269+
public static function skippedCucumberFeatures(): iterable
270+
{
271+
foreach (self::goodCucumberFeatures() as $key => ['mode' => $mode, 'file' => $file]) {
272+
if (isset(self::$notParsingCorrectly[$mode->value][$file->getFilename()])) {
273+
yield $key => ['mode' => $mode, 'file' => $file];
274+
}
275+
}
276+
foreach (self::badCucumberFeatures() as $key => ['mode' => $mode, 'file' => $file]) {
277+
if (isset(self::$parsedButShouldNotBe[$mode->value][$file->getFilename()])) {
278+
yield $key => ['mode' => $mode, 'file' => $file];
279+
}
280+
}
281+
}
282+
198283
/**
199284
* @phpstan-return iterable<string, TCucumberParsingTestCase>
200285
*/
@@ -229,4 +314,9 @@ static function ($errno, $errstr) {
229314
$this->expectExceptionMessageMatches($message);
230315
$this->expectException(RuntimeException::class);
231316
}
317+
318+
private function getExpectedVariantFilename(GherkinCompatibilityMode $mode, SplFileInfo $file): string
319+
{
320+
return self::EXPECTED_VARIANTS_PATH . '/' . $mode->value . '/' . $file->getFilename() . '.expected.yaml';
321+
}
232322
}

‎tests/Cucumber/ParserResultDumper.php‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Behat Gherkin Parser.
5+
* (c) Konstantin Kudryashov <ever.zet@gmail.com>
6+
*
7+
* For the full copyright and license information, please view the LICENSE
8+
* file that was distributed with this source code.
9+
*/
10+
11+
namespace Tests\Behat\Gherkin\Cucumber;
12+
13+
use Behat\Gherkin\Exception\ParserException;
14+
use Behat\Gherkin\Node\FeatureNode;
15+
use ReflectionClass;
16+
use Symfony\Component\Yaml\Yaml;
17+
18+
class ParserResultDumper
19+
{
20+
public function __construct(
21+
private readonly string $baseDir,
22+
) {
23+
}
24+
25+
public function dump(FeatureNode|ParserException|null $result): string
26+
{
27+
if ($result instanceof ParserException) {
28+
$values = $this->dumpParserExceptionValues($result);
29+
} else {
30+
$values = $this->recursiveDump($result);
31+
}
32+
33+
$result = Yaml::dump(
34+
$values,
35+
inline: 999,
36+
flags: Yaml::DUMP_MULTI_LINE_LITERAL_BLOCK | Yaml::DUMP_NULL_AS_TILDE | Yaml::DUMP_EMPTY_ARRAY_AS_SEQUENCE,
37+
);
38+
39+
return $this->replaceAbsolutePaths($result);
40+
}
41+
42+
/**
43+
* @return array<string, array<string,mixed>>
44+
*/
45+
private function dumpParserExceptionValues(ParserException $exception): array
46+
{
47+
// We don't want to include all the properties of the exception, we're not worried about matching the trace etc
48+
return [
49+
$exception::class => [
50+
'message' => $exception->getMessage(),
51+
'code' => $exception->getCode(),
52+
],
53+
];
54+
}
55+
56+
private function recursiveDump(mixed $value): mixed
57+
{
58+
if (is_array($value)) {
59+
return array_map($this->recursiveDump(...), $value);
60+
}
61+
62+
if ($value === null || is_scalar($value)) {
63+
return $value;
64+
}
65+
66+
assert(is_object($value), 'Expected object, got: ' . get_debug_type($value));
67+
68+
// Use reflection to export the object properties for completeness and simplicity.
69+
// Note that this is not safe against recursive object references, but is safe to use in this context because
70+
// we know that the FeatureNode and the objects it contains do not contain any recursive references.
71+
$values = [];
72+
$reflection = new ReflectionClass($value);
73+
foreach ($reflection->getProperties() as $property) {
74+
$values[$property->getName()] = match ($property->isInitialized($value)) {
75+
true => $this->recursiveDump($property->getValue($value)),
76+
false => '**NOT INITIALIZED**',
77+
};
78+
}
79+
80+
return [$value::class => $values];
81+
}
82+
83+
private function replaceAbsolutePaths(string $text): string
84+
{
85+
$result = preg_replace('/' . preg_quote($this->baseDir, '/') . '/', '{BASEDIR}', $text);
86+
assert(is_string($result), 'Replacing paths should succeed');
87+
88+
return $result;
89+
}
90+
}

‎tests/Cucumber/expected_variants/gherkin-32/complex_background.feature.expected.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Behat\Gherkin\Exception\UnexpectedParserNodeException:
2+
message: 'Expected Step, Examples table, or end of Scenario, but got text: " Rule: My Rule" in file: {BASEDIR}/vendor/cucumber/gherkin-monorepo/testdata/good/complex_background.feature'
3+
code: 0

‎tests/Cucumber/expected_variants/gherkin-32/descriptions.feature.expected.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Behat\Gherkin\Exception\UnexpectedParserNodeException:
2+
message: 'Expected Examples table or end of Scenario, but got text: "This is an examples description" in file: {BASEDIR}/vendor/cucumber/gherkin-monorepo/testdata/good/descriptions.feature'
3+
code: 0

‎tests/Cucumber/expected_variants/gherkin-32/descriptions_with_comments.feature.expected.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Behat\Gherkin\Exception\UnexpectedParserNodeException:
2+
message: 'Expected Examples table or end of Scenario, but got text: "This is an examples description" in file: {BASEDIR}/vendor/cucumber/gherkin-monorepo/testdata/good/descriptions_with_comments.feature'
3+
code: 0

0 commit comments

Comments
 (0)