[#1950] Added --destination to build and check-requirements Installer's commands.
#2168
Conversation
WalkthroughAdds a reusable DestinationAwareTrait and updates installer commands and tests to accept a --destination option, resolve it into a protected $cwd, replace implicit getcwd() usage with the resolved cwd, and wire the process runner to use that working directory; tests and TUI messages updated for destination validation. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
.vortex/installer/src/Command/BuildCommand.php(8 hunks).vortex/installer/src/Command/CheckRequirementsCommand.php(3 hunks).vortex/installer/src/Command/DestinationAwareTrait.php(1 hunks).vortex/installer/src/Command/InstallCommand.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
.vortex/installer/src/Command/BuildCommand.php (6)
.vortex/installer/src/Command/CheckRequirementsCommand.php (1)
getProcessRunner(343-345).vortex/installer/src/Command/DestinationAwareTrait.php (2)
addDestinationOption(18-25)getDestination(39-57).vortex/docs/.utils/svg-term-render.js (1)
input(55-55).vortex/installer/src/Runner/ProcessRunnerAwareTrait.php (1)
getProcessRunner(25-27).vortex/installer/src/Runner/AbstractRunner.php (1)
setCwd(97-102).vortex/installer/src/Runner/RunnerInterface.php (1)
setCwd(78-78)
⏰ 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). (11)
- GitHub Check: build (1)
- GitHub Check: build (0)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-workflow (4)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-installer (8.2)
🔇 Additional comments (3)
.vortex/installer/src/Command/InstallCommand.php (2)
597-598: Footer build-success messages look good.The new inline “Get site info” and “Login” lines are clear, compact, and consistent with the rest of the installer messaging; no issues here.
456-462: The--destinationand--profileflags are correctly wired tobuild. BuildCommand defines the destination option via DestinationAwareTrait (line 20 of DestinationAwareTrait.php), and CommandRunner::run() merges both$argsand$inputsarrays (line 38 of CommandRunner.php) before passing them to ArrayInput, which properly recognizes associative keys as option names. No changes needed..vortex/installer/src/Command/CheckRequirementsCommand.php (1)
27-28: Destination-aware working directory for requirement checks is wired correctly.Using
DestinationAwareTrait, storing the resolved directory in$this->cwd, and initialising$this->processRunnerwithsetCwd($this->cwd)ensures all process-based checks share a consistent working directory without exposing uninitialised state. Since the current checks don’t depend on CWD semantics, this is backward compatible while keeping the command ready for destination-specific logic later.Also applies to: 69-72, 81-82, 92-93, 97-99
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace DrevOps\VortexInstaller\Command; | ||
|
|
||
| use Symfony\Component\Console\Input\InputInterface; | ||
| use Symfony\Component\Console\Input\InputOption; | ||
|
|
||
| /** | ||
| * Provides destination directory option for commands. | ||
| */ | ||
| trait DestinationAwareTrait { | ||
|
|
||
| /** | ||
| * Add the destination option to the command. | ||
| */ | ||
| protected function addDestinationOption(): void { | ||
| $this->addOption( | ||
| 'destination', | ||
| 'd', | ||
| InputOption::VALUE_REQUIRED, | ||
| 'Target directory for the operation. Defaults to current directory.' | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Get the destination directory from input. | ||
| * | ||
| * @param \Symfony\Component\Console\Input\InputInterface $input | ||
| * The input interface. | ||
| * | ||
| * @return string | ||
| * The validated destination directory path. | ||
| * | ||
| * @throws \InvalidArgumentException | ||
| * If the destination directory does not exist. | ||
| */ | ||
| protected function getDestination(InputInterface $input): string { | ||
| $destination = $input->getOption('destination'); | ||
|
|
||
| if ($destination === NULL || $destination === '') { | ||
| return getcwd() ?: '.'; | ||
| } | ||
|
|
||
| if (!is_string($destination)) { | ||
| throw new \InvalidArgumentException('Destination must be a string.'); | ||
| } | ||
|
|
||
| if (!is_dir($destination)) { | ||
| throw new \InvalidArgumentException( | ||
| sprintf('Destination directory does not exist: %s', $destination) | ||
| ); | ||
| } | ||
|
|
||
| return realpath($destination) ?: $destination; | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Trait cleanly encapsulates destination option and validation; minor doc tweak possible.
The trait nicely centralises --destination handling: option registration is reusable, and getDestination() provides a clear, validated, realpath-normalised directory with a sensible fallback to the current working directory. One minor polish point: the docblock currently mentions throwing when the directory does not exist, but the method can also throw if the option value is not a string. If you care about strict docs, you could broaden the @throws description to cover both cases.
🤖 Prompt for AI Agents
.vortex/installer/src/Command/DestinationAwareTrait.php lines 1-59: update the
method docblock for getDestination() to reflect both error conditions that can
throw an InvalidArgumentException (non-string option value and non-existent
destination directory) by broadening the @throws description to mention both
cases and, optionally, add a short inline note about the non-string check; keep
implementation unchanged.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #2168 +/- ##
===========================================
- Coverage 76.64% 76.07% -0.58%
===========================================
Files 111 105 -6
Lines 5836 5696 -140
Branches 44 0 -44
===========================================
- Hits 4473 4333 -140
Misses 1363 1363 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
AlexSkrypnyk
force-pushed
the
feature/add-destination-installer-build-check-reqs
branch
2 times, most recently
from
2b23a82 to
194b056
Compare
AlexSkrypnyk
force-pushed
the
feature/add-destination-installer-build-check-reqs
branch
from
194b056 to
c09df9f
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
.vortex/installer/src/Command/DestinationAwareTrait.php (1)
27-57: Destination handling is solid; consider broadening the@throwsdescription.The trait cleanly centralises
--destinationregistration and validation, and the runtime behaviour (defaulting to CWD, type-checking, directory existence,realpath()normalisation) looks correct.The docblock, however, only mentions throwing when the directory does not exist, while
getDestination()can also throw for a non-string value. You could make the@throwsdescription slightly more accurate, for example:- * @throws \InvalidArgumentException - * If the destination directory does not exist. + * @throws \InvalidArgumentException + * If the destination is not a string or the directory does not exist.Implementation can stay as-is.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
.vortex/installer/src/Command/BuildCommand.php(8 hunks).vortex/installer/src/Command/CheckRequirementsCommand.php(3 hunks).vortex/installer/src/Command/DestinationAwareTrait.php(1 hunks).vortex/installer/src/Command/InstallCommand.php(2 hunks).vortex/installer/tests/Functional/Command/BuildCommandTest.php(4 hunks).vortex/installer/tests/Functional/Command/CheckRequirementsCommandTest.php(4 hunks).vortex/installer/tests/Helpers/TuiOutput.php(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-13T04:14:41.765Z
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 2011
File: .vortex/tests/phpunit/Traits/Steps/StepAhoyTrait.php:20-23
Timestamp: 2025-09-13T04:14:41.765Z
Learning: In .vortex/tests/phpunit PHPUnit tests using cmd/cmdFail helpers, patterns in the expected output arrays support '*' markers for positive assertions (text must be present) and '!' markers for negative assertions (text must NOT be present). The '*' markers are NOT literal text matches but rather indicate expected presence.
Applied to files:
.vortex/installer/tests/Helpers/TuiOutput.php.vortex/installer/tests/Functional/Command/CheckRequirementsCommandTest.php.vortex/installer/tests/Functional/Command/BuildCommandTest.php
📚 Learning: 2025-06-01T08:10:15.903Z
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 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/tests/Functional/Command/CheckRequirementsCommandTest.php.vortex/installer/tests/Functional/Command/BuildCommandTest.php
🧬 Code graph analysis (5)
.vortex/installer/src/Command/CheckRequirementsCommand.php (5)
.vortex/installer/src/Command/DestinationAwareTrait.php (2)
addDestinationOption(18-25)getDestination(39-57).vortex/installer/src/Runner/ProcessRunnerAwareInterface.php (1)
getProcessRunner(18-18).vortex/installer/src/Runner/ProcessRunnerAwareTrait.php (1)
getProcessRunner(25-27).vortex/installer/src/Runner/RunnerInterface.php (1)
setCwd(78-78).vortex/installer/src/Runner/AbstractRunner.php (1)
setCwd(97-102)
.vortex/installer/src/Command/InstallCommand.php (6)
.vortex/installer/src/Utils/Config.php (1)
getDst(101-103).vortex/installer/src/Runner/CommandRunnerAwareTrait.php (1)
getCommandRunner(26-29).vortex/installer/src/Runner/ProcessRunner.php (1)
run(21-75).vortex/installer/src/Runner/RunnerInterface.php (1)
run(41-41).vortex/installer/src/Runner/CommandRunner.php (1)
run(34-72).vortex/installer/src/Utils/Git.php (1)
run(29-34)
.vortex/installer/tests/Functional/Command/CheckRequirementsCommandTest.php (1)
.vortex/installer/tests/Helpers/TuiOutput.php (2)
TuiOutput(7-314)present(173-175)
.vortex/installer/src/Command/BuildCommand.php (4)
.vortex/installer/src/Command/DestinationAwareTrait.php (2)
addDestinationOption(18-25)getDestination(39-57).vortex/installer/src/Runner/ProcessRunnerAwareTrait.php (1)
getProcessRunner(25-27).vortex/installer/src/Runner/RunnerInterface.php (1)
setCwd(78-78).vortex/installer/src/Runner/AbstractRunner.php (1)
setCwd(97-102)
.vortex/installer/tests/Functional/Command/BuildCommandTest.php (1)
.vortex/installer/tests/Helpers/TuiOutput.php (2)
TuiOutput(7-314)present(173-175)
⏰ 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). (11)
- GitHub Check: build (1)
- GitHub Check: build (0)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (4)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-installer (8.4)
🔇 Additional comments (4)
.vortex/installer/src/Command/InstallCommand.php (2)
496-510: runBuildCommand now correctly forwards destination and profile tobuild.Passing
--destinationfrom$this->config->getDst()and conditionally adding--profileensures thebuildcommand runs in the same target directory and mode as the installer just configured. Using a single$argsarray and wiring it intoCommandRunner::run()is consistent and keeps this step easy to extend.
635-640: Footer alignment tweak is consistent with test helpers.Inlining
"Get site info: ahoy info"and"Login: ahoy login"keeps the messages simple while still matching theTuiOutputpatterns (FOOTER_GET_SITE_INFO,FOOTER_AHOY_INFO,FOOTER_AHOY_LOGIN) used in tests..vortex/installer/tests/Helpers/TuiOutput.php (1)
160-163: Destination error constants align with command validation.
DESTINATION_NOT_EXISTandDESTINATION_MUST_BE_STRINGmirror theDestinationAwareTraitexception messages and give tests a stable way to assert on destination-related failures without hardcoding full error lines..vortex/installer/src/Command/BuildCommand.php (1)
25-28: BuildCommand’s destination-aware cwd wiring is consistent and robust.Using
DestinationAwareTraitplus a typed$cwdproperty, then:
- Initialising
$this->cwdviagetDestination($input)inexecute(),- Setting the process runner cwd with
$this->getProcessRunner()->setCwd($this->cwd)beforeahoy build,- Deriving
.envand docker-compose config from$this->cwd,makes
buildproperly target the chosen destination both when invoked directly and viaInstallCommand. The fallback ingetProjectMachineName()tobasename($this->cwd)when.envis missing also keeps behaviour sensible for non-.env setups.Also applies to: 47-51, 55-60, 70-72, 91-107, 125-136, 147-149
| /** | ||
| * The working directory for checks. | ||
| */ | ||
| protected string $cwd; | ||
|
|
There was a problem hiding this comment.
Always apply setCwd() to the process runner, even when injected.
Destination handling and $cwd initialisation look good, but this line:
$this->processRunner ??= $this->getProcessRunner()->setCwd($this->cwd);means that when a runner is injected via setProcessRunner(), setCwd() is never called, so check-requirements will ignore --destination in that scenario.
To keep behaviour consistent with BuildCommand and ensure destination-aware checks regardless of how the runner is provided, consider:
- $this->processRunner ??= $this->getProcessRunner()->setCwd($this->cwd);
+ $this->processRunner = $this->getProcessRunner()->setCwd($this->cwd);This still reuses an existing runner when present, but always updates its working directory.
Also applies to: 81-82, 92-98
🤖 Prompt for AI Agents
.vortex/installer/src/Command/CheckRequirementsCommand.php around lines 69-73
(also apply same pattern at 81-82 and 92-98): the current conditional assignment
skips calling setCwd() when a ProcessRunner is injected, so --destination is
ignored; always obtain the runner (either the injected one or a newly created
one), call setCwd($this->cwd) on it, then assign it back to
$this->processRunner; apply this change in all mentioned locations so the
runner's working directory is updated regardless of whether it was injected or
created.
.vortex/installer/tests/Functional/Command/CheckRequirementsCommandTest.php
Show resolved
Hide resolved
…taller's commands.
AlexSkrypnyk
force-pushed
the
feature/add-destination-installer-build-check-reqs
branch
from
c09df9f to
73176e7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.vortex/installer/src/Command/CheckRequirementsCommand.php (1)
97-99:setCwd()is skipped when a runner is injected, causing--destinationto be ignored.The null coalescing assignment (
??=) means that when aProcessRunneris injected viasetProcessRunner()(e.g., in tests),setCwd($this->cwd)is never called. This causes the--destinationoption to be silently ignored in those scenarios.Apply this fix to ensure
setCwd()is always called:- $this->processRunner ??= $this->getProcessRunner()->setCwd($this->cwd); + $this->processRunner = $this->getProcessRunner()->setCwd($this->cwd);This still reuses an existing runner when present (via
getProcessRunner()'s internal??=), but always updates its working directory..vortex/installer/src/Command/DestinationAwareTrait.php (1)
36-37: Docblock incomplete: mentions only one of two exception conditions.The
@throwsannotation only documents the "directory does not exist" case, butgetDestination()can also throw when the destination is not a string (line 47). Consider updating the docblock for completeness:* @throws \InvalidArgumentException - * If the destination directory does not exist. + * If the destination is not a string or the directory does not exist.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
.vortex/installer/src/Command/BuildCommand.php(8 hunks).vortex/installer/src/Command/CheckRequirementsCommand.php(3 hunks).vortex/installer/src/Command/DestinationAwareTrait.php(1 hunks).vortex/installer/src/Command/InstallCommand.php(2 hunks).vortex/installer/tests/Functional/Command/BuildCommandTest.php(4 hunks).vortex/installer/tests/Functional/Command/CheckRequirementsCommandTest.php(4 hunks).vortex/installer/tests/Helpers/TuiOutput.php(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-13T04:14:41.765Z
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 2011
File: .vortex/tests/phpunit/Traits/Steps/StepAhoyTrait.php:20-23
Timestamp: 2025-09-13T04:14:41.765Z
Learning: In .vortex/tests/phpunit PHPUnit tests using cmd/cmdFail helpers, patterns in the expected output arrays support '*' markers for positive assertions (text must be present) and '!' markers for negative assertions (text must NOT be present). The '*' markers are NOT literal text matches but rather indicate expected presence.
Applied to files:
.vortex/installer/tests/Helpers/TuiOutput.php.vortex/installer/tests/Functional/Command/BuildCommandTest.php.vortex/installer/tests/Functional/Command/CheckRequirementsCommandTest.php
📚 Learning: 2025-08-08T12:02:24.652Z
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 1896
File: .vortex/tests/bats/unit/download-db-lagoon.bats:24-25
Timestamp: 2025-08-08T12:02:24.652Z
Learning: In .vortex/tests/bats/unit Bats tests using ../_helper.bash (run_steps), prefixing a STEPS entry with "- " denotes a negative assertion (the substring must NOT appear in output). Unprefixed entries are positive assertions. Example: "- Database dump refresh requested. Will create a new dump." asserts absence; "Database dump refresh requested. Will create a new dump." asserts presence.
Applied to files:
.vortex/installer/tests/Functional/Command/BuildCommandTest.php.vortex/installer/tests/Functional/Command/CheckRequirementsCommandTest.php
📚 Learning: 2025-06-01T08:10:15.903Z
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 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/tests/Functional/Command/BuildCommandTest.php.vortex/installer/tests/Functional/Command/CheckRequirementsCommandTest.php
🧬 Code graph analysis (4)
.vortex/installer/src/Command/CheckRequirementsCommand.php (5)
.vortex/installer/src/Command/DestinationAwareTrait.php (2)
addDestinationOption(18-25)getDestination(39-57).vortex/installer/src/Runner/ProcessRunnerAwareTrait.php (1)
getProcessRunner(25-27).vortex/installer/src/Runner/ProcessRunnerAwareInterface.php (1)
getProcessRunner(18-18).vortex/installer/src/Runner/RunnerInterface.php (1)
setCwd(78-78).vortex/installer/src/Runner/AbstractRunner.php (1)
setCwd(97-102)
.vortex/installer/src/Command/InstallCommand.php (5)
.vortex/installer/src/Utils/Config.php (1)
getDst(101-103).vortex/installer/src/Runner/CommandRunnerAwareTrait.php (1)
getCommandRunner(26-29).vortex/installer/src/Runner/ProcessRunner.php (1)
run(21-75).vortex/installer/src/Runner/RunnerInterface.php (1)
run(41-41).vortex/installer/src/Runner/CommandRunner.php (1)
run(34-72)
.vortex/installer/tests/Functional/Command/BuildCommandTest.php (2)
.vortex/installer/src/Utils/File.php (1)
File(10-121).vortex/installer/tests/Helpers/TuiOutput.php (2)
TuiOutput(7-314)present(173-175)
.vortex/installer/tests/Functional/Command/CheckRequirementsCommandTest.php (1)
.vortex/installer/tests/Helpers/TuiOutput.php (2)
TuiOutput(7-314)present(173-175)
⏰ 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). (11)
- GitHub Check: build (0)
- GitHub Check: build (1)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (4)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-installer (8.2)
🔇 Additional comments (10)
.vortex/installer/src/Command/InstallCommand.php (2)
501-507: LGTM! Clean args construction for build command invocation.The destination and profile arguments are properly constructed and passed to the build command. The logic correctly determines when to include the
--profileflag based on the selected starter type.
638-639: Minor formatting improvement for output alignment.The fixed-width inline strings improve visual alignment of the "Get site info" and "Login" instructions in the success footer.
.vortex/installer/tests/Helpers/TuiOutput.php (1)
160-163: LGTM! Test helper constants align with trait error messages.The new constants correctly match the error messages thrown by
DestinationAwareTrait::getDestination(), enabling consistent test assertions for destination validation scenarios..vortex/installer/src/Command/CheckRequirementsCommand.php (1)
69-72: Destination handling integration is well-structured.The trait integration follows a clear pattern: option registration in
configure(), resolution inexecute(), and storage in the$cwdproperty. The docblock is clear and the placement is appropriate.Also applies to: 81-81, 92-93
.vortex/installer/src/Command/DestinationAwareTrait.php (1)
1-59: Well-designed trait for reusable destination handling.The trait cleanly encapsulates the
--destinationoption pattern with proper validation and sensible defaults. The fallback chain (getcwd()→'.') andrealpath()normalization are appropriate..vortex/installer/tests/Functional/Command/BuildCommandTest.php (3)
38-42: Clean before-hook pattern for per-test filesystem setup.The optional
$beforeclosure provides a flexible way to prepare test-specific filesystem state without complicating the main test harness. The pattern of receiving$command_inputsand returning modified inputs is intuitive.
453-497: Comprehensive destination validation test coverage.The three new test cases effectively exercise:
- Valid directory path → success
- File path (not a directory) → fails with
DESTINATION_NOT_EXIST- Non-existent path → fails with
DESTINATION_NOT_EXISTUsing
uniqid()for path uniqueness prevents test collisions. The assertions correctly align with the error messages defined inDestinationAwareTrait::getDestination().
7-8: Import addition is appropriate.The
AlexSkrypnyk\File\Fileimport supports the filesystem operations (mkdir,dump) used in the newbeforeclosures..vortex/installer/src/Command/BuildCommand.php (1)
27-27: LGTM! Destination integration is clean and consistent.The
DestinationAwareTraitintegration is well-executed:$this->cwdis initialized early inexecute()and consistently used throughout for.envresolution, project name derivation, URL inspection, and process runner configuration. Removing the closure capture (line 93) in favor of the property is cleaner and aligns with the trait pattern.Also applies to: 47-50, 59-59, 71-71, 93-107, 126-127, 135-136, 148-149
.vortex/installer/tests/Functional/Command/CheckRequirementsCommandTest.php (1)
7-7: LGTM! Test coverage for destination validation is thorough.The
beforeclosure hook cleanly extends the test harness to support filesystem setup, and the three new test cases properly exercise the destination validation logic: valid directory (success), file instead of directory (error), and non-existent path (error). The use ofuniqid()for isolation andself::$tmpfor cleanup is appropriate.Also applies to: 28-38, 87-89, 392-430
| if (!is_string($destination)) { | ||
| throw new \InvalidArgumentException('Destination must be a string.'); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Type check is likely unreachable but serves as defensive coding.
Since InputOption::VALUE_REQUIRED ensures the option is either null, empty string, or a string, the !is_string($destination) check at line 46 should never be true after the null/empty check at line 42. This is acceptable as defensive coding against future edge cases, but it may warrant a @phpstan-ignore-next-line annotation if static analysis flags it as dead code.
🤖 Prompt for AI Agents
.vortex/installer/src/Command/DestinationAwareTrait.php lines 46-48: the
defensive is_string($destination) check is flagged by static analysis as
unreachable; add a @phpstan-ignore-next-line (or equivalent PHPStan suppression)
immediately above that line to silence the dead-code warning while keeping the
runtime defensive check intact.
Related to #1950
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.