Skip to content

Commit ec008d9

Browse files
Fahl-DesignFahl-Design
authored
feat(refactor): refactor and simplify reporter internals (#14)
* feat(extension): show failing test name in inline AI Context Add `Test failed: <full_name>` line to inline AI Context output so report consumers see which test produced the captured failure. Extend unit coverage for inline-context printing across failure/error/ warning statuses, the disabled `report` option, non-failure statuses, and end-to-end report writing. * chore(deps): bump php 8.3.30→8.3.31 and phpunit 12.5.23→12.5.24 * refactor(extension): tighten inline AI Context output Drop redundant array_values() wrap on HintGenerator output (already a list), nullsafe the suite-name extraction, collapse normalizeArtifacts into a single ternary, and extract a printSection() helper to remove the repeated section-printing boilerplate in printInlineContext(). * refactor(trace): collapse opposing branches in isNoiseFrame * refactor(report): mark PathNormalizer roots readonly * refactor(config): extract reusable validators in fromArray Replace six near-identical type/value checks with three private helpers (readEnum, readPositiveInt, readBool) plus one dedicated readOutput that falls back to the supplied default on null/empty. The helpers narrow `mixed` cleanly, so the @phpstan-ignore-next-line suppressions are no longer needed. * test: share PathNormalizer factory and dedupe invalid-input cases Add tests/Support/Fixture/PathNormalizerFactory to replace three ad-hoc constructions of PathNormalizer across the report and trace test files. Collapse the two single-case `testInvalidFormatThrows` / `testInvalidMaxFramesThrows` methods into a data-driven `testInvalidConfigThrows` and extend coverage to the bool / output / non-int cases exercised by the new ReporterConfig validator helpers. * fix: harden two edge cases flagged by CodeRabbit ReporterConfig::fromArray previously rtrim-ed the resolved output directory, which could collapse a root path (e.g. "/") into an empty string and violate the @var non-empty-string contract. Keep the trimmed value only when it is still non-empty; otherwise fall back to the resolved path. AiReporterTest::makeStubTest cast ReflectionClass::getFileName() straight to string. Guard with assertNotFalse so a future internal class swap fails loudly instead of silently passing an empty filename.
1 parent d6cbcbd commit ec008d9

14 files changed

Lines changed: 538 additions & 214 deletions

‎.php-version‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
8.3.30
1+
8.3.31

‎composer.lock‎

Lines changed: 120 additions & 103 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/Config/ReporterConfig.php‎

Lines changed: 67 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
namespace WebProject\Codeception\Module\AiReporter\Config;
66

7+
use function implode;
78
use function in_array;
89
use InvalidArgumentException;
910
use function is_bool;
@@ -54,59 +55,85 @@ private function __construct(
5455
public static function fromArray(array $raw, string $defaultOutputDir, string $projectRoot): self
5556
{
5657
/** @var self::FORMAT_* $format */
57-
$format = $raw['format'] ?? self::FORMAT_BOTH;
58-
// @phpstan-ignore-next-line
59-
if (!is_string($format) || !in_array($format, [self::FORMAT_TEXT, self::FORMAT_JSON, self::FORMAT_BOTH], true)) {
60-
throw new InvalidArgumentException('Invalid `format`; expected one of: text, json, both.');
58+
$format = self::readEnum(
59+
$raw['format'] ?? null,
60+
'format',
61+
[self::FORMAT_TEXT, self::FORMAT_JSON, self::FORMAT_BOTH],
62+
self::FORMAT_BOTH,
63+
);
64+
65+
$output = self::readOutput($raw['output'] ?? null, $defaultOutputDir);
66+
$resolved = self::resolvePath($output, $projectRoot);
67+
$trimmed = rtrim($resolved, '/\\');
68+
69+
/** @var non-empty-string $outputDir */
70+
$outputDir = '' === $trimmed ? $resolved : $trimmed;
71+
72+
return new self(
73+
format: $format,
74+
outputDir: $outputDir,
75+
maxFrames: self::readPositiveInt($raw['max_frames'] ?? null, 'max_frames', self::DEFAULT_MAX_FRAMES),
76+
includeSteps: self::readBool($raw['include_steps'] ?? null, 'include_steps', true),
77+
includeArtifacts: self::readBool($raw['include_artifacts'] ?? null, 'include_artifacts', true),
78+
compactPaths: self::readBool($raw['compact_paths'] ?? null, 'compact_paths', true),
79+
);
80+
}
81+
82+
/**
83+
* @param list<string> $allowed
84+
*/
85+
private static function readEnum(mixed $value, string $field, array $allowed, string $default): string
86+
{
87+
if (null === $value) {
88+
return $default;
89+
}
90+
if (!is_string($value) || !in_array($value, $allowed, true)) {
91+
throw new InvalidArgumentException(sprintf('Invalid `%s`; expected one of: %s.', $field, implode(', ', $allowed)));
6192
}
6293

63-
$output = $raw['output'] ?? $defaultOutputDir;
64-
if ('' === $output) {
65-
$output = $defaultOutputDir;
94+
return $value;
95+
}
96+
97+
/** @param non-empty-string $default */
98+
private static function readOutput(mixed $value, string $default): string
99+
{
100+
if (null === $value || '' === $value) {
101+
return $default;
66102
}
67-
// @phpstan-ignore-next-line
68-
if (!is_string($output) || '' === $output) {
103+
if (!is_string($value)) {
69104
throw new InvalidArgumentException('Invalid `output`; expected a non-empty directory path.');
70105
}
71106

72-
$outputDir = self::resolvePath($output, $projectRoot);
107+
return $value;
108+
}
73109

74-
/** @var int<1, max> $maxFrames */
75-
$maxFrames = $raw['max_frames'] ?? self::DEFAULT_MAX_FRAMES;
76-
// @phpstan-ignore-next-line
77-
if (!is_int($maxFrames) || $maxFrames < 1) {
78-
throw new InvalidArgumentException('Invalid `max_frames`; expected a positive integer.');
110+
/**
111+
* @param int<1, max> $default
112+
*
113+
* @return int<1, max>
114+
*/
115+
private static function readPositiveInt(mixed $value, string $field, int $default): int
116+
{
117+
if (null === $value) {
118+
return $default;
79119
}
80-
81-
$includeSteps = $raw['include_steps'] ?? true;
82-
// @phpstan-ignore-next-line
83-
if (!is_bool($includeSteps)) {
84-
throw new InvalidArgumentException('Invalid `include_steps`; expected boolean.');
120+
if (!is_int($value) || $value < 1) {
121+
throw new InvalidArgumentException(sprintf('Invalid `%s`; expected a positive integer.', $field));
85122
}
86123

87-
$includeArtifacts = $raw['include_artifacts'] ?? true;
88-
// @phpstan-ignore-next-line
89-
if (!is_bool($includeArtifacts)) {
90-
throw new InvalidArgumentException('Invalid `include_artifacts`; expected boolean.');
91-
}
124+
return $value;
125+
}
92126

93-
$compactPaths = $raw['compact_paths'] ?? true;
94-
// @phpstan-ignore-next-line
95-
if (!is_bool($compactPaths)) {
96-
throw new InvalidArgumentException('Invalid `compact_paths`; expected boolean.');
127+
private static function readBool(mixed $value, string $field, bool $default): bool
128+
{
129+
if (null === $value) {
130+
return $default;
131+
}
132+
if (!is_bool($value)) {
133+
throw new InvalidArgumentException(sprintf('Invalid `%s`; expected boolean.', $field));
97134
}
98135

99-
/** @var non-empty-string $outputDir */
100-
$outputDir = rtrim($outputDir, '/\\');
101-
102-
return new self(
103-
format: $format,
104-
outputDir: $outputDir,
105-
maxFrames: $maxFrames,
106-
includeSteps: $includeSteps,
107-
includeArtifacts: $includeArtifacts,
108-
compactPaths: $compactPaths,
109-
);
136+
return $value;
110137
}
111138

112139
public function wantsJson(): bool

‎src/Extension/AiReporter.php‎

Lines changed: 42 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44

55
namespace WebProject\Codeception\Module\AiReporter\Extension;
66

7+
use function array_map;
78
use function array_slice;
8-
use function array_values;
99
use Codeception\Event\FailEvent;
1010
use Codeception\Event\PrintResultEvent;
1111
use Codeception\Event\SuiteEvent;
@@ -129,8 +129,7 @@ public function _initialize(): void
129129

130130
public function beforeSuite(SuiteEvent $event): void
131131
{
132-
$suite = $event->getSuite();
133-
$this->currentSuite = null !== $suite ? $suite->getName() : '';
132+
$this->currentSuite = $event->getSuite()?->getName() ?? '';
134133
}
135134

136135
public function onFailure(FailEvent $event): void
@@ -201,7 +200,7 @@ private function captureFailure(string $status, FailEvent $event): void
201200
? $this->scenarioExtractor->extract($test, $this->runtimeConfig->maxFrames())
202201
: [];
203202

204-
$hints = array_values($this->hintGenerator->generate($throwable, $trace, $scenarioSteps));
203+
$hints = $this->hintGenerator->generate($throwable, $trace, $scenarioSteps);
205204

206205
$exception = [
207206
'class' => $throwable::class,
@@ -259,42 +258,53 @@ private function printInlineContext(array $failure): void
259258
$artifacts = $failure['artifacts'];
260259

261260
$this->writeln(' <comment>AI Context</comment>');
261+
$this->writeln(sprintf(' Test failed: %s', $this->consoleText->escape($failure['test']['full_name'])));
262262
$this->writeln(sprintf(' Exception: %s', $this->consoleText->escape($exception['class'])));
263263
$this->writeln(sprintf(' Message: %s', $this->consoleText->escape($this->consoleText->truncate($exception['message']))));
264264

265-
if (isset($exception['comparison_diff']) && '' !== $exception['comparison_diff']) {
266-
$this->writeln(' Diff:');
267-
foreach (explode("\n", $exception['comparison_diff']) as $diffLine) {
268-
$this->writeln(sprintf(' %s', $this->consoleText->escape($diffLine)));
269-
}
265+
$diff = $exception['comparison_diff'] ?? '';
266+
$this->printSection('Diff', '' === $diff ? [] : array_map(
267+
fn (string $line): string => $this->consoleText->escape($line),
268+
explode("\n", $diff),
269+
));
270+
271+
$traceLines = [];
272+
foreach (array_slice($trace, 0, $this->runtimeConfig->maxFrames()) as $index => $frame) {
273+
$traceLines[] = sprintf('#%d %s', $index + 1, $this->consoleText->escape($this->traceFrameProcessor->formatFrame($frame)));
270274
}
275+
$this->printSection('Trace', $traceLines);
271276

272-
if ([] !== $trace) {
273-
$this->writeln(' Trace:');
274-
foreach (array_slice($trace, 0, $this->runtimeConfig->maxFrames()) as $index => $frame) {
275-
$this->writeln(sprintf(' #%d %s', $index + 1, $this->consoleText->escape($this->traceFrameProcessor->formatFrame($frame))));
276-
}
277+
$stepLines = [];
278+
foreach (array_slice($steps, 0, 2) as $step) {
279+
$stepLines[] = sprintf('- %s', $this->consoleText->escape($step['step']));
277280
}
281+
$this->printSection('Scenario', $stepLines);
278282

279-
if ([] !== $steps) {
280-
$this->writeln(' Scenario:');
281-
foreach (array_slice($steps, 0, 2) as $step) {
282-
$this->writeln(sprintf(' - %s', $this->consoleText->escape($step['step'])));
283-
}
283+
$artifactLines = [];
284+
foreach ($artifacts as $type => $path) {
285+
$artifactLines[] = sprintf('- %s: %s', $this->consoleText->escape($type), $this->consoleText->escape($path));
284286
}
287+
$this->printSection('Artifacts', $artifactLines);
285288

286-
if ([] !== $artifacts) {
287-
$this->writeln(' Artifacts:');
288-
foreach ($artifacts as $type => $path) {
289-
$this->writeln(sprintf(' - %s: %s', $this->consoleText->escape($type), $this->consoleText->escape($path)));
290-
}
289+
$hintLines = [];
290+
foreach (array_slice($hints, 0, 3) as $hint) {
291+
$hintLines[] = sprintf('- %s', $this->consoleText->escape($hint));
291292
}
293+
$this->printSection('Hints', $hintLines);
294+
}
292295

293-
if ([] !== $hints) {
294-
$this->writeln(' Hints:');
295-
foreach (array_slice($hints, 0, 3) as $hint) {
296-
$this->writeln(sprintf(' - %s', $this->consoleText->escape($hint)));
297-
}
296+
/**
297+
* @param list<string> $lines
298+
*/
299+
private function printSection(string $title, array $lines): void
300+
{
301+
if ([] === $lines) {
302+
return;
303+
}
304+
305+
$this->writeln(sprintf(' %s:', $title));
306+
foreach ($lines as $line) {
307+
$this->writeln(sprintf(' %s', $line));
298308
}
299309
}
300310

@@ -359,12 +369,9 @@ private function normalizeArtifacts(array $reports): array
359369
{
360370
$normalized = [];
361371
foreach ($reports as $type => $path) {
362-
if (is_scalar($path)) {
363-
$normalized[(string) $type] = $this->pathNormalizer->normalize((string) $path);
364-
continue;
365-
}
366-
367-
$normalized[(string) $type] = (string) json_encode($path, JSON_INVALID_UTF8_SUBSTITUTE);
372+
$normalized[(string) $type] = is_scalar($path)
373+
? $this->pathNormalizer->normalize((string) $path)
374+
: (string) json_encode($path, JSON_INVALID_UTF8_SUBSTITUTE);
368375
}
369376

370377
return $normalized;

‎src/Report/PathNormalizer.php‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,14 @@
1414

1515
final class PathNormalizer
1616
{
17-
private string $normalizedRoot;
18-
private string $normalizedRootLower;
17+
private readonly string $normalizedRoot;
18+
private readonly string $normalizedRootLower;
1919

2020
public function __construct(string $projectRoot, private readonly bool $compactPaths)
2121
{
22-
$normalized = str_replace('\\', '/', rtrim($projectRoot, '/\\'));
23-
$this->normalizedRoot = $normalized . '/';
24-
$this->normalizedRootLower = strtolower($this->normalizedRoot);
22+
$normalized = str_replace('\\', '/', rtrim($projectRoot, '/\\'));
23+
$this->normalizedRoot = $normalized . '/';
24+
$this->normalizedRootLower = strtolower($this->normalizedRoot);
2525
}
2626

2727
/**

‎src/Util/TraceFrameProcessor.php‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -114,12 +114,8 @@ private function isNoiseFrame(array $frame): bool
114114
$file = (string)($frame['file'] ?? '');
115115
$call = (string)($frame['call'] ?? '');
116116

117-
if ($file !== '' && !$this->isFrameworkFile($file)) {
118-
return false;
119-
}
120-
121-
if ($file !== '' && $this->isFrameworkFile($file)) {
122-
return true;
117+
if ($file !== '') {
118+
return $this->isFrameworkFile($file);
123119
}
124120

125121
return str_starts_with($call, '[throw] PHPUnit\\Framework\\')

‎tests/Support/Fixture/CapturingOutput.php‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace WebProject\Codeception\Module\AiReporter\Tests\Support\Fixture;
6+
7+
use Codeception\Lib\Console\Output;
8+
9+
/**
10+
* Buffers console writes so tests can assert on inline AI Context output.
11+
*/
12+
final class CapturingOutput extends Output
13+
{
14+
private string $buffer = '';
15+
16+
public function __construct()
17+
{
18+
parent::__construct(['colors' => false, 'interactive' => false]);
19+
}
20+
21+
protected function doWrite(string $message, bool $newline): void
22+
{
23+
$this->buffer .= $message;
24+
if ($newline) {
25+
$this->buffer .= "\n";
26+
}
27+
}
28+
29+
public function fetch(): string
30+
{
31+
$out = $this->buffer;
32+
$this->buffer = '';
33+
34+
return $out;
35+
}
36+
}

‎tests/Support/Fixture/PathNormalizerFactory.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace WebProject\Codeception\Module\AiReporter\Tests\Support\Fixture;
6+
7+
use WebProject\Codeception\Module\AiReporter\Report\PathNormalizer;
8+
9+
final class PathNormalizerFactory
10+
{
11+
public static function make(bool $compactPaths = true, string $projectRoot = '/repo/project'): PathNormalizer
12+
{
13+
return new PathNormalizer($projectRoot, $compactPaths);
14+
}
15+
}

‎tests/Support/Fixture/StubTest.php‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace WebProject\Codeception\Module\AiReporter\Tests\Support\Fixture;
6+
7+
use Codeception\Test\Metadata;
8+
use Codeception\Test\Test;
9+
10+
/**
11+
* Minimal Codeception Test used to drive FailEvent dispatching in unit tests.
12+
*/
13+
final class StubTest extends Test
14+
{
15+
public function __construct(
16+
string $name,
17+
string $filename,
18+
private readonly string $signature,
19+
) {
20+
$metadata = new Metadata();
21+
$metadata->setName($name);
22+
$metadata->setFilename($filename);
23+
$this->setMetadata($metadata);
24+
}
25+
26+
public function test(): void
27+
{
28+
}
29+
30+
public function run(): void
31+
{
32+
}
33+
34+
public function toString(): string
35+
{
36+
return $this->getName();
37+
}
38+
39+
public function getSignature(): string
40+
{
41+
return $this->signature;
42+
}
43+
}

0 commit comments

Comments
 (0)