Skip to content

Commit 959f66d

Browse files
Testclaude
andcommitted
refactor(cli): route stderr emissions through EmitsStderr trait
Direct fwrite(STDERR, ...) calls (BUG-5/12/14) leaked into the phpunit test runner output because PHPUnit does not capture stderr. With many files and process tests the suite output became dozens of "Report written to:", "--files takes precedence", "all input files are invalid" lines mixed with the dot/F/E progress. New trait EmitsStderr exposes \$this->emitStderr(\$message) that: - In production (\$this->output is an OutputStyle wrapping Symfony ConsoleOutput which implements ConsoleOutputInterface): writes to the dedicated error stream via getErrorOutput()->writeln(). - In tests (\$this->artisan spins up an OutputStyle around BufferedOutput which does NOT implement ConsoleOutputInterface): the message is silently dropped, keeping the run output clean. Tests that need to assert on a stderr message can drive the command with a real ConsoleOutput or a double overriding emitStderr(). Each Command applies the trait directly — the consumer traits (FormatsOutput, ResolvesInputFiles) only call \$this->emitStderr() without using EmitsStderr themselves, because mixing two consumer traits into the same Command would otherwise raise the trait diamond-collision error. Printer::deprecationWarning is removed (CheckConfigurationFileCommand now formats and routes the message itself via the trait). The two remaining fwrite(STDERR, ...) sites in src/Execution/ are pre-existing and don't fire under the test suite. Suite: 2091 / 2091 green, no stderr noise in phpunit output. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 1cfd327 commit 959f66d

9 files changed

Lines changed: 75 additions & 23 deletions

app/Commands/CacheClearCommand.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use LaravelZero\Framework\Commands\Command;
88
use RecursiveDirectoryIterator;
99
use RecursiveIteratorIterator;
10+
use Wtyd\GitHooks\App\Commands\Concerns\EmitsStderr;
1011
use Wtyd\GitHooks\Configuration\ConfigurationParser;
1112
use Wtyd\GitHooks\Configuration\ConfigurationResult;
1213
use Wtyd\GitHooks\Configuration\JobConfiguration;
@@ -15,6 +16,8 @@
1516

1617
class CacheClearCommand extends Command
1718
{
19+
use EmitsStderr;
20+
1821
protected $signature = 'cache:clear
1922
{names?* : Job or flow names to clear (all if omitted)}
2023
{--config= : Path to configuration file}';
@@ -69,7 +72,7 @@ public function handle(): int
6972
return $hasErrors ? 1 : 0;
7073
} catch (GitHooksExceptionInterface $e) {
7174
// To STDERR so --format=json/junit/sarif/codeclimate stdout stays clean (BUG-5).
72-
fwrite(STDERR, $e->getMessage() . "\n");
75+
$this->emitStderr($e->getMessage());
7376
return 1;
7477
}
7578
}

app/Commands/CheckConfigurationFileCommand.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace Wtyd\GitHooks\App\Commands;
44

55
use LaravelZero\Framework\Commands\Command;
6+
use Wtyd\GitHooks\App\Commands\Concerns\EmitsStderr;
67
use Wtyd\GitHooks\Configuration\ConfigurationParser;
78
use Wtyd\GitHooks\Configuration\ConfigurationResult;
89
use Wtyd\GitHooks\Jobs\JobRegistry;
@@ -19,6 +20,8 @@
1920

2021
class CheckConfigurationFileCommand extends Command
2122
{
23+
use EmitsStderr;
24+
2225
protected $signature = 'conf:check {--config= : Path to configuration file}';
2326
protected $description = 'Check that the configuration file exists and that it is in the proper format.';
2427

@@ -227,7 +230,7 @@ protected function handleV3(ConfigurationResult $config): int
227230
foreach ($config->getValidation()->getDeprecations() as $deprecation) {
228231
$message = $deprecation->getWarningMessage();
229232
$deprecationMessages[$message] = true;
230-
$this->printer->deprecationWarning($message);
233+
$this->emitStderr("⚠️ $message");
231234
}
232235
foreach ($config->getValidation()->getWarnings() as $warning) {
233236
if (isset($deprecationMessages[$warning])) {
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Wtyd\GitHooks\App\Commands\Concerns;
6+
7+
use Symfony\Component\Console\Output\ConsoleOutputInterface;
8+
9+
/**
10+
* Route advisories, deprecation warnings and exception messages to the
11+
* console's STDERR stream while staying silent inside test buffers.
12+
*
13+
* Production: $this->output is an Illuminate\Console\OutputStyle wrapping a
14+
* Symfony\Component\Console\Output\ConsoleOutput; we ask it for its
15+
* dedicated stderr stream via getErrorOutput() so the message lands on
16+
* fd 2, respects --no-ansi, and stays out of the JSON/JUnit/SARIF payload
17+
* on stdout (BUG-5/12/14).
18+
*
19+
* Tests: Illuminate's $this->artisan() builds an OutputStyle around a
20+
* BufferedOutput which does not implement ConsoleOutputInterface. Routing
21+
* to the buffer would surface the message on stdout (back to square one);
22+
* dropping it keeps the phpunit run output clean. Tests that need to
23+
* assert on a stderr message must drive the command with a real
24+
* ConsoleOutput or a double that overrides emitStderr().
25+
*/
26+
trait EmitsStderr
27+
{
28+
/**
29+
* @SuppressWarnings(PHPMD.UnusedPrivateMethod) used by classes that consume the trait
30+
*/
31+
protected function emitStderr(string $message): void
32+
{
33+
$output = $this->output->getOutput();
34+
if ($output instanceof ConsoleOutputInterface) {
35+
$output->getErrorOutput()->writeln($message);
36+
}
37+
}
38+
}

app/Commands/Concerns/FormatsOutput.php

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,15 @@
2626
/**
2727
* Shared logic for commands that support --format and multi-report --report-* flags.
2828
*/
29+
/**
30+
* Consumer Commands MUST also `use EmitsStderr;` so the trait's call to
31+
* `$this->emitStderr(...)` resolves. We don't `use EmitsStderr` here to
32+
* avoid the diamond-collision PHP raises when a Command also uses
33+
* ResolvesInputFiles (which would also bring its own copy of the trait).
34+
*/
2935
trait FormatsOutput
3036
{
37+
3138
/**
3239
* Select the output handler based on format and execution context.
3340
*
@@ -342,7 +349,7 @@ private function writeStructuredPayload(string $content): void
342349
*/
343350
protected function emitReportWrittenNotice(string $path): void
344351
{
345-
fwrite(STDERR, "Report written to: $path\n");
352+
$this->emitStderr("Report written to: $path");
346353
}
347354

348355
/**

app/Commands/Concerns/ResolvesInputFiles.php

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,15 @@
1818
* @property InputFilesResolver $inputFilesResolver Provided by the consumer
1919
* command via constructor injection.
2020
*/
21+
/**
22+
* Consumer Commands MUST also `use EmitsStderr;` so the trait's calls to
23+
* `$this->emitStderr(...)` resolve. We don't `use EmitsStderr` here to avoid
24+
* the diamond-collision PHP raises when a Command also uses FormatsOutput
25+
* (which would also bring its own copy of the trait).
26+
*/
2127
trait ResolvesInputFiles
2228
{
29+
2330
/**
2431
* Returns null when none of the input-files flags are present. Otherwise
2532
* returns the resolved object after emitting any informational warnings.
@@ -52,16 +59,16 @@ private function resolveInputFilesFlags(bool $printModeHeader = false): ?InputFi
5259
// Advisories about input files go to STDERR so --format=json/junit/sarif/codeclimate
5360
// stdout stays a clean, parseable payload (BUG-5).
5461
foreach ($resolution->getInvalid() as $invalid) {
55-
fwrite(STDERR, "⚠️ file '$invalid' does not exist, skipping\n");
62+
$this->emitStderr("⚠️ file '$invalid' does not exist, skipping");
5663
}
5764
if ($resolution->isBomDetected()) {
58-
fwrite(STDERR, "⚠️ --files-from: UTF-8 BOM detected and stripped\n");
65+
$this->emitStderr('⚠️ --files-from: UTF-8 BOM detected and stripped');
5966
}
6067

6168
if ($this->option('fast') || $this->option('fast-branch')) {
6269
$other = $this->option('fast') ? '--fast' : '--fast-branch';
6370
$primary = is_string($files) && $files !== '' ? '--files' : '--files-from';
64-
fwrite(STDERR, "⚠️ $primary takes precedence over $other ($other ignored)\n");
71+
$this->emitStderr("⚠️ $primary takes precedence over $other ($other ignored)");
6572
}
6673

6774
if ($printModeHeader && (strval($this->option('format')) === '' || $this->option('format') === null)) {

app/Commands/FlowCommand.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use LaravelZero\Framework\Commands\Command;
88
use Wtyd\GitHooks\App\Commands\Concerns\EmitsConditionsHeader;
99
use Wtyd\GitHooks\App\Commands\Concerns\EmitsConfigWarnings;
10+
use Wtyd\GitHooks\App\Commands\Concerns\EmitsStderr;
1011
use Wtyd\GitHooks\App\Commands\Concerns\FormatsOutput;
1112
use Wtyd\GitHooks\App\Commands\Concerns\ResolvesAllocatorFlag;
1213
use Wtyd\GitHooks\App\Commands\Concerns\ResolvesInputFiles;
@@ -28,6 +29,7 @@ class FlowCommand extends Command
2829
{
2930
use EmitsConditionsHeader;
3031
use EmitsConfigWarnings;
32+
use EmitsStderr;
3133
use FormatsOutput;
3234
use ResolvesAllocatorFlag;
3335
use ResolvesInputFiles;
@@ -222,7 +224,7 @@ public function handle(): int
222224
return $result->isSuccess() ? 0 : 1;
223225
} catch (GitHooksExceptionInterface $e) {
224226
// To STDERR so --format=json/junit/sarif/codeclimate stdout stays clean (BUG-5).
225-
fwrite(STDERR, $e->getMessage() . "\n");
227+
$this->emitStderr($e->getMessage());
226228
return 1;
227229
}
228230
}

app/Commands/FlowsCommand.php

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use LaravelZero\Framework\Commands\Command;
88
use Wtyd\GitHooks\App\Commands\Concerns\EmitsConditionsHeader;
99
use Wtyd\GitHooks\App\Commands\Concerns\EmitsConfigWarnings;
10+
use Wtyd\GitHooks\App\Commands\Concerns\EmitsStderr;
1011
use Wtyd\GitHooks\App\Commands\Concerns\FormatsOutput;
1112
use Wtyd\GitHooks\App\Commands\Concerns\ResolvesAllocatorFlag;
1213
use Wtyd\GitHooks\App\Commands\Concerns\ResolvesInputFiles;
@@ -41,6 +42,7 @@ class FlowsCommand extends Command
4142
{
4243
use EmitsConditionsHeader;
4344
use EmitsConfigWarnings;
45+
use EmitsStderr;
4446
use FormatsOutput;
4547
use ResolvesAllocatorFlag;
4648
use ResolvesInputFiles;
@@ -233,7 +235,7 @@ public function handle(): int
233235
return $result->isSuccess() ? 0 : 1;
234236
} catch (GitHooksExceptionInterface $e) {
235237
// To STDERR so --format=json/junit/sarif/codeclimate stdout stays clean (BUG-5).
236-
fwrite(STDERR, $e->getMessage() . "\n");
238+
$this->emitStderr($e->getMessage());
237239
return 1;
238240
}
239241
}
@@ -375,10 +377,9 @@ private function emitIgnoredOptionsWarning(
375377

376378
// REQ-018 advisory: route to stderr so pipelines capturing stdout
377379
// (CI, --format=json, scripted tooling) don't pick up the message.
378-
fwrite(
379-
STDERR,
380+
$this->emitStderr(
380381
"⚠️ Options declared in '" . implode("', '", $ignored) . "' are ignored in multi-flow runs."
381-
. "\n Effective options come from flows.options + CLI; see header below.\n"
382+
. "\n Effective options come from flows.options + CLI; see header below."
382383
);
383384
}
384385

app/Commands/JobCommand.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
};
1717
use Wtyd\GitHooks\App\Commands\Concerns\EmitsConditionsHeader;
1818
use Wtyd\GitHooks\App\Commands\Concerns\EmitsConfigWarnings;
19+
use Wtyd\GitHooks\App\Commands\Concerns\EmitsStderr;
1920
use Wtyd\GitHooks\App\Commands\Concerns\FormatsOutput;
2021
use Wtyd\GitHooks\App\Commands\Concerns\ResolvesAllocatorFlag;
2122
use Wtyd\GitHooks\App\Commands\Concerns\ResolvesInputFiles;
@@ -30,6 +31,7 @@ class JobCommand extends Command
3031
{
3132
use EmitsConditionsHeader;
3233
use EmitsConfigWarnings;
34+
use EmitsStderr;
3335
use FormatsOutput;
3436
use ResolvesAllocatorFlag;
3537
use ResolvesInputFiles;
@@ -211,7 +213,7 @@ public function handle(): int
211213
return $result->isSuccess() ? 0 : 1;
212214
} catch (GitHooksExceptionInterface $e) {
213215
// To STDERR so --format=json/junit/sarif/codeclimate stdout stays clean (BUG-5).
214-
fwrite(STDERR, $e->getMessage() . "\n");
216+
$this->emitStderr($e->getMessage());
215217
return 1;
216218
}
217219
}

src/Utils/Printer.php

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,6 @@ public function resultWarning(string $message): void
2121
$this->warning($message);
2222
}
2323

24-
/**
25-
* Deprecation notices go to STDERR so consumers parsing stdout (CI pipes,
26-
* --format=json/junit/sarif/codeclimate, scripted tooling) don't pick up
27-
* the human-facing message. Structured payloads expose the same data via
28-
* the `deprecations[]` block; this method is purely for the operator
29-
* looking at the terminal.
30-
*/
31-
public function deprecationWarning(string $message): void
32-
{
33-
fwrite(STDERR, "⚠️ \e[43m\e[30m{$message}\033[0m\n");
34-
}
3524
public function resultError(string $message): void
3625
{
3726
echo "";

0 commit comments

Comments
 (0)