Added '--prompts' CLI option to separate prompt answers from installer configuration.#2435
Open
AlexSkrypnyk wants to merge 2 commits intomainfrom
Open
Added '--prompts' CLI option to separate prompt answers from installer configuration.#2435AlexSkrypnyk wants to merge 2 commits intomainfrom
AlexSkrypnyk wants to merge 2 commits intomainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.vortex/installer/src/Prompts/PromptManager.php:
- Around line 676-679: The current check prevents normalizeValue() from running
for empty-string defaults, so multiselect `''` never becomes `[]`; in
PromptManager.php call `$handler->normalizeValue($default)` before filtering out
`''` (i.e., if `$default` is not null, normalize it first, then only set
`$args['default']` when the normalized value is not an empty string) so
multiselect empty defaults are converted to an empty array and propagated.
In @.vortex/installer/tests/Unit/Handlers/AbstractHandlerNormalizeValueTest.php:
- Around line 25-57: Add a regression test that exercises the full PromptManager
resolution flow — specifically call PromptManager::args(...) to trigger the
fallback-to-short-key logic and ensure PromptManager calls
handler->normalizeValue(...) before assigning prompt defaults (covering the
behavior fixed at the args() fallback + normalization steps). In practice,
create a new test method that constructs Config::fromString('{}') and
PromptManager, invokes $prompt_manager->args(...) with an input that only
matches the handler via its short key (and with values needing normalization,
e.g., comma-separated string for Modules::id() or Services::id()), and assert
the returned args contain the normalized array values (not raw strings) and
correct defaults; reference PromptManager::args, handler->normalizeValue, and a
handler id like Modules::id() or Services::id() to locate the code to exercise.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 82c9d16e-5875-468f-8659-6cc2c42072d8
📒 Files selected for processing (4)
.vortex/installer/src/Prompts/Handlers/AbstractHandler.php.vortex/installer/src/Prompts/Handlers/HandlerInterface.php.vortex/installer/src/Prompts/PromptManager.php.vortex/installer/tests/Unit/Handlers/AbstractHandlerNormalizeValueTest.php
Comment on lines
+25
to
+57
| #[DataProvider('dataProviderNormalizeValue')] | ||
| public function testNormalizeValue(string $handler_id, mixed $input, mixed $expected): void { | ||
| $config = Config::fromString('{}'); | ||
| $prompt_manager = new PromptManager($config); | ||
| $handlers = $prompt_manager->getHandlers(); | ||
|
|
||
| $this->assertArrayHasKey($handler_id, $handlers); | ||
|
|
||
| $handler = $handlers[$handler_id]; | ||
| $this->assertSame($expected, $handler->normalizeValue($input)); | ||
| } | ||
|
|
||
| public static function dataProviderNormalizeValue(): \Iterator { | ||
| // MultiSelect: comma-separated string converted to array. | ||
| yield 'modules - comma string' => [Modules::id(), 'admin_toolbar,coffee', ['admin_toolbar', 'coffee']]; | ||
| // MultiSelect: single string converted to single-element array. | ||
| yield 'modules - single string' => [Modules::id(), 'admin_toolbar', ['admin_toolbar']]; | ||
| // MultiSelect: array passed through unchanged. | ||
| yield 'modules - array' => [Modules::id(), ['admin_toolbar', 'coffee'], ['admin_toolbar', 'coffee']]; | ||
| // MultiSelect: empty string converted to empty array. | ||
| yield 'modules - empty string' => [Modules::id(), '', []]; | ||
|
|
||
| // Other multiselect handlers. | ||
| yield 'services - single string' => [Services::id(), 'redis', ['redis']]; | ||
| yield 'services - comma string' => [Services::id(), 'redis,solr', ['redis', 'solr']]; | ||
| yield 'deploy_types - single string' => [DeployTypes::id(), 'lagoon', ['lagoon']]; | ||
| yield 'notification_channels - single string' => [NotificationChannels::id(), 'email', ['email']]; | ||
|
|
||
| // Text handler: string passed through unchanged. | ||
| yield 'name - string passthrough' => [Name::id(), 'My Project', 'My Project']; | ||
| // Text handler: array passed through unchanged (not its concern). | ||
| yield 'name - array passthrough' => [Name::id(), ['a'], ['a']]; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add a PromptManager-level regression test for the fixed path.
This suite validates handler normalization well, but it does not cover the actual resolution flow in PromptManager::args() (Line 651 fallback to short key + Line 677 normalization before prompt default assignment). A focused regression test there would lock in the bugfix behavior end-to-end.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.vortex/installer/tests/Unit/Handlers/AbstractHandlerNormalizeValueTest.php
around lines 25 - 57, Add a regression test that exercises the full
PromptManager resolution flow — specifically call PromptManager::args(...) to
trigger the fallback-to-short-key logic and ensure PromptManager calls
handler->normalizeValue(...) before assigning prompt defaults (covering the
behavior fixed at the args() fallback + normalization steps). In practice,
create a new test method that constructs Config::fromString('{}') and
PromptManager, invokes $prompt_manager->args(...) with an input that only
matches the handler via its short key (and with values needing normalization,
e.g., comma-separated string for Modules::id() or Services::id()), and assert
the returned args contain the normalized array values (not raw strings) and
correct defaults; reference PromptManager::args, handler->normalizeValue, and a
handler id like Modules::id() or Services::id() to locate the code to exercise.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2435 +/- ##
==========================================
- Coverage 79.71% 79.31% -0.41%
==========================================
Files 127 120 -7
Lines 6832 6666 -166
Branches 44 0 -44
==========================================
- Hits 5446 5287 -159
+ Misses 1386 1379 -7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
AlexSkrypnyk
force-pushed
the
feature/config-no-interaction
branch
from
6b03d39 to
9ebc30b
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
AlexSkrypnyk
force-pushed
the
feature/config-no-interaction
branch
from
9ebc30b to
d201824
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
.vortex/installer/tests/Functional/Handlers/ModulesHandlerProcessTest.php (1)
14-101: 🛠️ Refactor suggestion | 🟠 MajorKeep one
Modulescase on the config/env path.These fixtures now inject native arrays via
$test->prompts, so they no longer exercise either regression this PR is fixing: resolving the shortMODULESconfig key in--no-interactionmode, or normalizing a string default like'pathauto,redirect'before the multiselect prompt sees it. Please keep one end-to-end case that drivesDrevOps\VortexInstaller\Prompts\Handlers\Modulesthrough config/env instead of--prompts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.vortex/installer/tests/Functional/Handlers/ModulesHandlerProcessTest.php around lines 14 - 101, Update the dataProviderHandlerProcess test set to retain one end-to-end case that exercises the Modules handler via config/env rather than via injected prompts: keep a yield entry that does not set $test->prompts[Modules::id()] but instead relies on reading the MODULES config/env value so DrevOps\VortexInstaller\Prompts\Handlers\Modules resolves the short MODULES key in non-interactive mode and normalizes string defaults like 'pathauto,redirect'; locate dataProviderHandlerProcess and the Modules::id() usages and replace one of the prompt-driven yields with a case that leaves $test->prompts unset and asserts the same module absences through the normal config/env code path..vortex/installer/tests/Functional/Handlers/MigrationDownloadSourceHandlerProcessTest.php (1)
18-82:⚠️ Potential issue | 🟠 MajorUse uppercase boolean constants to satisfy the PHPCS rule.
These setups use lowercase
true; the active coding standard in this PR expectsTRUE/FALSE, which can fail checks.🔧 Suggested fix
- $test->prompts[Migration::id()] = true; + $test->prompts[Migration::id()] = TRUE; ... - $test->prompts[Migration::id()] = true; + $test->prompts[Migration::id()] = TRUE; ... - $test->prompts[Migration::id()] = true; + $test->prompts[Migration::id()] = TRUE; ... - $test->prompts[Migration::id()] = true; + $test->prompts[Migration::id()] = TRUE; ... - $test->prompts[Migration::id()] = true; + $test->prompts[Migration::id()] = TRUE; ... - $test->prompts[Migration::id()] = true; + $test->prompts[Migration::id()] = TRUE;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.vortex/installer/tests/Functional/Handlers/MigrationDownloadSourceHandlerProcessTest.php around lines 18 - 82, The test data providers use lowercase boolean literals (e.g. in static::cw closures setting $test->prompts[Migration::id()] = true and other prompt assignments) which violates the PHPCS rule that requires uppercase boolean constants; update all occurrences of lowercase true/false in these closures (including assignments for Migration::id(), MigrationDownloadSource::id(), and MigrationImage::id() setups) to use TRUE and FALSE so the tests conform to the coding standard..vortex/installer/src/Prompts/PromptManager.php (1)
692-708:⚠️ Potential issue | 🔴 CriticalThis precedence drops the existing config/env prompt path entirely.
Once this block only switches between
--prompts, discovery,$default_override, and handler defaults, prompt answers supplied through the existing--config/ env flow can never reach the prompt defaults anymore. That leaves documented non-interactive input paths ignored even when the config keys are correct.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.vortex/installer/src/Prompts/PromptManager.php around lines 692 - 708, The prompt-default precedence in PromptManager.php currently checks $default_from_prompts, $default_from_discovery, $default_override, and $default_from_handler but omits the existing config/env prompt source; reintroduce the config/env check (the variable used for that flow—e.g. $default_from_config or $default_from_env in this method) into the precedence chain so the order becomes: $default_from_prompts, then the config/env default, then $default_from_discovery, then $default_override, then $default_from_handler; update the if/elseif block around $default_from_prompts / $default_from_discovery to include this additional branch accordingly..vortex/installer/src/Schema/SchemaValidator.php (2)
53-63:⚠️ Potential issue | 🟠 MajorStill validate values behind
_systemdependencies.When
checkDependency()returns'skip', the supplied value is copied straight intoresolvedandvalidateType()never runs. An invalid value for a system-gated prompt will therefore pass--validateand only fail later.🛡️ Suggested fix
if ($dep_result === 'skip') { - if ($has_value) { - $resolved[$id] = $value; - } - continue; + if (!$has_value) { + continue; + } + + $type_error = $this->validateType($handler, $value); + if ($type_error !== NULL) { + $errors[] = [ + 'prompt' => $id, + 'message' => $type_error, + ]; + continue; + } + + $resolved[$id] = $value; + continue; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.vortex/installer/src/Schema/SchemaValidator.php around lines 53 - 63, When checkDependency(...) returns 'skip' you currently copy $value into $resolved without type validation; update the block handling $dep_result === 'skip' in SchemaValidator (around the dependsOn/checkDependency logic) to still call $this->validateType($id, $value, $handler, $normalized) when $has_value before assigning to $resolved, and only set $resolved[$id] = $value if validateType succeeds (or propagate/collect the validation error the same way as for non-skipped prompts); keep the existing continue behavior for truly skipped-without-value cases.
79-92:⚠️ Potential issue | 🟠 Major
isRequired()is only enforced for dependent prompts now.This path skips every omitted prompt, so any required handler without
depends_onis silently accepted as missing.--validatecan therefore report success for an input set that the non-interactive install still cannot satisfy once defaults/discovery are exhausted.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.vortex/installer/src/Schema/SchemaValidator.php around lines 79 - 92, When skipping prompts not provided in input, required handlers that do not have a dependency are currently ignored; update the block that follows "if (!$has_value) { continue; }" to instead check if the handler is required and has no dependency and then push an error. Specifically, inside SchemaValidator (use the existing $handler and isRequired()), replace the unconditional skip with: if (!$has_value) { if ($handler->isRequired() && /* no depends_on set for this handler */) { add the same error entry to $errors with the handler id/message } continue; } so required prompts without depends_on are treated as validation errors.
♻️ Duplicate comments (1)
.vortex/installer/src/Prompts/PromptManager.php (1)
710-712:⚠️ Potential issue | 🔴 CriticalCall
normalizeValue()before attaching the resolved default.Line 711 writes
$defaultstraight into the prompt args. That bypasses the handler normalization contract, so multiselect defaults discovered or loaded as comma-separated strings still won't be converted to arrays before the prompt runs.💡 Minimal fix
- if (!is_null($default) && $default !== '') { - $args['default'] = $default; + if (!is_null($default)) { + $default = $handler->normalizeValue($default); + if ($default !== '') { + $args['default'] = $default; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.vortex/installer/src/Prompts/PromptManager.php around lines 710 - 712, In PromptManager.php, before assigning $default into $args['default'], pass it through the prompt handler normalization by calling normalizeValue() (e.g., $default = $this->normalizeValue($handler, $default)) so that handler-specific defaults (like comma-separated multiselect strings) are converted to the expected type; then attach the normalized $default into $args['default'] only when not null/empty.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.vortex/installer/src/Command/InstallCommand.php:
- Around line 18-20: Reorder the schema use statements so they are
alphabetically sorted: place DrevOps\VortexInstaller\Schema\SchemaGenerator
before DrevOps\VortexInstaller\Schema\SchemaValidator (keeping AgentHelp where
it belongs alphabetically), i.e., update the import list that contains
AgentHelp, SchemaValidator, and SchemaGenerator so Slevomat sees SchemaGenerator
before SchemaValidator.
- Around line 332-336: The code is rejecting '{}' because json_decode(..., TRUE)
loses the object/array distinction; change the logic to first
json_decode($prompts_json) without the TRUE to inspect the top-level type (use
is_object() vs is_array()), then convert to an associative array only if it was
an object (cast or json_decode with TRUE after verifying), and keep using
array_is_list() to detect true JSON arrays; update the validation branch around
$prompts_json / json_decode / $user_config / array_is_list to accept an empty
object (stdClass) as a valid prompt map and only reject true JSON lists or
scalars.
In @.vortex/installer/src/Schema/SchemaValidator.php:
- Around line 42-45: The validator currently normalizes config keys via
normalizeConfig($config) but only iterates $this->handlers in validate(), so
unknown prompt IDs are silently ignored; update validate() to compare the keys
from $normalized (use array_keys($normalized)) against the known handler IDs
(array_keys($this->handlers)) and when there are any extras (unknown IDs) add a
validation error and return invalid (or include the error in the returned errors
array); apply the same unknown-ID guard where the handler iteration is repeated
(the code around the block handling lines 126-139) so typos in --prompts are
reported instead of dropped.
In @.vortex/installer/src/Utils/OptionsResolver.php:
- Around line 130-131: The code currently treats any existing --prompts file as
readable and feeds file_get_contents() output directly into json_decode, causing
opaque JSON errors when the file is unreadable; update the logic around
$prompts_candidate/$prompts_json in OptionsResolver.php to explicitly check
is_file($prompts_candidate) && is_readable($prompts_candidate), call
file_get_contents() and verify it did not return false, and when it does return
false throw or return a clear error/exception indicating the prompts file is
unreadable (include the $prompts_candidate path), before attempting json_decode
and assigning to $prompts so failures are distinguished from actual JSON parse
errors.
In
@.vortex/installer/tests/Functional/Handlers/AiCodeInstructionsHandlerProcessTest.php:
- Line 23: Replace the lowercase boolean literal with the project-standard
uppercase constant: in the closure passed to static::cw (the lambda referencing
$test->prompts[AiCodeInstructions::id()]) change the assignment from false to
FALSE so the code uses the uppercase boolean constant per coding standards.
- Line 16: Replace the lowercase boolean literal in the static closure so it
uses the project-standard uppercase constant: change the assignment setting
prompts[AiCodeInstructions::id()] = true to use TRUE instead; locate the
static::cw(...) call (the closure that sets
$test->prompts[AiCodeInstructions::id()]) and update the boolean literal
accordingly.
In @.vortex/installer/tests/Functional/Handlers/DocsHandlerProcessTest.php:
- Line 15: The test sets a prompt value using a boolean literal in the closure
passed to static::cw — replace the lowercase literal "true" with the uppercase
PHP constant "TRUE" (i.e., change the assignment
$test->prompts[PreserveDocsProject::id()] = true to use TRUE) to satisfy the
UpperCaseConstant coding standard; the change should be made inside the
static::cw(...) closure where PreserveDocsProject::id() is used as the prompt
key.
- Line 18: The test uses a lowercase boolean literal in the static callback
assignment (the array value set for PreserveDocsProject::id()) which violates
PHP coding standards; update the value from false to the uppercase constant
FALSE in the closure where static::cw(fn($test) =>
$test->prompts[PreserveDocsProject::id()] = false) is set so the literal matches
the required PHP constant casing.
In
@.vortex/installer/tests/Functional/Handlers/HostingProviderHandlerProcessTest.php:
- Line 21: Replace the lowercase boolean literal used in the test setup with the
uppercase PHP constant to satisfy PHPCS: change assignments like
$test->prompts[AiCodeInstructions::id()] = true; to use TRUE (uppercase)
wherever they appear (e.g., the assignment to $test->prompts keyed by
AiCodeInstructions::id()), and update the other similar occurrence as well.
In @.vortex/installer/tests/Functional/Handlers/MigrationHandlerProcessTest.php:
- Line 18: The PHPCS error is caused by lowercase boolean literals; update all
occurrences where prompts are assigned with lowercase true/false (e.g., in the
static::cw closures that set $test->prompts[Migration::id()] = true) to use
uppercase TRUE or FALSE instead; search for the same pattern (other closures
assigning $test->prompts[...] = true/false around the file) and replace each
lowercase boolean with the uppercase constant to satisfy
Generic.PHP.UpperCaseConstant.Found.
In
@.vortex/installer/tests/Functional/Handlers/ProvisionTypeHandlerProcessTest.php:
- Around line 23-29: The test setup uses lowercase boolean literals; change them
to the project's uppercase constants by replacing occurrences like
$test->prompts[AiCodeInstructions::id()] = true and
$test->prompts[ProvisionType::id()] = ProvisionType::PROFILE (where applicable
boolean flags are set) with uppercase TRUE (e.g.
$test->prompts[AiCodeInstructions::id()] = TRUE). Update any other test prompts
in this file that assign boolean values using lowercase true/false to TRUE/FALSE
to comply with the coding standard; focus on the blocks referencing
AiCodeInstructions::id() and ProvisionType::id().
In
@.vortex/installer/tests/Functional/Handlers/PullRequestHandlerProcessTest.php:
- Line 17: Replace the lowercase boolean literal with the project-standard
uppercase constant: in the anonymous callback passed to static::cw (the line
assigning $test->prompts[AssignAuthorPr::id()]), change the value from true to
TRUE so the assignment uses the uppercase boolean constant per coding standards.
- Line 23: The test uses lowercase true; update the invocation that sets the
prompt for LabelMergeConflictsPr::id() to use the uppercase constant TRUE to
comply with project coding standards—locate the static::cw(...) call (and the
array access $test->prompts[LabelMergeConflictsPr::id()]) and replace the
lowercase true with TRUE.
- Line 20: Replace the lowercase boolean literal in the test setup so it follows
project coding standards: change the assignment in the closure passed to
static::cw (the line setting $test->prompts[AssignAuthorPr::id()]) to use the
uppercase constant FALSE instead of false.
- Line 26: The test sets a boolean using lowercase false which violates project
coding standards; update the assignment in the closure so that
LabelMergeConflictsPr::id() is assigned the uppercase constant FALSE (i.e.,
change $test->prompts[LabelMergeConflictsPr::id()] = false to = FALSE) to comply
with the coding convention in the closure passed to static::cw.
In @.vortex/installer/tests/Functional/Handlers/ServicesHandlerProcessTest.php:
- Around line 17-19: Replace lowercase boolean literals with PHP uppercase
constants in the test setup: in the closure passed to static::cw where
$test->prompts is populated (references Services::id() and
AiCodeInstructions::id()), change the three occurrences of the literal true to
the uppercase constant TRUE so PHPCS Generic.PHP.UpperCaseConstant.Found is
satisfied across that block.
---
Outside diff comments:
In @.vortex/installer/src/Prompts/PromptManager.php:
- Around line 692-708: The prompt-default precedence in PromptManager.php
currently checks $default_from_prompts, $default_from_discovery,
$default_override, and $default_from_handler but omits the existing config/env
prompt source; reintroduce the config/env check (the variable used for that
flow—e.g. $default_from_config or $default_from_env in this method) into the
precedence chain so the order becomes: $default_from_prompts, then the
config/env default, then $default_from_discovery, then $default_override, then
$default_from_handler; update the if/elseif block around $default_from_prompts /
$default_from_discovery to include this additional branch accordingly.
In @.vortex/installer/src/Schema/SchemaValidator.php:
- Around line 53-63: When checkDependency(...) returns 'skip' you currently copy
$value into $resolved without type validation; update the block handling
$dep_result === 'skip' in SchemaValidator (around the dependsOn/checkDependency
logic) to still call $this->validateType($id, $value, $handler, $normalized)
when $has_value before assigning to $resolved, and only set $resolved[$id] =
$value if validateType succeeds (or propagate/collect the validation error the
same way as for non-skipped prompts); keep the existing continue behavior for
truly skipped-without-value cases.
- Around line 79-92: When skipping prompts not provided in input, required
handlers that do not have a dependency are currently ignored; update the block
that follows "if (!$has_value) { continue; }" to instead check if the handler is
required and has no dependency and then push an error. Specifically, inside
SchemaValidator (use the existing $handler and isRequired()), replace the
unconditional skip with: if (!$has_value) { if ($handler->isRequired() && /* no
depends_on set for this handler */) { add the same error entry to $errors with
the handler id/message } continue; } so required prompts without depends_on are
treated as validation errors.
In
@.vortex/installer/tests/Functional/Handlers/MigrationDownloadSourceHandlerProcessTest.php:
- Around line 18-82: The test data providers use lowercase boolean literals
(e.g. in static::cw closures setting $test->prompts[Migration::id()] = true and
other prompt assignments) which violates the PHPCS rule that requires uppercase
boolean constants; update all occurrences of lowercase true/false in these
closures (including assignments for Migration::id(),
MigrationDownloadSource::id(), and MigrationImage::id() setups) to use TRUE and
FALSE so the tests conform to the coding standard.
In @.vortex/installer/tests/Functional/Handlers/ModulesHandlerProcessTest.php:
- Around line 14-101: Update the dataProviderHandlerProcess test set to retain
one end-to-end case that exercises the Modules handler via config/env rather
than via injected prompts: keep a yield entry that does not set
$test->prompts[Modules::id()] but instead relies on reading the MODULES
config/env value so DrevOps\VortexInstaller\Prompts\Handlers\Modules resolves
the short MODULES key in non-interactive mode and normalizes string defaults
like 'pathauto,redirect'; locate dataProviderHandlerProcess and the
Modules::id() usages and replace one of the prompt-driven yields with a case
that leaves $test->prompts unset and asserts the same module absences through
the normal config/env code path.
---
Duplicate comments:
In @.vortex/installer/src/Prompts/PromptManager.php:
- Around line 710-712: In PromptManager.php, before assigning $default into
$args['default'], pass it through the prompt handler normalization by calling
normalizeValue() (e.g., $default = $this->normalizeValue($handler, $default)) so
that handler-specific defaults (like comma-separated multiselect strings) are
converted to the expected type; then attach the normalized $default into
$args['default'] only when not null/empty.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 64ba13d4-9acf-491f-a61b-50aed23a79ed
⛔ Files ignored due to path filters (1)
.vortex/installer/tests/Fixtures/schema/valid_env_keys.jsonis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (37)
.vortex/installer/src/Command/InstallCommand.php.vortex/installer/src/Prompts/PromptManager.php.vortex/installer/src/Schema/AgentHelp.php.vortex/installer/src/Schema/SchemaGenerator.php.vortex/installer/src/Schema/SchemaValidator.php.vortex/installer/src/Utils/Config.php.vortex/installer/src/Utils/OptionsResolver.php.vortex/installer/tests/Functional/Command/SchemaValidateCommandTest.php.vortex/installer/tests/Functional/Handlers/AbstractHandlerProcessTestCase.php.vortex/installer/tests/Functional/Handlers/AiCodeInstructionsHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/BaselineHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/CiProviderHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/CodeProviderHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/CustomModulesHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/DatabaseDownloadSourceHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/DependencyUpdatesProviderHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/DeployTypeHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/DocsHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/HostingProjectNameHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/HostingProviderHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/MigrationDownloadSourceHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/MigrationHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/ModulesHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/NamesHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/NotificationChannelsHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/ProfileHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/ProvisionTypeHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/PullRequestHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/ServicesHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/StarterHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/ThemeHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/TimezoneHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/ToolsHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/VersionSchemeHandlerProcessTest.php.vortex/installer/tests/Unit/Schema/AgentHelpTest.php.vortex/installer/tests/Unit/Schema/SchemaGeneratorTest.php.vortex/installer/tests/Unit/Schema/SchemaValidatorTest.php
💤 Files with no reviewable changes (1)
- .vortex/installer/tests/Unit/Schema/AgentHelpTest.php
.vortex/installer/tests/Functional/Handlers/AiCodeInstructionsHandlerProcessTest.php
Outdated
Show resolved
Hide resolved
.vortex/installer/tests/Functional/Handlers/PullRequestHandlerProcessTest.php
Outdated
Show resolved
Hide resolved
.vortex/installer/tests/Functional/Handlers/PullRequestHandlerProcessTest.php
Outdated
Show resolved
Hide resolved
.vortex/installer/tests/Functional/Handlers/PullRequestHandlerProcessTest.php
Outdated
Show resolved
Hide resolved
.vortex/installer/tests/Functional/Handlers/PullRequestHandlerProcessTest.php
Outdated
Show resolved
Hide resolved
.vortex/installer/tests/Functional/Handlers/ServicesHandlerProcessTest.php
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
AlexSkrypnyk
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
.vortex/installer/tests/Functional/Handlers/MigrationHandlerProcessTest.php (1)
88-115:⚠️ Potential issue | 🔴 CriticalMissing required
hosting_project_nameprompt for Lagoon cases causes pipeline failures.Both
migration_enabled_lagoonandmigration_disabled_lagoontest cases fail because selectingHostingProvider::LAGOONrequires ahosting_project_namevalue.🐛 Proposed fix
yield 'migration_enabled_lagoon' => [ static::cw(function ($test): void { $test->prompts[Migration::id()] = TRUE; $test->prompts[HostingProvider::id()] = HostingProvider::LAGOON; + $test->prompts[HostingProjectName::id()] = 'test-project'; }), static::cw(function (AbstractHandlerProcessTestCase $test): void { ... }), ]; yield 'migration_disabled_lagoon' => [ static::cw(function ($test): void { $test->prompts[Migration::id()] = FALSE; $test->prompts[HostingProvider::id()] = HostingProvider::LAGOON; + $test->prompts[HostingProjectName::id()] = 'test-project'; }), ... ];Add the import at the top of the file:
use DrevOps\VortexInstaller\Prompts\Handlers\HostingProjectName;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.vortex/installer/tests/Functional/Handlers/MigrationHandlerProcessTest.php around lines 88 - 115, The two Lagoon test cases (migration_enabled_lagoon and migration_disabled_lagoon) are missing the required hosting_project_name prompt; add the HostingProjectName prompt to both case setup closures and import its class at the top of the test file (use DrevOps\VortexInstaller\Prompts\Handlers\HostingProjectName) so each static::cw that sets HostingProvider::LAGOON also sets $test->prompts[HostingProjectName::id()] to a suitable string value..vortex/installer/tests/Unit/Schema/SchemaGeneratorTest.php (1)
66-75: 🧹 Nitpick | 🔵 TrivialAssert the legacy
envkey is absent.This now proves
envis no longer required, but not that it is gone. A negative assertion here would catch any accidental reintroduction of the legacy key.💡 Suggested assertion
foreach ($schema['prompts'] as $index => $prompt) { foreach ($required_fields as $required_field) { $this->assertArrayHasKey($required_field, $prompt, sprintf('Prompt at index %d missing field "%s".', $index, $required_field)); } + $this->assertArrayNotHasKey('env', $prompt, sprintf('Prompt at index %d should not expose legacy field "env".', $index)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.vortex/installer/tests/Unit/Schema/SchemaGeneratorTest.php around lines 66 - 75, The test method testAllPromptsHaveRequiredFields currently checks for presence of required fields in each prompt; add a negative assertion inside the same foreach over $schema['prompts'] to ensure the legacy 'env' key is not present (use PHPUnit's assertArrayNotHasKey or equivalent) so each $prompt is asserted to not contain 'env' and will fail if the legacy key is reintroduced..vortex/installer/tests/Functional/Handlers/ToolsHandlerProcessTest.php (1)
16-528: 🧹 Nitpick | 🔵 TrivialRetain one functional case that passes
Toolsas a string.All scenarios now inject an array directly into
$test->prompts[Tools::id()], so this suite no longer exercises the multiselect normalization path added in this PR. Please keep one--no-interactioncase that suppliestoolsas a single or comma-separated string via config/env to cover the crash regression end-to-end.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.vortex/installer/tests/Functional/Handlers/ToolsHandlerProcessTest.php around lines 16 - 528, The test suite stopped exercising the multiselect normalization path because every case now injects an array into $test->prompts[Tools::id()]; add back a single functional case in dataProviderHandlerProcess that supplies Tools as a string (single or comma-separated) to exercise the --no-interaction path and prevent the regression. Concretely, add a new yield (e.g. 'tools_string_no_interaction') using static::cw that sets $test->prompts[Tools::id()] to a string like 'phpcs,phpstan' (not an array) and marks the run as non-interactive (or sets the config/env flag the test harness uses for --no-interaction), then add appropriate assertions similar to the existing cases to validate behavior; reference Tools::id(), dataProviderHandlerProcess and static::cw to locate where to add this case..vortex/installer/tests/Functional/Handlers/BaselineHandlerProcessTest.php (1)
79-105:⚠️ Potential issue | 🟡 MinorPipeline failures indicate snapshot mismatches with the new service selections.
The test cases override
Services::id()with different values than the baseline snapshot expects:
non_interactive_prompts_file: Sets[SOLR, CLAMAV]but snapshot expects Redisnon_interactive_prompts_string: Sets[SOLR, REDIS]but snapshot expects ClamAVEither update the snapshots to match these new service selections, or align the test services with the baseline snapshot expectations.
Additionally, consider using the constant
InstallCommand::OPTION_PROMPTSinstead of the string literal'prompts'for consistency with other tests (Lines 88, 101).💡 Suggested fix for constant usage
- $test->installOptions['prompts'] = $prompts_file; + $test->installOptions[InstallCommand::OPTION_PROMPTS] = $prompts_file;- $test->installOptions['prompts'] = $prompts_string; + $test->installOptions[InstallCommand::OPTION_PROMPTS] = $prompts_string;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.vortex/installer/tests/Functional/Handlers/BaselineHandlerProcessTest.php around lines 79 - 105, The two test cases in BaselineHandlerProcessTest override Services::id() values that don't match the baseline snapshots; update the service selections to match the snapshot (make non_interactive_prompts_file use [Services::SOLR, Services::REDIS] and make non_interactive_prompts_string use [Services::SOLR, Services::CLAMAV]) or alternatively update the stored snapshots to reflect the new selections—pick one approach and apply it consistently. Also replace the string literal 'prompts' used to set $test->installOptions with the constant InstallCommand::OPTION_PROMPTS in both static closures to keep option usage consistent with other tests.
♻️ Duplicate comments (2)
.vortex/installer/src/Command/InstallCommand.php (1)
332-339:⚠️ Potential issue | 🟠 Major
{}is still rejected due toarray_is_list([])returningtrue.The previous review flagged that
json_decode('{}', TRUE)returns[], andarray_is_list([])istruein PHP 8.1+, causing valid empty prompt objects to be rejected.💡 Proposed fix to preserve object/array distinction
$prompts_json = is_file($prompts_option) ? (string) file_get_contents($prompts_option) : $prompts_option; - $user_config = json_decode($prompts_json, TRUE); + $decoded = json_decode($prompts_json); - if (!is_array($user_config) || array_is_list($user_config)) { + if (!is_object($decoded)) { $output->writeln('Invalid JSON in --prompts. Expected a JSON object, not an array or scalar.'); return Command::FAILURE; } + + $user_config = (array) $decoded;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.vortex/installer/src/Command/InstallCommand.php around lines 332 - 339, The validation incorrectly rejects JSON objects that decode to an empty array because array_is_list([]) is true; to fix, after computing $prompts_json and $user_config keep a second decode like $user_config_raw = json_decode($prompts_json) and treat the value as a valid object when $user_config_raw is an instance of stdClass even if $user_config is an empty array; update the conditional that currently uses array_is_list($user_config) to also allow the case where $user_config_raw instanceof stdClass and only return Command::FAILURE when the decoded value is not an array/object or is a numeric-indexed list..vortex/installer/src/Schema/SchemaValidator.php (1)
42-45:⚠️ Potential issue | 🟠 MajorReject unknown prompt IDs instead of dropping them.
normalizeConfig()keeps unrecognized keys, butvalidate()only walks$this->handlers. A typo—or a fullVORTEX_INSTALLER_PROMPT_*key—still returnsvalid: trueand then disappears fromresolved, which makes--validateunreliable.🧭 Suggested guard
$normalized = $this->normalizeConfig($config); + $known_ids = array_diff(array_keys($this->handlers), SchemaGenerator::getExcludedHandlers()); + foreach (array_diff(array_keys($normalized), $known_ids) as $unknown_id) { + $errors[] = [ + 'prompt' => $unknown_id, + 'message' => sprintf('Unknown prompt ID "%s".', $unknown_id), + ]; + } foreach ($this->handlers as $id => $handler) {Also applies to: 126-139
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.vortex/installer/src/Schema/SchemaValidator.php around lines 42 - 45, normalizeConfig() currently preserves unknown config keys but validate() only iterates $this->handlers, so stray or misspelled prompt IDs in $normalized silently vanish from $resolved; update validate() to detect any keys in $normalized that are not recognized handler IDs (present in $this->handlers) nor allowable env-style names and treat them as validation failures. Concretely, after $normalized = $this->normalizeConfig($config) (and before walking $this->handlers), compute the set difference between array_keys($normalized) and the accepted keys (array_keys($this->handlers') and any canonical env names), and if any extras exist add errors and return valid: false (or append to the validator error collection) so unknown prompt IDs are rejected rather than dropped; apply the same guard where the other validation pass uses $normalized (the block mirroring lines 126-139).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
@.vortex/installer/tests/Functional/Handlers/DatabaseDownloadSourceHandlerProcessTest.php:
- Around line 16-38: Keep one end-to-end (no-interaction) case that exercises
the PromptManager/config/env resolution so the handler-id fallback can't
regress: in dataProviderHandlerProcess replace or add a yield where, instead of
writing directly to $test->prompts, you set the database download source through
the test harness' config/env setter (or the PromptManager helper used elsewhere
in tests) so the handler runs with --no-interaction and resolves the value via
config/env; target the dataProviderHandlerProcess provider and the
container_registry or url scenario so the handler-id fallback and PromptManager
lookup are exercised.
In
@.vortex/installer/tests/Functional/Handlers/ProvisionTypeHandlerProcessTest.php:
- Around line 19-24: The test case for the LAGOON path is missing the required
HostingProjectName prompt which causes the pipeline to fail; update the test in
ProvisionTypeHandlerProcessTest by adding the HostingProjectName prompt to the
prompts array (e.g. $test->prompts[HostingProjectName::id()] =
'<valid-project-name>') and add the corresponding import for HostingProjectName
so the symbol resolves (add use
DrevOps\VortexInstaller\Prompts\Handlers\HostingProjectName; at the top of the
test file).
---
Outside diff comments:
In @.vortex/installer/tests/Functional/Handlers/BaselineHandlerProcessTest.php:
- Around line 79-105: The two test cases in BaselineHandlerProcessTest override
Services::id() values that don't match the baseline snapshots; update the
service selections to match the snapshot (make non_interactive_prompts_file use
[Services::SOLR, Services::REDIS] and make non_interactive_prompts_string use
[Services::SOLR, Services::CLAMAV]) or alternatively update the stored snapshots
to reflect the new selections—pick one approach and apply it consistently. Also
replace the string literal 'prompts' used to set $test->installOptions with the
constant InstallCommand::OPTION_PROMPTS in both static closures to keep option
usage consistent with other tests.
In @.vortex/installer/tests/Functional/Handlers/MigrationHandlerProcessTest.php:
- Around line 88-115: The two Lagoon test cases (migration_enabled_lagoon and
migration_disabled_lagoon) are missing the required hosting_project_name prompt;
add the HostingProjectName prompt to both case setup closures and import its
class at the top of the test file (use
DrevOps\VortexInstaller\Prompts\Handlers\HostingProjectName) so each static::cw
that sets HostingProvider::LAGOON also sets
$test->prompts[HostingProjectName::id()] to a suitable string value.
In @.vortex/installer/tests/Functional/Handlers/ToolsHandlerProcessTest.php:
- Around line 16-528: The test suite stopped exercising the multiselect
normalization path because every case now injects an array into
$test->prompts[Tools::id()]; add back a single functional case in
dataProviderHandlerProcess that supplies Tools as a string (single or
comma-separated) to exercise the --no-interaction path and prevent the
regression. Concretely, add a new yield (e.g. 'tools_string_no_interaction')
using static::cw that sets $test->prompts[Tools::id()] to a string like
'phpcs,phpstan' (not an array) and marks the run as non-interactive (or sets the
config/env flag the test harness uses for --no-interaction), then add
appropriate assertions similar to the existing cases to validate behavior;
reference Tools::id(), dataProviderHandlerProcess and static::cw to locate where
to add this case.
In @.vortex/installer/tests/Unit/Schema/SchemaGeneratorTest.php:
- Around line 66-75: The test method testAllPromptsHaveRequiredFields currently
checks for presence of required fields in each prompt; add a negative assertion
inside the same foreach over $schema['prompts'] to ensure the legacy 'env' key
is not present (use PHPUnit's assertArrayNotHasKey or equivalent) so each
$prompt is asserted to not contain 'env' and will fail if the legacy key is
reintroduced.
---
Duplicate comments:
In @.vortex/installer/src/Command/InstallCommand.php:
- Around line 332-339: The validation incorrectly rejects JSON objects that
decode to an empty array because array_is_list([]) is true; to fix, after
computing $prompts_json and $user_config keep a second decode like
$user_config_raw = json_decode($prompts_json) and treat the value as a valid
object when $user_config_raw is an instance of stdClass even if $user_config is
an empty array; update the conditional that currently uses
array_is_list($user_config) to also allow the case where $user_config_raw
instanceof stdClass and only return Command::FAILURE when the decoded value is
not an array/object or is a numeric-indexed list.
In @.vortex/installer/src/Schema/SchemaValidator.php:
- Around line 42-45: normalizeConfig() currently preserves unknown config keys
but validate() only iterates $this->handlers, so stray or misspelled prompt IDs
in $normalized silently vanish from $resolved; update validate() to detect any
keys in $normalized that are not recognized handler IDs (present in
$this->handlers) nor allowable env-style names and treat them as validation
failures. Concretely, after $normalized = $this->normalizeConfig($config) (and
before walking $this->handlers), compute the set difference between
array_keys($normalized) and the accepted keys (array_keys($this->handlers') and
any canonical env names), and if any extras exist add errors and return valid:
false (or append to the validator error collection) so unknown prompt IDs are
rejected rather than dropped; apply the same guard where the other validation
pass uses $normalized (the block mirroring lines 126-139).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7e0178c9-a7cb-49ee-bcc3-2066434b5ee5
⛔ Files ignored due to path filters (1)
.vortex/installer/tests/Fixtures/schema/valid_env_keys.jsonis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (37)
.vortex/installer/src/Command/InstallCommand.php.vortex/installer/src/Prompts/PromptManager.php.vortex/installer/src/Schema/AgentHelp.php.vortex/installer/src/Schema/SchemaGenerator.php.vortex/installer/src/Schema/SchemaValidator.php.vortex/installer/src/Utils/Config.php.vortex/installer/src/Utils/OptionsResolver.php.vortex/installer/tests/Functional/Command/SchemaValidateCommandTest.php.vortex/installer/tests/Functional/Handlers/AbstractHandlerProcessTestCase.php.vortex/installer/tests/Functional/Handlers/AiCodeInstructionsHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/BaselineHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/CiProviderHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/CodeProviderHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/CustomModulesHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/DatabaseDownloadSourceHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/DependencyUpdatesProviderHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/DeployTypeHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/DocsHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/HostingProjectNameHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/HostingProviderHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/MigrationDownloadSourceHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/MigrationHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/ModulesHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/NamesHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/NotificationChannelsHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/ProfileHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/ProvisionTypeHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/PullRequestHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/ServicesHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/StarterHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/ThemeHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/TimezoneHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/ToolsHandlerProcessTest.php.vortex/installer/tests/Functional/Handlers/VersionSchemeHandlerProcessTest.php.vortex/installer/tests/Unit/Schema/AgentHelpTest.php.vortex/installer/tests/Unit/Schema/SchemaGeneratorTest.php.vortex/installer/tests/Unit/Schema/SchemaValidatorTest.php
💤 Files with no reviewable changes (1)
- .vortex/installer/tests/Unit/Schema/AgentHelpTest.php
Comment on lines
16
to
38
| public static function dataProviderHandlerProcess(): \Iterator { | ||
| yield 'db_download_source_url' => [ | ||
| static::cw(fn() => Env::put(DatabaseDownloadSource::envName(), DatabaseDownloadSource::URL)), | ||
| static::cw(fn($test): string => $test->prompts[DatabaseDownloadSource::id()] = DatabaseDownloadSource::URL), | ||
| ]; | ||
| yield 'db_download_source_ftp' => [ | ||
| static::cw(fn() => Env::put(DatabaseDownloadSource::envName(), DatabaseDownloadSource::FTP)), | ||
| static::cw(fn($test): string => $test->prompts[DatabaseDownloadSource::id()] = DatabaseDownloadSource::FTP), | ||
| ]; | ||
| yield 'db_download_source_acquia' => [ | ||
| static::cw(fn() => Env::put(DatabaseDownloadSource::envName(), DatabaseDownloadSource::ACQUIA)), | ||
| static::cw(fn($test): string => $test->prompts[DatabaseDownloadSource::id()] = DatabaseDownloadSource::ACQUIA), | ||
| ]; | ||
| yield 'db_download_source_lagoon' => [ | ||
| static::cw(fn() => Env::put(DatabaseDownloadSource::envName(), DatabaseDownloadSource::LAGOON)), | ||
| static::cw(fn($test): string => $test->prompts[DatabaseDownloadSource::id()] = DatabaseDownloadSource::LAGOON), | ||
| ]; | ||
| yield 'db_download_source_container_registry' => [ | ||
| static::cw(function (): void { | ||
| Env::put(DatabaseDownloadSource::envName(), DatabaseDownloadSource::CONTAINER_REGISTRY); | ||
| Env::put(DatabaseImage::envName(), 'the_empire/star_wars:latest'); | ||
| Env::put(AiCodeInstructions::envName(), Env::TRUE); | ||
| static::cw(function ($test): void { | ||
| $test->prompts[DatabaseDownloadSource::id()] = DatabaseDownloadSource::CONTAINER_REGISTRY; | ||
| $test->prompts[DatabaseImage::id()] = 'the_empire/star_wars:latest'; | ||
| $test->prompts[AiCodeInstructions::id()] = TRUE; | ||
| }), | ||
| ]; | ||
| yield 'db_download_source_s3' => [ | ||
| static::cw(fn() => Env::put(DatabaseDownloadSource::envName(), DatabaseDownloadSource::S3)), | ||
| static::cw(fn($test): string => $test->prompts[DatabaseDownloadSource::id()] = DatabaseDownloadSource::S3), | ||
| ]; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Keep one end-to-end case on the config/env resolution path.
These setups now write straight to $test->prompts, so they no longer exercise the PromptManager config/env lookup path this PR fixes. Keeping one --no-interaction scenario driven by config or env would protect the handler-id fallback from regressing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
@.vortex/installer/tests/Functional/Handlers/DatabaseDownloadSourceHandlerProcessTest.php
around lines 16 - 38, Keep one end-to-end (no-interaction) case that exercises
the PromptManager/config/env resolution so the handler-id fallback can't
regress: in dataProviderHandlerProcess replace or add a yield where, instead of
writing directly to $test->prompts, you set the database download source through
the test harness' config/env setter (or the PromptManager helper used elsewhere
in tests) so the handler runs with --no-interaction and resolves the value via
config/env; target the dataProviderHandlerProcess provider and the
container_registry or url scenario so the handler-id fallback and PromptManager
lookup are exercised.
.vortex/installer/tests/Functional/Handlers/ProvisionTypeHandlerProcessTest.php
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
AlexSkrypnyk
force-pushed
the
feature/config-no-interaction
branch
from
7749061 to
f1a5092
Compare
This comment has been minimized.
This comment has been minimized.
1 similar comment
|
Code coverage (threshold: 90%) Per-class coverage |
This comment has been minimized.
This comment has been minimized.
2 similar comments
Member
Author
|
Code coverage (threshold: 90%) Per-class coverage |
Member
Author
|
Code coverage (threshold: 90%) Per-class coverage |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Separated prompt answers from installer configuration by adding a new
--promptsCLI option. Previously,--configconflated installer settings (repo, ref, destination) with prompt answers (modules, hosting provider, etc.), causing key mismatches that made programmatic installation via--confignon-functional.The new
--promptsoption accepts a JSON object keyed by prompt handler ID (theidfield from--schemaoutput). The installer validates provided values against the prompt schema, while unprovided prompts fall back to discovery or handler defaults. The--configoption remains for installer-level configuration only.Changes
New
--promptsCLI optionInstallCommand: added--prompts(-p) option accepting a JSON string or file pathOptionsResolver: parses--promptsJSON, validates it is a JSON object, stores the parsed array in Config asVORTEX_INSTALLER_PROMPTSConfig: addedPROMPTSconstant for the new config keyPromptManager reads and applies prompt overrides
PromptManager::resolvePromptOverrides(): reads prompt values from Config, validates them against handlers viaSchemaValidator, stores validated overridesPromptManager::args(): prompt overrides are the highest priority source in the default value resolution chain (overrides → discover → handler default)VORTEX_INSTALLER_PROMPT_*) for prompt values fromargs()— prompt values now flow exclusively through--promptsSchema changes
SchemaGenerator: removedenvfield from schema output — the schema describes pure JSON format only, env vars are an internal concernSchemaValidator(renamed fromConfigValidator): validates only provided prompt values (partial validation), enforces dependency constraints, does not error on missing non-dependent promptsSchemaValidatorandSchemaGeneratornow receive handlers via constructor instead of method parameters--validateuses--promptshandleValidate()reads from--promptsinstead of--configAgentHelpupdated to describe the new workflow with--promptsTest refactoring
Env::put(Handler::envName(), value)with$test->prompts[Handler::id()] = valueAbstractHandlerProcessTestCase: added$promptsproperty, encodes to JSON and passes as--promptsinstall optiontheme: 'custom'+theme_custom: 'light_saber')envfield assertions, removedvalid_env_keys.jsonfixtureBefore / After