Skip to content

Include the exit code and stderr when a bin/rector output assertion fails - #8440

Open
SanderMuller wants to merge 1 commit into
rectorphp:mainfrom
SanderMuller:rector-test-failure-context
Open

Include the exit code and stderr when a bin/rector output assertion fails#8440
SanderMuller wants to merge 1 commit into
rectorphp:mainfrom
SanderMuller:rector-test-failure-context

Conversation

@SanderMuller

Copy link
Copy Markdown
Contributor

testConsoleOutput asserts on getOutput() alone, so when the spawned bin/rector writes nothing to stdout the failure is a diff against an empty string with no reason attached. The exit code and anything on stderr are both dropped.

That is exactly what a failing run looks like in PHPStan's integration job, where all three cases report '' including the --version one:

1) ...testConsoleOutput@Version with data ('/usr/bin/php8.4 bin/rector --version', 'Rector @package_version@\n')
-'Rector @package_version@
+''

Nothing there says whether the process failed to start, fataled, or exited non-zero.

assertSame() takes a message, so this puts both in the failure:

exit code 0, stderr:
Failed asserting that two strings are identical.

No new test: the change is in the assertion itself, and I checked it renders by pointing one expectation at a deliberately wrong string. tests/Bin stays green (3 tests, 3 assertions), vendor/bin/ecs is clean on the file, and vendor/bin/phpstan analyse tests/Bin/RectorTest.php reports no errors.

getExitCode() is nullable, so it is rendered with %s and a fallback rather than %d, which would print a misleading 0 for a process that never started.

…ails

testConsoleOutput asserts on getOutput() alone, so when the spawned
bin/rector writes nothing to stdout the failure is a diff against an empty
string with no reason attached - the exit code and whatever went to stderr
are both dropped.

That is what a failing run looks like in PHPStan's integration job, where
all three cases report '' including the --version one, and the log gives
nothing to work from.

assertSame() takes a message, so both are now in the failure.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants