Repository navigation
Initialize headless terminal dimensions in Testbench and unit tests - #71
Conversation
…tion boot Testbench creates applications through its own bootstrap sequence and its console kernel has no bootstrappers. As a result, package tests missed the terminal initialization that normal HTTP and console applications already receive. Programmatic console calls could repeatedly spawn stty inside Swoole's hooked subprocess wait when no terminal dimensions were exported. Run ConfigureTerminalDimensions after environment loading and before configuration, matching the normal kernels. This also initializes console output before database test traits run commands during setup and teardown. Preserve dimensions loaded from the application's environment file. Add isolated regressions that clear inherited dimensions before refreshing the application and verify both defaults and environment-file values. The missing-default regression failed before the bootstrap call was added. Refs #686
Reuse the existing headless terminal policy through an application-free configure() method. Invoke it before test execution enters either the framework coroutine or a test-managed coroutine, so unit tests that never boot an application receive the same protection as application tests. Symfony Process does not reliably forward putenv-only values, so each test process establishes its own defaults without requiring PHPUnit XML pins. Keep explicit dimensions and real TTY behavior unchanged. Document why failed Symfony dimension probes repeat, their exposure to Swoole's child process wait bug, and the process-wide lifetime of environment defaults. Add isolated regressions for both coroutine modes, partial dimension configuration, and a real pseudo-terminal. The unit-test regressions failed before initialization was added and pass without PHPUnit configuration. Retain the components suite's forced XML dimensions solely for consistent console-output assertions and replace the old hang-prevention comment. Refs #686 Related: #454
A unit test can initialize COLUMNS and LINES before an application test loads its environment file. Dotenv treats those process values as external, so the later application's configured dimensions are ignored. This makes console formatting depend on test order even with normal framework state cleanup enabled. Track only the two variable names populated by ConfigureTerminalDimensions and remove them through the existing after-each-test subscriber. Values provided before initialization, including shell and PHPUnit XML settings, are never owned or removed. Clear the ownership set after cleanup so a subsequent test's explicit dimensions survive later cleanup passes. Add an isolated regression that failed before the fix and verifies both owned-default removal and explicit-value preservation. Also verify the unit-before-application reproduction now honors the application's env-file width without terminal settings in PHPUnit XML. Refs #686
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Configure Symfony terminal dimensions src/foundation/src/Bootstrap/ConfigureTerminalDimensions.php |
The bootstrapper adds configure(). For non-TTY output, it sets Symfony Terminal’s cached width and height to 80 and 24 only when each cached value is null. |
Invoke terminal setup during tests and application creation src/foundation/src/Testing/Concerns/RunTestsInCoroutine.php, src/testbench/src/Concerns/CreatesApplication.php |
Test invocations configure terminal dimensions before coroutine handling or PHPUnit delegation. Testbench application creation invokes the bootstrapper after environment resolution and before configuration. |
Validate terminal sizing across test environments tests/Foundation/Bootstrap/ConfigureTerminalDimensionsTest.php, tests/Foundation/Fixtures/configure-terminal-dimensions.php, tests/Testbench/TerminalDimensionsTest.php, tests/Testing/TerminalDimensionsTest.php, phpunit.xml.dist |
Tests cover cached and environment dimensions, PTY behavior, bootstrap order, coroutine modes, and Testbench environment loading. phpunit.xml.dist changes display_errors to stderr; the terminal-dimensions values remain unchanged. |
Test-command configuration handling
| Layer / File(s) | Summary |
|---|---|
Add the configuration shortcut and update guidance src/testing/src/Console/TestCommand.php, src/testbench/src/Foundation/Console/TestCommand.php, src/docs/porting-from-laravel.md, src/docs/testbench.md, src/testbench/README.md |
Both test commands use -c as the shortcut for --configuration. The documentation describes the alias and removes references to the unused --custom-argument option. |
Resolve and forward PHPUnit configuration src/testing/src/Console/TestCommandBase.php |
Configuration lookup supports ordered default filenames, explicit paths, and directories. Argument filtering handles the configuration shortcut. ParaTest cache handling checks explicit options and the selected XML configuration. |
Test configuration selection and runner options tests/Testing/Console/TestCommandTest.php, tests/Testing/ParallelRunnerTest.php |
Tests cover configuration option forms, path resolution, default precedence, profiling, cache-directory options, and preserving active PHPUnit configuration when constructing ParaTest options. |
Priority:
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Bug fix · Severity of issue fixed: Medium
Merge Risk: 
191c8
-
The terminal initialization and its regression test cover the supported test workflow; no identified issue needs resolution before merge.
Pre-merge checks: 
-
Check name Status Explanation Resolution Linked Issues check #686requires headless Testbench applications to provideCOLUMNS=80andLINES=24, as shown by its expected behavior and reproduction.ConfigureTerminalDimensions::configure()now writes only Sym…Either update the linked issue's requirements to accept Symfony cache initialization as the replacement behavior, or set the required missing COLUMNSandLINESenvironment variables in the headless Testbench path and retain regression c…Out of Scope Changes check The terminal bootstrap changes, Testbench integration, per-process initialization, and related regression tests directly support #686. The changes to PHPUnit configuration selection and forwarding in …Remove the independent PHPUnit/Testbench command behavior changes from this pull request, or track them in a separate linked issue and pull request. Keep only changes required to initialize headless terminal dimensions and to test that beha… Docstring Coverage Docstring coverage is 45.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 40 functions across 12 files. (4 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Title check The title clearly summarizes the primary change: initializing headless terminal dimensions in Testbench and unit tests. Description check The description clearly explains the problem, implementation, behavior, regression coverage, related scope, and extensive verification results. It does not explicitly select a contribution type or mar… Full details: Description check
-
Explanation
The description clearly explains the problem, implementation, behavior, regression coverage, related scope, and extensive verification results. It does not explicitly select a contribution type or mark the submission checklist, but the required technical content is otherwise substantially complete.
-
Full details: Linked Issues check
-
Explanation
#686requires headless Testbench applications to provideCOLUMNS=80andLINES=24, as shown by its expected behavior and reproduction.ConfigureTerminalDimensions::configure()now writes only SymfonyTerminal::$widthandTerminal::$heightthrough reflection. It never writes those environment variables. The implementation does satisfy the no-sttygoal, preserves populated cache entries, runs after environment loading inCreatesApplication, and initializes application-free test processes, but it does not meet the issue's explicit environment-variable behavior.Resolution
Either update the linked issue's requirements to accept Symfony cache initialization as the replacement behavior, or set the required missing
COLUMNSandLINESenvironment variables in the headless Testbench path and retain regression coverage for that behavior.
Full details: Out of Scope Changes check
-
Explanation
The terminal bootstrap changes, Testbench integration, per-process initialization, and related regression tests directly support
#686. The changes to PHPUnit configuration selection and forwarding inTestCommandBase, removal of Testbench's--custom-argument, parallel-runner configuration preservation, and the general isolated-childdisplay_errorschange address separate test-command behavior. The supplied evidence describes these as fixes for an existing test-order and error-reporting problem, not as requirements for terminal-dimension initialization.Resolution
Remove the independent PHPUnit/Testbench command behavior changes from this pull request, or track them in a separate linked issue and pull request. Keep only changes required to initialize headless terminal dimensions and to test that behavior.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 45.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 40 functions across 12 files. (4 skipped: 4 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
PR Summary by QodoInitialize headless terminal dimensions in Testbench and unit tests
AI Description
Diagram
High-Level Assessment
Files changed (10)
|
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
 [Medium impact] The PR appears safe to merge; both previous findings are addressed, and no new actionable issues were found. SummaryThe PR initializes headless terminal dimensions in Testbench and application-free tests.
No new actionable issues were found. Reviews (2) · Last reviewed commit: "test: report isolated child startup erro..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Writing fallback COLUMNS and LINES into the process environment makes immutable dotenv loading mistake framework defaults for explicit settings. Tracking variable names for cleanup also removes values restored by test environment helpers. Those values share a channel, so cleanup cannot reliably infer which code owns them. Seed only the missing width and height entries in Symfony Terminal's process-local cache when stdout is headless. Symfony continues to prefer explicit environment values on every lookup, including values loaded after initialization. Existing cached dimensions and real terminals are left alone. No environment writes, ownership state, or test cleanup are needed. Use reflection at this single boundary because Symfony exposes no cache setter. Document the reason and test both cache entries before accessing terminal dimensions, so Symfony's native width fallback cannot conceal a failed probe or a dependency change. Remove the obsolete cleanup hook and its test, and retain per-process initialization for Testbench and unit tests. Cover missing, partial, explicit, temporarily restored, and later dotenv values, real PTY behavior, both coroutine modes, and initialization before provider registration. Terminal regressions pass without PHPUnit XML or the cleanup extension. Full composer fix passed, including formatting, source and type-fixture analysis, framework tests, Testbench package mode, and both dogfood suites. Focused checks passed after the final test-only refinements. Refs #686
The provider-order regression already verifies that both kernels include the terminal bootstrapper before comparing its position with provider registration. Remove the separate inclusion test, which checks the same condition without covering another behavior. The focused bootstrap suite passes with 7 tests and 37 assertions. Targeted formatting and git diff --check are clean.
Building parallel command arguments called ParaTest's Options factory only to inspect its cache setting. That factory replaces PHPUnit's active configuration, causing later isolated tests to load the command fixture's bootstrap instead of the suite bootstrap. Test order determined whether the temporary autoloader still existed, producing intermittent CI failures. Inspect the CLI cache option and selected XML directly, leaving runner initialization to the child process. Preserve explicit cache paths and explain the configuration side effect beside the argument-building code. Use PHPUnit's default configuration filename order, search explicit configuration directories, preserve relative and absolute paths, and forward missing explicit paths for PHPUnit to reject instead of silently running a different configuration. Make -c the configuration shortcut for both application and package tests. Remove the original short or long selector before forwarding so it cannot override the generated profile configuration. Remove Testbench's unused custom-argument flag and document that difference in the source, README, command reference, and porting guide. Regression coverage preserves PHPUnit configuration identity and exercises cache options, discovery, missing paths, profiling, and real Symfony input for both commands. The relevant failures were reproduced before fixing them. Validation: command tests (31 tests, 165 assertions), Testing suite (696 tests, 3017 assertions), Testbench package mode (647 tests, 2103 assertions, 3 skips), PHPStan source/type fixtures, and changed-file style checks all pass. The isolated-test CI failure sequence also passes.
Construct ParaTest Options directly with PHPUnit's active configuration instead of calling the CLI factory, which replaces the process-wide configuration and emits a runner-configuration event during a test. Keep the requested process count and fixture settings explicit through named constructor arguments. Explain both the avoided global side effect and the dependency on ParaTest's internal constructor beside the fixture. Add an identity assertion to the existing process-token test so returning to the mutating factory fails immediately. The assertion failed before the change. Validation: all 12 parallel-runner tests (48 assertions) and the affected Testing and Testbench package-mode suites pass; style and static analysis checks are clean.
PHPUnit sets display_errors to stderr before booting isolated children, but the suite XML changed it back to on, which sends errors to stdout. PHPUnit does not include that stdout in an unexpected-child-exit error. With error logging disabled in CI, a missing bootstrap therefore produced only "Test was run in child process and ended unexpectedly". Keep displayed errors on stderr and explain why in the suite configuration. This exposes the actual startup error without changing terminal settings or adding custom child-process reporting code. Verified with the exact failing CI image: the old source and a five-test sequence produce two generic child-exit errors with display_errors=on; changing only this setting exposes both missing-autoloader fatal messages. The complete fixed sequence passes. The full four-worker framework suite with stderr enabled passes: 52899 tests, 188711 assertions, 4180 skips.
|
Follow-up to the review and summary: the final implementation is pushed at 191c811, and both inline findings have individual replies. The cleanup finding was correct. Terminal defaults now live only in Symfony's cache; environment writes, ownership tracking, and cleanup are removed. Regression coverage verifies that restored explicit dimensions survive and that later environment files can supply dimensions. The proposed bridge from a Testbench environment repository with putenv disabled into the process environment is not appropriate: disabling that adapter deliberately keeps those channels separate, while Symfony reads getenv(). The new initializer does not write process defaults, and explicit process dimensions still take precedence. The PR description now includes the final design, the isolated-process CI root cause and fix, and verification results. |
|
Both findings in the Greptile review were confirmed and fixed in 4d447c6, now included in the pushed head 191c811. Each inline finding has an individual reply. Fallback dimensions are stored in Symfony Terminal's cache without changing the environment. This removes both causes: later dotenv loads are no longer masked, and there is no ownership cleanup that can delete restored explicit values. Regression tests exercise both sequences. No additional environment-restoration mechanism is needed. |
|
@coderabbitai review |
✅ Action performed
|
|
Reviewed the final pre-merge warnings:
No source changes are needed for these three warnings. |
|
The remaining PHP 8.5 failure is a pre-existing coroutine timeout race, unrelated to this PR. The timeout test and The coroutine deadline can enter its timeout catch block just before PHPUnit's native SIGALRM arrives. That signal then throws from I reproduced the same failure on the base commit This PR is being merged with that unrelated failure understood. The timeout fix will be handled on the maintenance branch. |
Fixes hypervel/components#686.
Testbench's manual application bootstrap omitted
ConfigureTerminalDimensions, even though the normal HTTP and console kernels run it. In a headless process, Symfony does not cache failed terminal-size probes, so repeated console calls can repeatedly spawnsttyand enter Swoole's affected subprocess-wait path.The bootstrapper now fills Symfony Terminal's missing dimension-cache entries with 80×24 when stdout is headless. It leaves real terminals and already populated cache entries alone. Symfony's own accessors continue to prefer explicit
COLUMNSandLINESvalues, including values loaded after initialization.The fallback never writes to the environment. This avoids masking later environment files, changing values restored by temporary-environment helpers, or requiring cleanup between tests. The earlier environment-ownership tracking and subscriber cleanup have been removed entirely.
Symfony exposes no dimension-cache setter, so this integration uses reflection in one place. Regression tests check both cache entries and the public terminal behavior, making changes to the dependency's private properties visible during upgrades.
Testbench invokes the initializer during application creation before configuration and provider execution. The shared test-invocation path also initializes every PHPUnit/ParaTest process, including application-free tests and tests that manage their own coroutines. Applications and packages do not need terminal settings in PHPUnit XML for this protection.
The components suite retains its existing XML dimensions solely to stabilize console-output wrapping assertions. Their comment now explains that purpose. This change prevents unnecessary terminal probes; it does not repair Swoole's general subprocess-wait defect, tracked in swoole/swoole-src#6328.
The new isolated tests also exposed an existing test-order failure. Building parallel test-command arguments called
Options::fromConsoleInput(), which replaced PHPUnit's active configuration with the command's configuration. Later isolated tests then tried to require a temporary Testbench autoloader that another test had removed. The shared command now reads the cache setting without initializing a runner; the parallel-runner test fixture also preserves the active configuration. Both paths have explanatory comments and regression assertions. Default XML discovery now follows PHPUnit's filename order, includingphpunit.dist.xml, and explicit relative and absolute configuration paths are preserved.Explicit configuration directories use the same filename order, and missing explicit paths are passed to PHPUnit to reject instead of silently selecting the default config. Both test commands recognize
-cas--configurationand remove the original selector before forwarding arguments, so it cannot override the generated profile configuration or select a different file for the cache check. Testbench's unused--custom-argumentflag is removed; this difference is documented beside the signature and in the Testbench and porting documentation.The suite sends displayed PHP errors to stderr so PHPUnit includes isolated-child startup errors in its failure messages. The original setting sent them to stdout, hiding the missing-bootstrap fatal behind “ended unexpectedly.” This was reproduced and verified using the exact failing CI image.
Regression coverage includes:
testEnvironmentFileOverridesEarlierTerminalDefaults: a later environment file wins after fallbacks have already been initialized.Validation completed sequentially:
composer fixpassed for the terminal-cache change: formatting, source and type-fixture PHPStan, the full parallel framework suite, Testbench package mode, and both dogfood suites.--no-configuration --bootstrap vendor/autoload.php, without PHPUnit XML or the cleanup extension.git diff --checkis clean.The environment-mutation regressions were observed failing against the previous implementation before the cache change. No dependency manifests or lockfiles are changed.
Note
Initialize headless terminal dimensions via Symfony cache instead of env vars
ConfigureTerminalDimensions::configurenow sets Symfony Console's cached width/height fallbacks through reflection when output is headless. It no longer writesCOLUMNS/LINESinto the process environment, and existing cached values are preserved.-cas a shortcut for--configuration. Forwarded args and ParaTest construction can no longer override the selected configuration. TestCommandBase.phpCOLUMNS/LINESenv vars still wins;package:testno longer declares the custom-argument option, and child PHP errors go to stderr per phpunit.xml.dist.Macroscope summarized 191c811.
Summary by CodeRabbit
-cas a shortcut for--configuration. Configuration can be selected by file or directory, with default PHPUnit configuration files searched in order.