Fixed installer warning when ran over invalid .env.
#1887
Conversation
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant Test as PHPUnit Test
participant Env as Env::parseDotenv
participant File as .env File
participant ErrorHandler as Custom Error Handler
Test->>Env: Call parseDotenv(filename)
Env->>File: Read contents
alt File not readable
Env-->>Test: Return []
else File readable
Env->>ErrorHandler: Install error handler
Env->>Env: parse_ini_string(contents)
ErrorHandler-->>Env: Capture errors/warnings
Env->>ErrorHandler: Restore error handler
alt Parsing failed
Env-->>Test: Throw RuntimeException with errors
else Parsing succeeded
Env-->>Test: Return parsed array
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 phpcs (3.7.2).vortex/installer/src/Utils/Env.phpERROR: Referenced sniff "Drupal" does not exist Run "phpcs --help" for usage information .vortex/installer/tests/Unit/EnvTest.phpERROR: Referenced sniff "Drupal" does not exist Run "phpcs --help" for usage information ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
.vortex/installer/src/Utils/Env.php(1 hunks).vortex/installer/tests/Unit/EnvTest.php(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: the file utility class in .vortex/installer/src/utils/file.php extends alexskrypnyk\file\file, and m...
Learnt from: AlexSkrypnyk
PR: drevops/vortex#1693
File: .vortex/installer/src/Prompts/Handlers/Internal.php:52-52
Timestamp: 2025-06-01T08:10:15.903Z
Learning: The File utility class in .vortex/installer/src/Utils/File.php extends AlexSkrypnyk\File\File, and methods like collapseRepeatedEmptyLines() are available from this external parent class dependency.
Applied to files:
.vortex/installer/src/Utils/Env.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: build (1)
- GitHub Check: build (0)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-docs
🔇 Additional comments (5)
.vortex/installer/tests/Unit/EnvTest.php (5)
203-223: Well-structured test method with comprehensive exception handling.The test properly covers both success and failure scenarios for
parseDotenv, with appropriate exception expectations and cleanup.
225-243: Comprehensive test data covering valid and invalid .env scenarios.The data provider effectively tests various .env formats including quoted values, comments, and invalid syntax that should trigger exceptions.
245-262: Thorough testing of file accessibility edge cases.Both test methods properly verify the fallback behavior when
.envfiles are not accessible, with appropriate cleanup in the permission manipulation test.
264-290: Comprehensive testing of value conversion functionality.The test method and data provider thoroughly cover all conversion scenarios in the
toValuemethod, with appropriate strict assertions.
292-301: Simple and effective test for environment variable setting.The test properly verifies the
putmethod functionality with appropriate cleanup.
| set_error_handler(function ($errno, $errstr, $errfile, $errline) use (&$errors): bool { | ||
| $errors[] = [ | ||
| 'errno' => $errno, | ||
| 'message' => $errstr, | ||
| 'file' => $errfile, | ||
| 'line' => $errline, | ||
| ]; | ||
|
|
||
| return TRUE; | ||
| }); | ||
|
|
||
| $result = parse_ini_string($contents); | ||
|
|
||
| restore_error_handler(); | ||
|
|
||
| if ($result === FALSE) { | ||
| $message = array_reduce($errors ?? [], function (string $carry, array $error): string { | ||
| return $carry . $error['message'] . PHP_EOL; | ||
| }, ''); | ||
|
|
||
| throw new \RuntimeException(sprintf('Unable to parse file %s: %s', $filename, $message)); | ||
| } | ||
|
|
||
| return $result; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Excellent error handling implementation with minor initialization improvement needed.
The custom error handler implementation effectively captures and reports parsing errors, which directly addresses the PR objective of fixing installer warnings with invalid .env files.
Consider initializing the $errors variable for clarity:
+ $errors = [];
set_error_handler(function ($errno, $errstr, $errfile, $errline) use (&$errors): bool {This makes the code more explicit about the variable's initialization, even though the null coalescing operator on line 113 handles the undefined case.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| set_error_handler(function ($errno, $errstr, $errfile, $errline) use (&$errors): bool { | |
| $errors[] = [ | |
| 'errno' => $errno, | |
| 'message' => $errstr, | |
| 'file' => $errfile, | |
| 'line' => $errline, | |
| ]; | |
| return TRUE; | |
| }); | |
| $result = parse_ini_string($contents); | |
| restore_error_handler(); | |
| if ($result === FALSE) { | |
| $message = array_reduce($errors ?? [], function (string $carry, array $error): string { | |
| return $carry . $error['message'] . PHP_EOL; | |
| }, ''); | |
| throw new \RuntimeException(sprintf('Unable to parse file %s: %s', $filename, $message)); | |
| } | |
| return $result; | |
| $errors = []; | |
| set_error_handler(function ($errno, $errstr, $errfile, $errline) use (&$errors): bool { | |
| $errors[] = [ | |
| 'errno' => $errno, | |
| 'message' => $errstr, | |
| 'file' => $errfile, | |
| 'line' => $errline, | |
| ]; | |
| return TRUE; | |
| }); | |
| $result = parse_ini_string($contents); | |
| restore_error_handler(); | |
| if ($result === FALSE) { | |
| $message = array_reduce($errors ?? [], function (string $carry, array $error): string { | |
| return $carry . $error['message'] . PHP_EOL; | |
| }, ''); | |
| throw new \RuntimeException(sprintf('Unable to parse file %s: %s', $filename, $message)); | |
| } | |
| return $result; |
🤖 Prompt for AI Agents
In .vortex/installer/src/Utils/Env.php around lines 97 to 120, initialize the
$errors variable as an empty array before setting the custom error handler. This
makes the code clearer and more explicit about the variable's initialization,
avoiding reliance on the null coalescing operator later. Simply add $errors =
[]; before the set_error_handler call.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1887 +/- ##
===========================================
+ Coverage 78.37% 78.70% +0.32%
===========================================
Files 85 85
Lines 5003 5019 +16
Branches 35 35
===========================================
+ Hits 3921 3950 +29
+ Misses 1082 1069 -13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
AlexSkrypnyk
added
the
AUTOMERGE
Summary by CodeRabbit
Bug Fixes
Tests