Moved installer handler unit and functional tests into own classes. #1937
Conversation
WalkthroughReplaces monolithic installer/unit tests with modular test architecture: adds abstract functional and unit test bases, many per-handler unit and functional tests with data providers and fixtures, removes two legacy monolithic tests, and updates documentation (duplicate "Installer Test Architecture" block added). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor PHPUnit
participant AbstractInstallTestCase as AbstractInstallTestCase::testInstall
participant Fixtures as Fixtures/Baseline
participant Installer as CLI Installer
participant FS as Filesystem
PHPUnit->>AbstractInstallTestCase: run testInstall(before?, after?, expected[])
AbstractInstallTestCase->>Fixtures: load scenario fixtures
alt before hook provided
AbstractInstallTestCase->>AbstractInstallTestCase: execute before()
end
AbstractInstallTestCase->>Installer: run non-interactive installer
Installer-->>AbstractInstallTestCase: output
AbstractInstallTestCase->>AbstractInstallTestCase: assert output contains expected
AbstractInstallTestCase->>Fixtures: load baseline
AbstractInstallTestCase->>FS: replace versions in SUT
AbstractInstallTestCase->>FS: compare SUT vs patched baseline
AbstractInstallTestCase->>AbstractInstallTestCase: assertCommon()
alt after hook provided
AbstractInstallTestCase->>AbstractInstallTestCase: execute after()
end
sequenceDiagram
autonumber
actor PHPUnit
participant AbstractPMTC as AbstractPromptManagerTestCase::testRunPrompts
participant Config as Config
participant Prompt as Prompt (TUI)
participant PM as PromptManager
PHPUnit->>AbstractPMTC: run testRunPrompts(answers, expected, before?, after?)
AbstractPMTC->>Config: build test Config/project
alt before hook provided
AbstractPMTC->>AbstractPMTC: execute before(Config)
end
AbstractPMTC->>Prompt: fake keystrokes
AbstractPMTC->>PM: runPrompts(Config, answers)
PM-->>AbstractPMTC: responses or exception
AbstractPMTC->>AbstractPMTC: assert responses or error message
alt after hook provided
AbstractPMTC->>AbstractPMTC: execute after(Config)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
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/tests/Functional/Handlers/AbstractInstallTestCase.phpERROR: Referenced sniff "Drupal" does not exist Run "phpcs --help" for usage information .vortex/installer/tests/Functional/Handlers/AiCodeInstructionsInstallTest.phpERROR: Referenced sniff "Drupal" does not exist Run "phpcs --help" for usage information .vortex/installer/tests/Functional/Handlers/BaselineInstallTest.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/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1937 +/- ##
===========================================
- Coverage 78.87% 76.21% -2.66%
===========================================
Files 87 87
Lines 5381 5381
Branches 35 35
===========================================
- Hits 4244 4101 -143
- Misses 1137 1280 +143 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 73
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (43)
.vortex/CLAUDE.md(1 hunks).vortex/installer/tests/Functional/Handlers/AbstractInstallTestCase.php(1 hunks).vortex/installer/tests/Functional/Handlers/AiCodeInstructionsInstallTest.php(1 hunks).vortex/installer/tests/Functional/Handlers/BaselineInstallTest.php(1 hunks).vortex/installer/tests/Functional/Handlers/CiProviderInstallTest.php(1 hunks).vortex/installer/tests/Functional/Handlers/CodeProviderInstallTest.php(1 hunks).vortex/installer/tests/Functional/Handlers/DatabaseDownloadSourceInstallTest.php(1 hunks).vortex/installer/tests/Functional/Handlers/DependencyUpdatesProviderInstallTest.php(1 hunks).vortex/installer/tests/Functional/Handlers/DeployTypeInstallTest.php(1 hunks).vortex/installer/tests/Functional/Handlers/DocsInstallTest.php(1 hunks).vortex/installer/tests/Functional/Handlers/HostingProviderInstallTest.php(1 hunks).vortex/installer/tests/Functional/Handlers/NamesInstallTest.php(1 hunks).vortex/installer/tests/Functional/Handlers/ProfileInstallTest.php(1 hunks).vortex/installer/tests/Functional/Handlers/ProvisionTypeInstallTest.php(1 hunks).vortex/installer/tests/Functional/Handlers/PullRequestInstallTest.php(1 hunks).vortex/installer/tests/Functional/Handlers/ServicesInstallTest.php(1 hunks).vortex/installer/tests/Functional/Handlers/ThemeInstallTest.php(1 hunks).vortex/installer/tests/Functional/Handlers/TimezoneInstallTest.php(1 hunks).vortex/installer/tests/Functional/Handlers/ToolsInstallTest.php(1 hunks).vortex/installer/tests/Functional/InstallTest.php(0 hunks).vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php(1 hunks).vortex/installer/tests/Unit/Handlers/AiCodeInstructionsPromptManagerTest.php(1 hunks).vortex/installer/tests/Unit/Handlers/BaselinePromptManagerTest.php(1 hunks).vortex/installer/tests/Unit/Handlers/CiProviderPromptManagerTest.php(1 hunks).vortex/installer/tests/Unit/Handlers/CodeProviderPromptManagerTest.php(1 hunks).vortex/installer/tests/Unit/Handlers/DatabaseDownloadSourcePromptManagerTest.php(1 hunks).vortex/installer/tests/Unit/Handlers/DatabaseImagePromptManagerTest.php(1 hunks).vortex/installer/tests/Unit/Handlers/DependencyUpdatesProviderPromptManagerTest.php(1 hunks).vortex/installer/tests/Unit/Handlers/DeployTypePromptManagerTest.php(1 hunks).vortex/installer/tests/Unit/Handlers/DocsPromptManagerTest.php(1 hunks).vortex/installer/tests/Unit/Handlers/DomainPromptManagerTest.php(1 hunks).vortex/installer/tests/Unit/Handlers/HostingProviderPromptManagerTest.php(1 hunks).vortex/installer/tests/Unit/Handlers/ModulePrefixPromptManagerTest.php(1 hunks).vortex/installer/tests/Unit/Handlers/NamesPromptManagerTest.php(1 hunks).vortex/installer/tests/Unit/Handlers/ProfilePromptManagerTest.php(1 hunks).vortex/installer/tests/Unit/Handlers/ProvisionTypePromptManagerTest.php(1 hunks).vortex/installer/tests/Unit/Handlers/PullRequestPromptManagerTest.php(1 hunks).vortex/installer/tests/Unit/Handlers/ServicesPromptManagerTest.php(1 hunks).vortex/installer/tests/Unit/Handlers/ThemePromptManagerTest.php(1 hunks).vortex/installer/tests/Unit/Handlers/TimezonePromptManagerTest.php(1 hunks).vortex/installer/tests/Unit/Handlers/ToolsPromptManagerTest.php(1 hunks).vortex/installer/tests/Unit/Handlers/WebrootPromptManagerTest.php(1 hunks).vortex/installer/tests/Unit/PromptManagerTest.php(0 hunks)
💤 Files with no reviewable changes (2)
- .vortex/installer/tests/Unit/PromptManagerTest.php
- .vortex/installer/tests/Functional/InstallTest.php
🧰 Additional context used
🧬 Code graph analysis (40)
.vortex/installer/tests/Unit/Handlers/NamesPromptManagerTest.php (8)
.vortex/installer/src/Prompts/Handlers/Domain.php (1)
Domain(12-93).vortex/installer/src/Prompts/Handlers/MachineName.php (1)
MachineName(12-101).vortex/installer/src/Prompts/Handlers/ModulePrefix.php (1)
ModulePrefix(10-112).vortex/installer/src/Prompts/Handlers/Name.php (1)
Name(11-85).vortex/installer/src/Prompts/Handlers/Org.php (1)
Org(11-88).vortex/installer/src/Prompts/Handlers/OrgMachineName.php (1)
OrgMachineName(11-89).vortex/installer/src/Prompts/Handlers/Theme.php (1)
Theme(11-181).vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (6)
AbstractPromptManagerTestCase(46-267)dataProviderRunPrompts(111-111)getExpectedDefaults(116-144)getExpectedDiscovered(165-176)stubDotenvValue(245-251)stubComposerJsonValue(214-219)
.vortex/installer/tests/Functional/Handlers/DependencyUpdatesProviderInstallTest.php (4)
.vortex/installer/src/Prompts/Handlers/CiProvider.php (1)
CiProvider(9-121).vortex/installer/src/Prompts/Handlers/DependencyUpdatesProvider.php (1)
DependencyUpdatesProvider(9-96).vortex/installer/src/Prompts/PromptManager.php (2)
PromptManager(57-536)makeEnvName(398-400).vortex/installer/src/Utils/Env.php (2)
Env(7-239)put(24-26)
.vortex/installer/tests/Unit/Handlers/CiProviderPromptManagerTest.php (2)
.vortex/installer/src/Prompts/Handlers/CiProvider.php (1)
CiProvider(9-121).vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (5)
AbstractPromptManagerTestCase(46-267)dataProviderRunPrompts(111-111)getExpectedDefaults(116-144)getExpectedInstalled(149-160)stubVortexProject(253-259)
.vortex/installer/tests/Functional/Handlers/DatabaseDownloadSourceInstallTest.php (6)
.vortex/installer/src/Prompts/Handlers/AiCodeInstructions.php (1)
AiCodeInstructions(7-71).vortex/installer/src/Prompts/Handlers/DatabaseDownloadSource.php (1)
DatabaseDownloadSource(10-120).vortex/installer/src/Prompts/Handlers/DatabaseImage.php (1)
DatabaseImage(12-106).vortex/installer/src/Prompts/PromptManager.php (2)
PromptManager(57-536)makeEnvName(398-400).vortex/installer/src/Utils/Env.php (2)
Env(7-239)put(24-26).vortex/installer/tests/Functional/Handlers/AbstractInstallTestCase.php (2)
AbstractInstallTestCase(19-96)dataProviderInstall(62-62)
.vortex/installer/tests/Unit/Handlers/DependencyUpdatesProviderPromptManagerTest.php (3)
.vortex/installer/src/Prompts/Handlers/CiProvider.php (1)
CiProvider(9-121).vortex/installer/src/Prompts/Handlers/DependencyUpdatesProvider.php (1)
DependencyUpdatesProvider(9-96).vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (4)
AbstractPromptManagerTestCase(46-267)getExpectedDefaults(116-144)getExpectedInstalled(149-160)stubVortexProject(253-259)
.vortex/installer/tests/Unit/Handlers/DocsPromptManagerTest.php (5)
.vortex/installer/src/Prompts/Handlers/PreserveDocsOnboarding.php (1)
PreserveDocsOnboarding(9-54).vortex/installer/src/Prompts/Handlers/PreserveDocsProject.php (1)
PreserveDocsProject(9-54).vortex/installer/src/Utils/Config.php (1)
Config(14-126).vortex/installer/src/Utils/File.php (1)
File(10-102).vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (4)
AbstractPromptManagerTestCase(46-267)getExpectedDefaults(116-144)getExpectedInstalled(149-160)stubVortexProject(253-259)
.vortex/installer/tests/Unit/Handlers/TimezonePromptManagerTest.php (3)
.vortex/installer/src/Prompts/Handlers/Timezone.php (1)
Timezone(10-666).vortex/installer/src/Utils/Config.php (1)
Config(14-126).vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (6)
AbstractPromptManagerTestCase(46-267)dataProviderRunPrompts(111-111)getExpectedDefaults(116-144)getExpectedInstalled(149-160)stubVortexProject(253-259)stubDotenvValue(245-251)
.vortex/installer/tests/Functional/Handlers/ServicesInstallTest.php (5)
.vortex/installer/src/Prompts/Handlers/AiCodeInstructions.php (1)
AiCodeInstructions(7-71).vortex/installer/src/Prompts/Handlers/Services.php (1)
Services(10-158).vortex/installer/src/Prompts/PromptManager.php (2)
PromptManager(57-536)makeEnvName(398-400).vortex/installer/src/Utils/Env.php (2)
Env(7-239)put(24-26).vortex/installer/tests/Functional/FunctionalTestCase.php (1)
assertSutNotContains(139-154)
.vortex/installer/tests/Functional/Handlers/AbstractInstallTestCase.php (2)
.vortex/installer/tests/Functional/FunctionalTestCase.php (2)
runNonInteractiveInstall(94-116)FunctionalTestCase(23-156).vortex/installer/tests/Unit/UnitTestCase.php (3)
replaceVersions(49-62)assertYamlFileIsValid(116-123)assertJsonFileIsValid(125-134)
.vortex/installer/tests/Unit/Handlers/PullRequestPromptManagerTest.php (5)
.vortex/installer/src/Prompts/Handlers/AssignAuthorPr.php (1)
AssignAuthorPr(7-47).vortex/installer/src/Prompts/Handlers/LabelMergeConflictsPr.php (1)
LabelMergeConflictsPr(7-48).vortex/installer/src/Utils/Config.php (1)
Config(14-126).vortex/installer/src/Utils/File.php (1)
File(10-102).vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (5)
AbstractPromptManagerTestCase(46-267)dataProviderRunPrompts(111-111)getExpectedDefaults(116-144)getExpectedInstalled(149-160)stubVortexProject(253-259)
.vortex/installer/tests/Unit/Handlers/CodeProviderPromptManagerTest.php (5)
.vortex/installer/src/Prompts/Handlers/CiProvider.php (1)
CiProvider(9-121).vortex/installer/src/Prompts/Handlers/CodeProvider.php (1)
CodeProvider(9-77).vortex/installer/src/Utils/Config.php (1)
Config(14-126).vortex/installer/src/Utils/Git.php (1)
Git(11-113).vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (4)
AbstractPromptManagerTestCase(46-267)getExpectedDefaults(116-144)getExpectedInstalled(149-160)stubVortexProject(253-259)
.vortex/installer/tests/Functional/Handlers/TimezoneInstallTest.php (4)
.vortex/installer/src/Prompts/Handlers/CiProvider.php (1)
CiProvider(9-121).vortex/installer/src/Prompts/Handlers/Timezone.php (1)
Timezone(10-666).vortex/installer/src/Prompts/PromptManager.php (2)
PromptManager(57-536)makeEnvName(398-400).vortex/installer/src/Utils/Env.php (2)
Env(7-239)put(24-26)
.vortex/installer/tests/Unit/Handlers/DatabaseDownloadSourcePromptManagerTest.php (2)
.vortex/installer/src/Prompts/Handlers/DatabaseDownloadSource.php (1)
DatabaseDownloadSource(10-120).vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (4)
AbstractPromptManagerTestCase(46-267)dataProviderRunPrompts(111-111)getExpectedDefaults(116-144)stubDotenvValue(245-251)
.vortex/installer/tests/Functional/Handlers/CiProviderInstallTest.php (4)
.vortex/installer/src/Prompts/Handlers/AiCodeInstructions.php (1)
AiCodeInstructions(7-71).vortex/installer/src/Prompts/Handlers/CiProvider.php (1)
CiProvider(9-121).vortex/installer/src/Prompts/PromptManager.php (2)
PromptManager(57-536)makeEnvName(398-400).vortex/installer/src/Utils/Env.php (2)
Env(7-239)put(24-26)
.vortex/installer/tests/Unit/Handlers/ProvisionTypePromptManagerTest.php (3)
.vortex/installer/src/Prompts/Handlers/DatabaseDownloadSource.php (1)
DatabaseDownloadSource(10-120).vortex/installer/src/Prompts/Handlers/ProvisionType.php (1)
ProvisionType(10-80).vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (3)
AbstractPromptManagerTestCase(46-267)getExpectedDefaults(116-144)stubDotenvValue(245-251)
.vortex/installer/tests/Unit/Handlers/ModulePrefixPromptManagerTest.php (3)
.vortex/installer/src/Prompts/Handlers/ModulePrefix.php (1)
ModulePrefix(10-112).vortex/installer/src/Utils/File.php (1)
File(10-102).vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (3)
AbstractPromptManagerTestCase(46-267)dataProviderRunPrompts(111-111)getExpectedDefaults(116-144)
.vortex/installer/tests/Functional/Handlers/PullRequestInstallTest.php (4)
.vortex/installer/src/Prompts/Handlers/AssignAuthorPr.php (1)
AssignAuthorPr(7-47).vortex/installer/src/Prompts/Handlers/LabelMergeConflictsPr.php (1)
LabelMergeConflictsPr(7-48).vortex/installer/src/Prompts/PromptManager.php (2)
PromptManager(57-536)makeEnvName(398-400).vortex/installer/src/Utils/Env.php (2)
Env(7-239)put(24-26)
.vortex/installer/tests/Unit/Handlers/ThemePromptManagerTest.php (4)
.vortex/installer/src/Prompts/Handlers/Theme.php (1)
Theme(11-181).vortex/installer/src/Utils/Config.php (1)
Config(14-126).vortex/installer/src/Utils/File.php (1)
File(10-102).vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (6)
AbstractPromptManagerTestCase(46-267)dataProviderRunPrompts(111-111)getExpectedDefaults(116-144)getExpectedInstalled(149-160)stubVortexProject(253-259)stubDotenvValue(245-251)
.vortex/installer/tests/Unit/Handlers/DeployTypePromptManagerTest.php (4)
.vortex/installer/src/Prompts/Handlers/DeployType.php (1)
DeployType(11-121).vortex/installer/src/Utils/Config.php (1)
Config(14-126).vortex/installer/src/Utils/Converter.php (1)
Converter(9-17).vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (4)
AbstractPromptManagerTestCase(46-267)dataProviderRunPrompts(111-111)getExpectedDefaults(116-144)stubDotenvValue(245-251)
.vortex/installer/tests/Functional/Handlers/ToolsInstallTest.php (5)
.vortex/installer/src/Prompts/Handlers/CiProvider.php (1)
CiProvider(9-121).vortex/installer/src/Prompts/Handlers/Tools.php (2)
Tools(14-337)getToolDefinitions(178-335).vortex/installer/src/Prompts/PromptManager.php (2)
PromptManager(57-536)makeEnvName(398-400).vortex/installer/src/Utils/Env.php (2)
Env(7-239)put(24-26).vortex/installer/tests/Functional/FunctionalTestCase.php (2)
assertSutNotContains(139-154)assertSutContains(122-137)
.vortex/installer/tests/Functional/Handlers/NamesInstallTest.php (2)
.vortex/installer/src/Prompts/PromptManager.php (2)
PromptManager(57-536)makeEnvName(398-400).vortex/installer/src/Utils/Env.php (2)
Env(7-239)put(24-26)
.vortex/installer/tests/Functional/Handlers/CodeProviderInstallTest.php (4)
.vortex/installer/src/Prompts/Handlers/CodeProvider.php (1)
CodeProvider(9-77).vortex/installer/src/Prompts/PromptManager.php (2)
PromptManager(57-536)makeEnvName(398-400).vortex/installer/src/Utils/Env.php (2)
Env(7-239)put(24-26).vortex/installer/tests/Functional/Handlers/AbstractInstallTestCase.php (2)
AbstractInstallTestCase(19-96)dataProviderInstall(62-62)
.vortex/installer/tests/Unit/Handlers/DomainPromptManagerTest.php (2)
.vortex/installer/src/Prompts/Handlers/Domain.php (1)
Domain(12-93).vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (3)
AbstractPromptManagerTestCase(46-267)getExpectedDefaults(116-144)stubDotenvValue(245-251)
.vortex/installer/tests/Functional/Handlers/DocsInstallTest.php (4)
.vortex/installer/src/Prompts/Handlers/PreserveDocsOnboarding.php (1)
PreserveDocsOnboarding(9-54).vortex/installer/src/Prompts/Handlers/PreserveDocsProject.php (1)
PreserveDocsProject(9-54).vortex/installer/src/Prompts/PromptManager.php (2)
PromptManager(57-536)makeEnvName(398-400).vortex/installer/src/Utils/Env.php (2)
Env(7-239)put(24-26)
.vortex/installer/tests/Functional/Handlers/ProfileInstallTest.php (3)
.vortex/installer/src/Prompts/Handlers/Profile.php (1)
Profile(10-134).vortex/installer/src/Prompts/PromptManager.php (2)
PromptManager(57-536)makeEnvName(398-400).vortex/installer/src/Utils/Env.php (2)
Env(7-239)put(24-26)
.vortex/installer/tests/Unit/Handlers/BaselinePromptManagerTest.php (1)
.vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (6)
AbstractPromptManagerTestCase(46-267)dataProviderRunPrompts(111-111)getExpectedDefaults(116-144)getExpectedInstalled(149-160)stubComposerJsonValue(214-219)stubVortexProject(253-259)
.vortex/installer/tests/Functional/Handlers/AiCodeInstructionsInstallTest.php (3)
.vortex/installer/src/Prompts/Handlers/AiCodeInstructions.php (1)
AiCodeInstructions(7-71).vortex/installer/src/Prompts/PromptManager.php (2)
PromptManager(57-536)makeEnvName(398-400).vortex/installer/src/Utils/Env.php (2)
Env(7-239)put(24-26)
.vortex/installer/tests/Unit/Handlers/ToolsPromptManagerTest.php (3)
.vortex/installer/src/Prompts/Handlers/Tools.php (1)
Tools(14-337).vortex/installer/src/Utils/Config.php (1)
Config(14-126).vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (5)
AbstractPromptManagerTestCase(46-267)getExpectedDefaults(116-144)getExpectedInstalled(149-160)stubVortexProject(253-259)stubComposerJsonDependencies(221-243)
.vortex/installer/tests/Unit/Handlers/AiCodeInstructionsPromptManagerTest.php (2)
.vortex/installer/src/Prompts/Handlers/AiCodeInstructions.php (1)
AiCodeInstructions(7-71).vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (5)
AbstractPromptManagerTestCase(46-267)dataProviderRunPrompts(111-111)getExpectedDefaults(116-144)getExpectedInstalled(149-160)stubVortexProject(253-259)
.vortex/installer/tests/Unit/Handlers/DatabaseImagePromptManagerTest.php (4)
.vortex/installer/src/Prompts/Handlers/DatabaseDownloadSource.php (1)
DatabaseDownloadSource(10-120).vortex/installer/src/Prompts/Handlers/DatabaseImage.php (1)
DatabaseImage(12-106).vortex/installer/src/Utils/Config.php (1)
Config(14-126).vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (4)
AbstractPromptManagerTestCase(46-267)dataProviderRunPrompts(111-111)getExpectedDefaults(116-144)stubDotenvValue(245-251)
.vortex/installer/tests/Unit/Handlers/ServicesPromptManagerTest.php (4)
.vortex/installer/src/Prompts/Handlers/Services.php (1)
Services(10-158).vortex/installer/src/Utils/Config.php (1)
Config(14-126).vortex/installer/src/Utils/File.php (1)
File(10-102).vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (5)
AbstractPromptManagerTestCase(46-267)dataProviderRunPrompts(111-111)getExpectedDefaults(116-144)getExpectedInstalled(149-160)stubVortexProject(253-259)
.vortex/installer/tests/Functional/Handlers/ProvisionTypeInstallTest.php (5)
.vortex/installer/src/Prompts/Handlers/AiCodeInstructions.php (1)
AiCodeInstructions(7-71).vortex/installer/src/Prompts/Handlers/HostingProvider.php (1)
HostingProvider(10-120).vortex/installer/src/Prompts/Handlers/ProvisionType.php (1)
ProvisionType(10-80).vortex/installer/src/Prompts/PromptManager.php (2)
PromptManager(57-536)makeEnvName(398-400).vortex/installer/src/Utils/Env.php (2)
Env(7-239)put(24-26)
.vortex/installer/tests/Unit/Handlers/ProfilePromptManagerTest.php (3)
.vortex/installer/src/Prompts/Handlers/Profile.php (1)
Profile(10-134).vortex/installer/src/Utils/Config.php (1)
Config(14-126).vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (6)
AbstractPromptManagerTestCase(46-267)dataProviderRunPrompts(111-111)getExpectedDefaults(116-144)getExpectedInstalled(149-160)stubVortexProject(253-259)stubDotenvValue(245-251)
.vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (4)
.vortex/installer/tests/Unit/UnitTestCase.php (1)
UnitTestCase(23-136).vortex/installer/src/Utils/Config.php (2)
Config(14-126)set(82-93).vortex/installer/src/Utils/File.php (1)
File(10-102).vortex/installer/tests/Traits/TuiTrait.php (2)
tuiSetUp(15-33)tuiTeardown(35-37)
.vortex/installer/tests/Unit/Handlers/HostingProviderPromptManagerTest.php (6)
.vortex/installer/src/Prompts/Handlers/DatabaseDownloadSource.php (1)
DatabaseDownloadSource(10-120).vortex/installer/src/Prompts/Handlers/DeployType.php (1)
DeployType(11-121).vortex/installer/src/Prompts/Handlers/HostingProvider.php (1)
HostingProvider(10-120).vortex/installer/src/Prompts/Handlers/Webroot.php (1)
Webroot(12-145).vortex/installer/src/Utils/Config.php (1)
Config(14-126).vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (4)
AbstractPromptManagerTestCase(46-267)dataProviderRunPrompts(111-111)getExpectedDefaults(116-144)stubDotenvValue(245-251)
.vortex/installer/tests/Functional/Handlers/HostingProviderInstallTest.php (5)
.vortex/installer/src/Prompts/Handlers/AiCodeInstructions.php (1)
AiCodeInstructions(7-71).vortex/installer/src/Prompts/Handlers/HostingProvider.php (1)
HostingProvider(10-120).vortex/installer/src/Prompts/PromptManager.php (2)
PromptManager(57-536)makeEnvName(398-400).vortex/installer/src/Utils/Env.php (2)
Env(7-239)put(24-26).vortex/installer/tests/Functional/FunctionalTestCase.php (1)
assertSutNotContains(139-154)
.vortex/installer/tests/Functional/Handlers/BaselineInstallTest.php (2)
.vortex/installer/tests/Functional/Handlers/AbstractInstallTestCase.php (2)
AbstractInstallTestCase(19-96)dataProviderInstall(62-62).vortex/installer/tests/Functional/InstallTest.php (1)
CoversClass(54-715)
.vortex/installer/tests/Functional/Handlers/ThemeInstallTest.php (4)
.vortex/installer/src/Prompts/Handlers/AiCodeInstructions.php (1)
AiCodeInstructions(7-71).vortex/installer/src/Prompts/Handlers/Theme.php (1)
Theme(11-181).vortex/installer/src/Prompts/PromptManager.php (2)
PromptManager(57-536)makeEnvName(398-400).vortex/installer/src/Utils/Env.php (2)
Env(7-239)put(24-26)
.vortex/installer/tests/Unit/Handlers/WebrootPromptManagerTest.php (4)
.vortex/installer/src/Prompts/Handlers/HostingProvider.php (1)
HostingProvider(10-120).vortex/installer/src/Prompts/Handlers/Webroot.php (1)
Webroot(12-145).vortex/installer/src/Utils/Config.php (1)
Config(14-126).vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (5)
AbstractPromptManagerTestCase(46-267)dataProviderRunPrompts(111-111)getExpectedDefaults(116-144)stubDotenvValue(245-251)stubComposerJsonValue(214-219)
.vortex/installer/tests/Functional/Handlers/DeployTypeInstallTest.php (4)
.vortex/installer/src/Prompts/Handlers/CiProvider.php (1)
CiProvider(9-121).vortex/installer/src/Prompts/Handlers/DeployType.php (1)
DeployType(11-121).vortex/installer/src/Prompts/PromptManager.php (2)
PromptManager(57-536)makeEnvName(398-400).vortex/installer/src/Utils/Env.php (2)
Env(7-239)put(24-26)
🪛 LanguageTool
.vortex/CLAUDE.md
[grammar] ~589-~589: Use correct spacing
Context: ...handlers ## Installer Test Architecture ### Handler-Specific Test Classes The insta...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~591-~591: Use correct spacing
Context: ...cture ### Handler-Specific Test Classes The installer tests have been refactored...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~593-~593: Use correct spacing
Context: ...nability and test execution flexibility. Abstract Base Class: `AbstractInstallT...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~595-~595: There might be a mistake here.
Context: ...all installer test scenarios, including: - Common setup and teardown procedures - C...
(QB_NEW_EN)
[grammar] ~596-~596: There might be a mistake here.
Context: ...: - Common setup and teardown procedures - Core testInstall() method with data pr...
(QB_NEW_EN)
[grammar] ~597-~597: There might be a mistake here.
Context: ...)` method with data provider integration - Fixture management and assertion helpers...
(QB_NEW_EN)
[grammar] ~598-~598: There might be a mistake here.
Context: ...Fixture management and assertion helpers - Version replacement utilities **Handler...
(QB_NEW_EN)
[grammar] ~599-~599: Use correct spacing
Context: ... helpers - Version replacement utilities Handler Test Organization: Each instal...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~601-~601: Use correct spacing
Context: ...ract base class. This approach provides: - Focused Testing: Each test class cover...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~603-~603: There might be a mistake here.
Context: ... specific to one handler or feature area - Better Maintainability: Smaller, focus...
(QB_NEW_EN_OTHER)
[grammar] ~606-~606: There might be a mistake here.
Context: ...ler tests following established patterns Key Benefits: - Run all handler tests:...
(QB_NEW_EN_OTHER)
[grammar] ~608-~608: There might be a mistake here.
Context: ... established patterns Key Benefits: - Run all handler tests: `--filter "Handle...
(QB_NEW_EN)
[grammar] ~609-~609: There might be a mistake here.
Context: ...ey Benefits**: - Run all handler tests: --filter "Handlers\\\\" - Run specific handler: `--filter "Handler...
(QB_NEW_EN)
[grammar] ~610-~610: There might be a mistake here.
Context: ..."Handlers\\"- Run specific handler:--filter "HandlerNameInstallTest"- Run specific scenarios:--filter "Handl...
(QB_NEW_EN)
[grammar] ~611-~611: There might be a mistake here.
Context: ...InstallTest"- Run specific scenarios:--filter "HandlerNameInstallTest.*scenario_pattern"` - Consistent structure across all handler ...
(QB_NEW_EN)
[grammar] ~612-~612: There might be a mistake here.
Context: ...tructure across all handler test classes - Clear separation between test logic (in ...
(QB_NEW_EN)
[grammar] ~613-~613: Use correct spacing
Context: ...lass) and test data (in handler classes) Usage with Fixture Updates: The `UPDAT...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~615-~615: Use correct spacing
Context: ...dates across all handler test scenarios. ## Resources - Documentation: `.vortex...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🪛 markdownlint-cli2 (0.17.2)
.vortex/CLAUDE.md
596-596: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
609-609: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
⏰ 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 (0)
- GitHub Check: build (1)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (1)
- 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.2)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-docs
🔇 Additional comments (52)
.vortex/installer/tests/Unit/Handlers/DependencyUpdatesProviderPromptManagerTest.php (2)
21-26: Good coverage of the prompt default pathThe ENTER-path returning the handler default (RENOVATEBOT_CI) aligns with DependencyUpdatesProvider::default().
33-46: Verify directory creation semantics of File::dump for nested pathsYou’re dumping nested paths (e.g.,
/.github/workflows/...,/.circleci/config.yml). If File::dump does not create parent directories, these calls will fail intermittently. Please confirm that File::dump ensures parent directory creation, or pre-create directories in the setup closure to make the test robust.Would you like me to add a small helper to the test base to mkdirp before dumps?
.vortex/installer/tests/Functional/Handlers/ServicesInstallTest.php (1)
18-53: Well-scoped negative assertions for each services scenarioThe data-provider scenarios and “not contains” checks map cleanly to Services::process() behavior for CLAMAV, SOLR, and VALKEY/REDIS. Good inclusion of both “valkey” and “redis” to catch legacy references.
.vortex/installer/tests/Unit/Handlers/DatabaseDownloadSourcePromptManagerTest.php (2)
14-21: LGTM: default selection via ENTER maps to URL as expected.The test correctly validates the default path when no hosting provider bias is set. Use of the array union operator preserves the override semantics.
23-29: Good discovery-path coverage.Stubbing VORTEX_DB_DOWNLOAD_SOURCE in a local .env and expecting FTP mirrors discover() and PromptManager validation. Solid.
.vortex/installer/tests/Functional/Handlers/AbstractInstallTestCase.php (1)
66-68: Verification Needed: Confirm the actual location ofvortex.ymlI wasn’t able to locate any
vortex.ymlin the repository (including hidden dirs and without excludingvendor) and the test is currently checking:if (file_exists(static::$root . '/scripts/vortex.yml')) { $this->assertFileEquals( static::$root . '/tests/behat/fixtures/image.jpg', static::$sut . '/tests/behat/fixtures/image.jpg', 'Binary files were not modified.' ); }• Please confirm where
vortex.ymlactually resides (is it generated at runtime, in a different directory, or committed under another path?).
• Once confirmed, update thefile_exists()check to point at the real location. For example, if it lives under.vortex/vortex.yml, change to:- if (file_exists(static::$root . '/scripts/vortex.yml')) { + if (file_exists(static::$root . '/.vortex/vortex.yml')) {.vortex/installer/tests/Functional/Handlers/CodeProviderInstallTest.php (1)
26-31: LGTM — clear coverage and assertions for the “other” providerThe “other” scenario correctly drives the handler and verifies the .github directory is removed.
.vortex/installer/tests/Functional/Handlers/ToolsInstallTest.php (1)
21-52: Good negative/positive coverage for “tools, none”Solid assertions: you verify backend tools and their artifacts are removed while generic lint targets remain.
.vortex/installer/tests/Functional/Handlers/TimezoneInstallTest.php (3)
1-4: LGTM! Standard PHP file header.The file follows PHP best practices with strict types declaration.
20-43: Well-structured test for timezone with GitHub Actions CI.The test correctly:
- Sets timezone to America/New_York via environment variable
- Verifies replacements in .env and renovate.json
- Confirms that GHA and docker-compose files retain UTC (as they should be overridden via UI, not code)
This follows the expected pattern for functional tests in this PR.
45-56: Good coverage for CircleCI timezone handling.The test appropriately validates that CircleCI config maintains UTC timezone when UI overrides are expected, consistent with the GHA test case above.
.vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (4)
41-46: Clear and comprehensive class documentation.The PHPDoc accurately describes the purpose and scope of this abstract base class.
54-60: Proper test setup with TUI initialization.The setUp method correctly initializes TUI testing and creates the test project directory structure.
71-106: Robust test runner with proper exception handling.The testRunPrompts method provides excellent flexibility:
- Supports both successful responses and exception scenarios
- Allows before/after hooks for test setup/teardown
- Properly merges default answers with test-specific inputs
- Uses the data provider pattern effectively
115-144: Comprehensive default expectations for all prompt handlers.The getExpectedDefaults method provides a complete set of default values for all installer prompts, serving as a single source of truth for expected behaviors across tests.
.vortex/installer/tests/Unit/Handlers/ToolsPromptManagerTest.php (5)
12-13: Correct test coverage annotation.The CoversClass attribute properly indicates this test covers the Tools handler.
20-23: Comprehensive test for default tool selection.Tests that pressing ENTER selects all default tools (PHPCS, PHPMD, PHPStan, Rector, PHPUnit, Behat).
25-40: Excellent coverage for tool discovery in Vortex projects.The test properly validates that all tools are discovered when their packages are present in composer.json. The alphabetical sorting in the expected result matches the Tools handler's discovery implementation.
51-65: Good boundary testing for non-Vortex projects.The test correctly verifies that tool discovery is skipped for non-Vortex projects, falling back to defaults even when tool packages are present.
67-118: Thorough individual tool discovery tests.Each tool has its own test case verifying discovery when only that specific package is present. This provides excellent granular coverage.
.vortex/installer/tests/Unit/Handlers/DatabaseImagePromptManagerTest.php (4)
13-14: Proper test class setup.The class correctly extends AbstractPromptManagerTestCase and uses the CoversClass attribute.
20-26: Good happy path test for container image input.Tests successful input of a valid container registry image with proper tag format.
28-42: Excellent validation testing.Both test cases properly validate that invalid image formats are rejected:
- Invalid format with multiple colons
- Invalid capitalization (container images should be lowercase)
This matches the Validator::containerImage validation in the DatabaseImage handler.
44-55: Proper discovery test implementation.The test correctly validates that the VORTEX_DB_IMAGE environment variable is discovered and used as the default value.
.vortex/installer/tests/Unit/Handlers/ProvisionTypePromptManagerTest.php (3)
13-14: Correct test class configuration.The class properly extends the base test case and covers the ProvisionType handler.
20-23: Simple and effective default selection test.Tests that pressing ENTER selects the default DATABASE provision type.
33-39: Important test for profile provisioning impact.The test correctly validates that when PROFILE is selected, DatabaseDownloadSource is set to NONE (since profiles don't need database downloads). This cross-handler interaction is critical to test.
.vortex/installer/tests/Unit/Handlers/BaselinePromptManagerTest.php (2)
18-26: Coverage annotations look appropriate for baseline flowGood choice to pin coverage to Internal/PromptManager and the key utility classes exercised by the baseline prompts run.
28-56: Data provider scenarios are well structured
- Defaults vs installed contexts are clearly separated.
- The “installed project - minimal” case is useful to ensure composer.json:type is optional for discovery.
No issues spotted here.
.vortex/installer/tests/Unit/Handlers/ThemePromptManagerTest.php (1)
44-50: Nice coverage of non-Vortex discovery pathSimulating a non-Vortex theme file in web/themes/custom ensures discovery works without isInstalled() being set. Looks good.
.vortex/installer/tests/Unit/Handlers/DocsPromptManagerTest.php (1)
22-31: ENTER handling and expectations look consistentUsing Key::ENTER to accept TRUE defaults for docs preservation matches the handlers’ defaults. Scenarios for installed/non-installed are covered.
.vortex/installer/tests/Unit/Handlers/DeployTypePromptManagerTest.php (1)
25-39: Good checks for normalized order on discoveryDeployType::discover() sorts types; your tests verify both direct and reversed env order normalize to [artifact, webhook]. Nice.
.vortex/installer/tests/Unit/Handlers/HostingProviderPromptManagerTest.php (3)
23-27: Good default-path coverage for interactive selectionSelecting ENTER to accept HostingProvider::default and asserting NONE matches handler behavior. Looks solid.
28-39: Acquia discovery path is exercised correctlyStubbed hooks directory ensures discover() resolves to ACQUIA; dependent expectations (Webroot DOCROOT, DeployType ARTIFACT, DB source ACQUIA) align with handlers. Nice.
41-52: ENV-based Acquia discovery mirrors production heuristicsStubbing VORTEX_DB_DOWNLOAD_SOURCE=acquia to drive HostingProvider discovery is accurate and keeps the test fast. Good use of closure setup.
.vortex/installer/tests/Unit/Handlers/WebrootPromptManagerTest.php (1)
50-65: Nice coverage for both dotenv- and composer-based discoveryThese scenarios match Webroot::discover() branches and keep expectations precise. Good job.
.vortex/installer/tests/Unit/Handlers/NamesPromptManagerTest.php (3)
28-40: End-to-end defaults propagation from Name → dependent prompts looks correctThe expected MachineName, Org, OrgMachineName, Domain, ModulePrefix, and Theme all align with handlers’ defaults given the prompted Name. Well composed.
107-117: Hyphenated machine name discovery edge case is covered—niceAsserting ‘discovered-project’ propagates to Domain and leaves Theme normalized is a good catch for Converter differences.
166-189: Confirm default discovery bypasses validation for hyphenated org machine namesI inspected PromptManager.php and saw that every handler—including OrgMachineName—always gets its
validate()callback wired up (e.g. PromptManager.php:494) and there’s no special case to disable validation when a default comes fromdiscover()(PromptManager.php:121). Please verify at runtime that Symfony’s question helper will accept a discovered default (even if it contains a hyphen) without invoking that validator callback. If it doesn’t, we should either:
- Remove or null out the validator when
discover()supplies the default,- Or introduce a flag so that only user-entered answers run through
validate().This will ensure the “org machine name – discovery – hyphenated” test continues to pass.
.vortex/installer/tests/Unit/Handlers/ServicesPromptManagerTest.php (3)
21-25: LGTM: interactive path mirrors defaultsENTER selects the default set [CLAMAV, SOLR, VALKEY] per Services::default. Straightforward and correct.
27-72: Well-constructed discovery scenarios with minimal fixturesUsing stubVortexProject plus docker-compose.yml snippets to drive each branch (single, all, none) is clean and focused. Sorting in handler is reflected in expectations. Nice.
87-93: Non-Vortex project fallback coveredEnsures discover() respects isInstalled() short-circuit. Good guardrail.
.vortex/installer/tests/Unit/Handlers/ProfilePromptManagerTest.php (2)
31-34: Invalid custom profile path aligns with handler expectationsError message matches the validation rule. Good negative coverage.
45-51: Discovery for non-Vortex project is realistic and minimalSeeding an info file under web/profiles is enough to trigger discovery; assertion is focused. Nice.
.vortex/installer/tests/Unit/Handlers/DomainPromptManagerTest.php (1)
17-35: Coverage looks solid; normalization and validation paths are exercisedThe scenarios cover protocol stripping, www-prefix, invalid input, and .env discovery. Good breadth for the handler’s behavior.
.vortex/installer/tests/Functional/Handlers/CiProviderInstallTest.php (1)
5-14: No Namespace Issues Detected
TheAbstractInstallTestCaseclass is declared inDrevOps\Installer\Tests\Functional\Handlers—the same namespace used byCiProviderInstallTest.php—so PHPUnit will correctly locate and extend it. No changes are required..vortex/installer/tests/Unit/Handlers/CiProviderPromptManagerTest.php (1)
26-50: LGTM: Discovery scenarios accurately reflect handler logic
- GHA: presence of
.github/workflows/build-test-deploy.yml- CircleCI: presence of
.circleci/config.yml- NONE: installed project without either file
Assertions align withCiProvider::discover()semantics..vortex/installer/tests/Unit/Handlers/CodeProviderPromptManagerTest.php (1)
28-34: Good dependency interplay test between CodeProvider and CiProviderSelecting "Other" for code provider and asserting
CiProvider::NONEvalidates the options-filtering behavior inCiProvider::options(). Nice coverage..vortex/installer/tests/Unit/Handlers/AiCodeInstructionsPromptManagerTest.php (4)
13-19: Solid test scaffolding and coverage targeting.Strict types, correct namespace/imports, and #[CoversClass(AiCodeInstructions::class)] are all on point. The data provider shape matches AbstractPromptManagerTestCase::testRunPrompts signature.
26-33: Discovery scenario correctly reflects handler behavior (installed + CLAUDE.md).Stubbing a Vortex project and writing CLAUDE.md to dstDir is a faithful simulation; expectation of CLAUDE on top of installed defaults is correct.
35-41: Discovery-removed scenario is accurate (installed, file absent ⇒ NONE).Matches AiCodeInstructions::discover() path returning NONE when installed but CLAUDE.md is missing.
43-49: Non-Vortex discovery behavior is captured well.Creating CLAUDE.md without installation rightly results in default (NONE), because discover() returns NULL when not installed. Good negative coverage.
| ## Installer Test Architecture | ||
| ### Handler-Specific Test Classes | ||
| The installer tests have been refactored to use a modular, handler-focused architecture that improves maintainability and test execution flexibility. | ||
| **Abstract Base Class**: `AbstractInstallTestCase` provides shared test logic for all installer test scenarios, including: | ||
| - Common setup and teardown procedures | ||
| - Core `testInstall()` method with data provider integration | ||
| - Fixture management and assertion helpers | ||
| - Version replacement utilities | ||
| **Handler Test Organization**: Each installer handler has its own dedicated test class in the `Handlers/` namespace that extends the abstract base class. This approach provides: | ||
| - **Focused Testing**: Each test class covers scenarios specific to one handler or feature area | ||
| - **Better Maintainability**: Smaller, focused data providers that are easier to understand and modify | ||
| - **Improved Filtering**: Granular test execution capabilities using PHPUnit filters | ||
| - **Scalable Architecture**: Easy to add new handler tests following established patterns | ||
| **Key Benefits**: | ||
| - Run all handler tests: `--filter "Handlers\\\\"` | ||
| - Run specific handler: `--filter "HandlerNameInstallTest"` | ||
| - Run specific scenarios: `--filter "HandlerNameInstallTest.*scenario_pattern"` | ||
| - Consistent structure across all handler test classes | ||
| - Clear separation between test logic (in base class) and test data (in handler classes) | ||
| **Usage with Fixture Updates**: The `UPDATE_FIXTURES=1` mechanism works seamlessly with the new architecture, allowing systematic fixture updates across all handler test scenarios. | ||
| ## Resources |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Fix Markdown list spacing and potential duplicated section
- Lists at “Abstract Base Class … including:” and “Key Benefits” aren’t surrounded by blank lines (MD032). Add blank lines before and after lists.
- Please also verify that “Installer Test Architecture” is not duplicated elsewhere in this doc. The PR summary mentions a duplicated new section.
@@
-**Abstract Base Class**: `AbstractInstallTestCase` provides shared test logic for all installer test scenarios, including:
+**Abstract Base Class**: `AbstractInstallTestCase` provides shared test logic for all installer test scenarios, including:
+
- Common setup and teardown procedures
- Core `testInstall()` method with data provider integration
- Fixture management and assertion helpers
- Version replacement utilities
+
@@
-**Key Benefits**:
+**Key Benefits**:
+
- Run all handler tests: `--filter "Handlers\\\\"`
- Run specific handler: `--filter "HandlerNameInstallTest"`
- Run specific scenarios: `--filter "HandlerNameInstallTest.*scenario_pattern"`
- Consistent structure across all handler test classes
- Clear separation between test logic (in base class) and test data (in handler classes)
+
-**Usage with Fixture Updates**: The `UPDATE_FIXTURES=1` mechanism works seamlessly with the new architecture, allowing systematic fixture updates across all handler test scenarios.
+**Usage with Fixture Updates**: The `UPDATE_FIXTURES=1` mechanism works seamlessly with the new architecture, allowing systematic fixture updates across all handler test scenarios.To check for duplicates quickly:
🏁 Script executed:
#!/bin/bash
# Find repeated "Installer Test Architecture" headings in this doc
rg -nP '^\s*##\s+Installer Test Architecture\b' .vortex/CLAUDE.md -C2Length of output: 265
Fix Markdown list spacing and confirm no duplicate section
The “Installer Test Architecture” heading appears only once in .vortex/CLAUDE.md (no duplicates found). Please update the Markdown around your lists to satisfy MD032:
• Add a blank line before and after the list under Abstract Base Class
• Add a blank line before and after the list under Key Benefits
Example adjustment:
## Installer Test Architecture
### Handler-Specific Test Classes
**Abstract Base Class**: `AbstractInstallTestCase` provides shared test logic for all installer test scenarios, including:
+
- Common setup and teardown procedures
- Core `testInstall()` method with data provider integration
- Fixture management and assertion helpers
- Version replacement utilities
+
**Handler Test Organization**: Each installer handler …
**Key Benefits**:
+
- Run all handler tests: `--filter "Handlers\\\\"`
- Run specific handler: `--filter "HandlerNameInstallTest"`
- Run specific scenarios: `--filter "HandlerNameInstallTest.*scenario_pattern"`
- Consistent structure across all handler test classes
- Clear separation between test logic (in base class) and test data (in handler classes)
+
**Usage with Fixture Updates**: The `UPDATE_FIXTURES=1` mechanism works seamlessly with the new architecture…📝 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.
| ## Installer Test Architecture | |
| ### Handler-Specific Test Classes | |
| The installer tests have been refactored to use a modular, handler-focused architecture that improves maintainability and test execution flexibility. | |
| **Abstract Base Class**: `AbstractInstallTestCase` provides shared test logic for all installer test scenarios, including: | |
| - Common setup and teardown procedures | |
| - Core `testInstall()` method with data provider integration | |
| - Fixture management and assertion helpers | |
| - Version replacement utilities | |
| **Handler Test Organization**: Each installer handler has its own dedicated test class in the `Handlers/` namespace that extends the abstract base class. This approach provides: | |
| - **Focused Testing**: Each test class covers scenarios specific to one handler or feature area | |
| - **Better Maintainability**: Smaller, focused data providers that are easier to understand and modify | |
| - **Improved Filtering**: Granular test execution capabilities using PHPUnit filters | |
| - **Scalable Architecture**: Easy to add new handler tests following established patterns | |
| **Key Benefits**: | |
| - Run all handler tests: `--filter "Handlers\\\\"` | |
| - Run specific handler: `--filter "HandlerNameInstallTest"` | |
| - Run specific scenarios: `--filter "HandlerNameInstallTest.*scenario_pattern"` | |
| - Consistent structure across all handler test classes | |
| - Clear separation between test logic (in base class) and test data (in handler classes) | |
| **Usage with Fixture Updates**: The `UPDATE_FIXTURES=1` mechanism works seamlessly with the new architecture, allowing systematic fixture updates across all handler test scenarios. | |
| ## Resources | |
| ## Installer Test Architecture | |
| ### Handler-Specific Test Classes | |
| The installer tests have been refactored to use a modular, handler-focused architecture that improves maintainability and test execution flexibility. | |
| **Abstract Base Class**: `AbstractInstallTestCase` provides shared test logic for all installer test scenarios, including: | |
| - Common setup and teardown procedures | |
| - Core `testInstall()` method with data provider integration | |
| - Fixture management and assertion helpers | |
| - Version replacement utilities | |
| **Handler Test Organization**: Each installer handler has its own dedicated test class in the `Handlers/` namespace that extends the abstract base class. This approach provides: | |
| - **Focused Testing**: Each test class covers scenarios specific to one handler or feature area | |
| - **Better Maintainability**: Smaller, focused data providers that are easier to understand and modify | |
| - **Improved Filtering**: Granular test execution capabilities using PHPUnit filters | |
| - **Scalable Architecture**: Easy to add new handler tests following established patterns | |
| **Key Benefits**: | |
| - Run all handler tests: `--filter "Handlers\\\\"` | |
| - Run specific handler: `--filter "HandlerNameInstallTest"` | |
| - Run specific scenarios: `--filter "HandlerNameInstallTest.*scenario_pattern"` | |
| - Consistent structure across all handler test classes | |
| - Clear separation between test logic (in base class) and test data (in handler classes) | |
| **Usage with Fixture Updates**: The `UPDATE_FIXTURES=1` mechanism works seamlessly with the new architecture, allowing systematic fixture updates across all handler test scenarios. | |
| ## Resources |
🧰 Tools
🪛 LanguageTool
[grammar] ~589-~589: Use correct spacing
Context: ...handlers ## Installer Test Architecture ### Handler-Specific Test Classes The insta...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~591-~591: Use correct spacing
Context: ...cture ### Handler-Specific Test Classes The installer tests have been refactored...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~593-~593: Use correct spacing
Context: ...nability and test execution flexibility. Abstract Base Class: `AbstractInstallT...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~595-~595: There might be a mistake here.
Context: ...all installer test scenarios, including: - Common setup and teardown procedures - C...
(QB_NEW_EN)
[grammar] ~596-~596: There might be a mistake here.
Context: ...: - Common setup and teardown procedures - Core testInstall() method with data pr...
(QB_NEW_EN)
[grammar] ~597-~597: There might be a mistake here.
Context: ...)` method with data provider integration - Fixture management and assertion helpers...
(QB_NEW_EN)
[grammar] ~598-~598: There might be a mistake here.
Context: ...Fixture management and assertion helpers - Version replacement utilities **Handler...
(QB_NEW_EN)
[grammar] ~599-~599: Use correct spacing
Context: ... helpers - Version replacement utilities Handler Test Organization: Each instal...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~601-~601: Use correct spacing
Context: ...ract base class. This approach provides: - Focused Testing: Each test class cover...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~603-~603: There might be a mistake here.
Context: ... specific to one handler or feature area - Better Maintainability: Smaller, focus...
(QB_NEW_EN_OTHER)
[grammar] ~606-~606: There might be a mistake here.
Context: ...ler tests following established patterns Key Benefits: - Run all handler tests:...
(QB_NEW_EN_OTHER)
[grammar] ~608-~608: There might be a mistake here.
Context: ... established patterns Key Benefits: - Run all handler tests: `--filter "Handle...
(QB_NEW_EN)
[grammar] ~609-~609: There might be a mistake here.
Context: ...ey Benefits**: - Run all handler tests: --filter "Handlers\\\\" - Run specific handler: `--filter "Handler...
(QB_NEW_EN)
[grammar] ~610-~610: There might be a mistake here.
Context: ..."Handlers\\"- Run specific handler:--filter "HandlerNameInstallTest"- Run specific scenarios:--filter "Handl...
(QB_NEW_EN)
[grammar] ~611-~611: There might be a mistake here.
Context: ...InstallTest"- Run specific scenarios:--filter "HandlerNameInstallTest.*scenario_pattern"` - Consistent structure across all handler ...
(QB_NEW_EN)
[grammar] ~612-~612: There might be a mistake here.
Context: ...tructure across all handler test classes - Clear separation between test logic (in ...
(QB_NEW_EN)
[grammar] ~613-~613: Use correct spacing
Context: ...lass) and test data (in handler classes) Usage with Fixture Updates: The `UPDAT...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~615-~615: Use correct spacing
Context: ...dates across all handler test scenarios. ## Resources - Documentation: `.vortex...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~617-~617: Use correct spacing
Context: ...ll handler test scenarios. ## Resources - Documentation: .vortex/docs/ and htt...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🪛 markdownlint-cli2 (0.17.2)
596-596: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
609-609: Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
🤖 Prompt for AI Agents
.vortex/CLAUDE.md around lines 589-617: the unordered lists under "Abstract Base
Class" and "Key Benefits" need blank lines inserted before and after each list
to satisfy MD032; edit the Markdown so there is an empty line above the first
"-" line and an empty line after the last "-" line in both sections (do not
change the list content or headings and do not add duplicate headings).
.vortex/installer/tests/Functional/Handlers/AbstractInstallTestCase.php
Outdated
Show resolved
Hide resolved
| public static function dataProviderInstall(): array { | ||
| return [ | ||
| 'ai instructions, claude' => [ | ||
| static::cw(fn() => Env::put(PromptManager::makeEnvName(AiCodeInstructions::id()), AiCodeInstructions::CLAUDE)), | ||
| ], | ||
|
|
||
| 'ai instructions, none' => [ | ||
| static::cw(fn() => Env::put(PromptManager::makeEnvName(AiCodeInstructions::id()), AiCodeInstructions::NONE)), | ||
| ], | ||
| ]; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add a minimal assertion for the “none” path
To make the outcome explicit, assert that the SUT does not contain the AI instructions when NONE is selected. This mirrors the other handler tests’ style.
@@
'ai instructions, none' => [
- static::cw(fn() => Env::put(PromptManager::makeEnvName(AiCodeInstructions::id()), AiCodeInstructions::NONE)),
+ static::cw(fn() => Env::put(PromptManager::makeEnvName(AiCodeInstructions::id()), AiCodeInstructions::NONE)),
+ // Ensure CLAUDE.md (or its contents) are not left in the SUT
+ static::cw(fn(\DrevOps\VortexInstaller\Tests\Functional\FunctionalTestCase $t) => $t->assertSutNotContains('CLAUDE.md')),
],If you prefer path-based assertions, I can add a helper to FunctionalTestCase to assert file absence by relative path.
📝 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.
| public static function dataProviderInstall(): array { | |
| return [ | |
| 'ai instructions, claude' => [ | |
| static::cw(fn() => Env::put(PromptManager::makeEnvName(AiCodeInstructions::id()), AiCodeInstructions::CLAUDE)), | |
| ], | |
| 'ai instructions, none' => [ | |
| static::cw(fn() => Env::put(PromptManager::makeEnvName(AiCodeInstructions::id()), AiCodeInstructions::NONE)), | |
| ], | |
| ]; | |
| public static function dataProviderInstall(): array { | |
| return [ | |
| 'ai instructions, claude' => [ | |
| static::cw(fn() => Env::put(PromptManager::makeEnvName(AiCodeInstructions::id()), AiCodeInstructions::CLAUDE)), | |
| ], | |
| 'ai instructions, none' => [ | |
| static::cw(fn() => Env::put(PromptManager::makeEnvName(AiCodeInstructions::id()), AiCodeInstructions::NONE)), | |
| // Ensure CLAUDE.md (or its contents) are not left in the SUT | |
| static::cw(fn(\DrevOps\VortexInstaller\Tests\Functional\FunctionalTestCase $t) => $t->assertSutNotContains('CLAUDE.md')), | |
| ], | |
| ]; | |
| } |
🤖 Prompt for AI Agents
.vortex/installer/tests/Functional/Handlers/AiCodeInstructionsInstallTest.php
around lines 15-24: the data provider includes a "none" case but there is no
explicit assertion that the resulting SUT does not contain AI instructions;
update the test that consumes this dataProvider to add a minimal assertion for
the NONE path — e.g. after exercising the installer, assert that the AI
instructions file or config entry is absent (or that the generated files do not
contain the AI instruction marker), matching the style of the other handler
tests; if the test helper pattern is used elsewhere, reuse the same
file-existence or content-absence assertion method, otherwise add a one-line
assertion verifying absence.
| #[CoversClass(AssignAuthorPr::class)] | ||
| #[CoversClass(CiProvider::class)] | ||
| #[CoversClass(CodeProvider::class)] | ||
| #[CoversClass(DatabaseDownloadSource::class)] | ||
| #[CoversClass(DatabaseImage::class)] | ||
| #[CoversClass(DependencyUpdatesProvider::class)] | ||
| #[CoversClass(DeployType::class)] | ||
| #[CoversClass(Domain::class)] | ||
| #[CoversClass(HostingProvider::class)] | ||
| #[CoversClass(Internal::class)] | ||
| #[CoversClass(LabelMergeConflictsPr::class)] | ||
| #[CoversClass(MachineName::class)] | ||
| #[CoversClass(ModulePrefix::class)] | ||
| #[CoversClass(Name::class)] | ||
| #[CoversClass(Org::class)] | ||
| #[CoversClass(OrgMachineName::class)] | ||
| #[CoversClass(PreserveDocsOnboarding::class)] | ||
| #[CoversClass(PreserveDocsProject::class)] | ||
| #[CoversClass(Profile::class)] | ||
| #[CoversClass(ProvisionType::class)] | ||
| #[CoversClass(Services::class)] | ||
| #[CoversClass(Theme::class)] | ||
| #[CoversClass(Timezone::class)] | ||
| #[CoversClass(Webroot::class)] | ||
| #[CoversClass(PromptManager::class)] | ||
| #[CoversClass(Downloader::class)] | ||
| #[CoversClass(Config::class)] | ||
| #[CoversClass(Git::class)] | ||
| #[CoversClass(Tui::class)] | ||
| class BaselineInstallTest extends AbstractInstallTestCase { |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider trimming CoversClass attributes to what this test actually exercises
Many of these classes are now covered by dedicated per-handler tests. Keeping BaselineInstallTest coverage focused (e.g., install command + essential utilities) prevents inflated or misleading coverage attribution.
If you agree, I can propose a pruned list aligned with the code paths executed here.
🤖 Prompt for AI Agents
.vortex/installer/tests/Functional/Handlers/BaselineInstallTest.php lines 38-67:
the test currently lists a large number of #[CoversClass(...)] attributes that
inflate coverage; trim this list to only the classes actually exercised by this
test (e.g., the install command handler and essential utilities it invokes) by:
inspect the test body to determine which handlers/services are executed, remove
CoversClass entries for classes covered by their own focused tests, and keep
only the minimal set required to accurately attribute coverage for
BaselineInstallTest; update the attribute block accordingly and run the test
coverage to verify attribution matches expected execution paths.
| 'non-interactive' => [ | ||
| NULL, | ||
| NULL, | ||
| ['Welcome to the Vortex non-interactive installer'], | ||
| ], |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove duplicate dataset
The ‘non-interactive’ dataset duplicates the baseline case verbatim and adds no new signal. Dropping it reduces runtime and noise.
Apply this diff:
- 'non-interactive' => [
- NULL,
- NULL,
- ['Welcome to the Vortex non-interactive installer'],
- ],📝 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.
| 'non-interactive' => [ | |
| NULL, | |
| NULL, | |
| ['Welcome to the Vortex non-interactive installer'], | |
| ], |
🤖 Prompt for AI Agents
.vortex/installer/tests/Functional/Handlers/BaselineInstallTest.php lines 77-81:
the 'non-interactive' dataset is a duplicate of the baseline and should be
removed to reduce runtime and noise; delete the entire 'non-interactive' array
entry (the NULL, NULL, ['Welcome to the Vortex non-interactive installer']
block) and fix surrounding array punctuation/commas so the datasets array
remains valid PHP.
| 'tools, groups, no be lint' => [ | ||
| static::cw(function (): void { | ||
| $tools = array_keys(Tools::getToolDefinitions('tools')); | ||
| Env::put(PromptManager::makeEnvName(Tools::id()), Converter::toList(array_diff($tools, [Tools::PHPCS, Tools::PHPMD, Tools::PHPSTAN, Tools::RECTOR]))); | ||
| Env::put(PromptManager::makeEnvName(CiProvider::id()), CiProvider::GITHUB_ACTIONS); | ||
| }), | ||
| static::cw(fn(FunctionalTestCase $test) => $test->assertSutNotContains([ | ||
| 'phpcs', | ||
| 'phpcbf', | ||
| 'dealerdirect/phpcodesniffer-composer-installer', | ||
| 'drupal/coder', | ||
| 'squizlabs/php_codesniffer', | ||
| 'phpmd', | ||
| 'phpmd/phpmd', | ||
| 'phpstan', | ||
| 'phpstan/phpstan', | ||
| 'mglaman/phpstan-drupal', | ||
| 'rector', | ||
| 'rector/rector', | ||
| ])), | ||
| ], |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Strengthen “groups, no backend lint” by asserting group-level Ahoy targets are gone
The Tools handler removes group commands if no group members are selected. Add explicit checks for “lint-be:” and “lint-be-fix:” vanishing to catch regressions in processGroup.
Apply this diff:
static::cw(fn(FunctionalTestCase $test) => $test->assertSutNotContains([
'phpcs',
'phpcbf',
'dealerdirect/phpcodesniffer-composer-installer',
'drupal/coder',
'squizlabs/php_codesniffer',
'phpmd',
'phpmd/phpmd',
'phpstan',
'phpstan/phpstan',
'mglaman/phpstan-drupal',
'rector',
'rector/rector',
+ '/\blint-be:/',
+ '/\blint-be-fix:/',
])),📝 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.
| 'tools, groups, no be lint' => [ | |
| static::cw(function (): void { | |
| $tools = array_keys(Tools::getToolDefinitions('tools')); | |
| Env::put(PromptManager::makeEnvName(Tools::id()), Converter::toList(array_diff($tools, [Tools::PHPCS, Tools::PHPMD, Tools::PHPSTAN, Tools::RECTOR]))); | |
| Env::put(PromptManager::makeEnvName(CiProvider::id()), CiProvider::GITHUB_ACTIONS); | |
| }), | |
| static::cw(fn(FunctionalTestCase $test) => $test->assertSutNotContains([ | |
| 'phpcs', | |
| 'phpcbf', | |
| 'dealerdirect/phpcodesniffer-composer-installer', | |
| 'drupal/coder', | |
| 'squizlabs/php_codesniffer', | |
| 'phpmd', | |
| 'phpmd/phpmd', | |
| 'phpstan', | |
| 'phpstan/phpstan', | |
| 'mglaman/phpstan-drupal', | |
| 'rector', | |
| 'rector/rector', | |
| ])), | |
| ], | |
| 'tools, groups, no be lint' => [ | |
| static::cw(function (): void { | |
| $tools = array_keys(Tools::getToolDefinitions('tools')); | |
| Env::put(PromptManager::makeEnvName(Tools::id()), Converter::toList(array_diff($tools, [Tools::PHPCS, Tools::PHPMD, Tools::PHPSTAN, Tools::RECTOR]))); | |
| Env::put(PromptManager::makeEnvName(CiProvider::id()), CiProvider::GITHUB_ACTIONS); | |
| }), | |
| static::cw(fn(FunctionalTestCase $test) => $test->assertSutNotContains([ | |
| 'phpcs', | |
| 'phpcbf', | |
| 'dealerdirect/phpcodesniffer-composer-installer', | |
| 'drupal/coder', | |
| 'squizlabs/php_codesniffer', | |
| 'phpmd', | |
| 'phpmd/phpmd', | |
| 'phpstan', | |
| 'phpstan/phpstan', | |
| 'mglaman/phpstan-drupal', | |
| 'rector', | |
| 'rector/rector', | |
| '/\blint-be:/', | |
| '/\blint-be-fix:/', | |
| ])), | |
| ], |
🤖 Prompt for AI Agents
.vortex/installer/tests/Functional/Handlers/ToolsInstallTest.php around lines
221 to 241: the test case "tools, groups, no be lint" currently asserts that
individual backend lint tools are not present but does not assert that
group-level targets ("lint-be:" and "lint-be-fix:") are removed; update the test
to also assert those group targets are absent by adding checks for 'lint-be:'
and 'lint-be-fix:' to the assertSutNotContains expectation (either by adding
them to the existing array or adding an extra assertion closure that checks
those two strings are not present).
| 'tools, groups, no be tests' => [ | ||
| static::cw(function (): void { | ||
| $tools = array_keys(Tools::getToolDefinitions('tools')); | ||
| Env::put(PromptManager::makeEnvName(Tools::id()), Converter::toList(array_diff($tools, [Tools::PHPUNIT, Tools::BEHAT]))); | ||
| Env::put(PromptManager::makeEnvName(CiProvider::id()), CiProvider::GITHUB_ACTIONS); | ||
| }), | ||
| static::cw(fn(FunctionalTestCase $test) => $test->assertSutNotContains([ | ||
| 'phpunit', | ||
| 'ahoy test-unit', | ||
| 'ahoy test-kernel', | ||
| 'ahoy test-functional', | ||
| 'behat', | ||
| 'behat/behat', | ||
| 'drupal/drupal-extension', | ||
| 'ahoy test-bdd', | ||
| 'gherkinlint', | ||
| 'gherkin-lint', | ||
| 'gherkin', | ||
| ])), | ||
| ], | ||
|
|
||
| 'tools, groups, no be tests, circleci' => [ | ||
| static::cw(function (): void { | ||
| $tools = array_keys(Tools::getToolDefinitions('tools')); | ||
| Env::put(PromptManager::makeEnvName(Tools::id()), Converter::toList(array_diff($tools, [Tools::PHPUNIT, Tools::BEHAT]))); | ||
| Env::put(PromptManager::makeEnvName(CiProvider::id()), CiProvider::CIRCLECI); | ||
| }), | ||
| static::cw(fn(FunctionalTestCase $test) => $test->assertSutNotContains([ | ||
| 'phpunit', | ||
| 'ahoy test-unit', | ||
| 'ahoy test-kernel', | ||
| 'ahoy test-functional', | ||
| 'behat', | ||
| 'behat/behat', | ||
| 'drupal/drupal-extension', | ||
| 'ahoy test-bdd', | ||
| 'gherkinlint', | ||
| 'gherkin-lint', | ||
| 'gherkin', | ||
| ])), | ||
| ], |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Likewise for “groups, no backend tests”
Consider asserting the test-related Ahoy aggregator(s) are removed (you already check specific test-* commands). If “test:” is meant to aggregate backend tests only, include it; otherwise, leave as-is.
If you want, I can add the exact assertions once we confirm whether “test:” aggregates only PHP tests in your templates.
🤖 Prompt for AI Agents
.vortex/installer/tests/Functional/Handlers/ToolsInstallTest.php around lines
265 to 305 — the test cases for "groups, no be tests" and "groups, no be tests,
circleci" currently assert specific test-related commands are absent but do not
assert whether the Ahoy test aggregator (the "test:" target) is removed; decide
whether your templates treat "test:" as an aggregator for backend (PHP) tests
only — if so, add "test:" to the assertSutNotContains arrays in both test cases;
if "test:" is intended to remain for other purposes, leave as-is.
| 'database download source - discovery - invalid' => [ | ||
| [], | ||
| $expected_defaults, | ||
| function (AbstractPromptManagerTestCase $test): void { | ||
| $test->stubDotenvValue('VORTEX_DB_DOWNLOAD_SOURCE', 'invalid_source'); | ||
| }, | ||
| ], |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add a “shouldRun=false” scenario (ProvisionType::PROFILE) to ensure the prompt is skipped.
DatabaseDownloadSource::shouldRun() returns false for PROFILE. Add a scenario toggling ProvisionType to PROFILE and assert the handler’s value is absent from responses.
'database download source - discovery - invalid' => [
[],
$expected_defaults,
function (AbstractPromptManagerTestCase $test): void {
$test->stubDotenvValue('VORTEX_DB_DOWNLOAD_SOURCE', 'invalid_source');
},
],
+
+ 'database download source - skipped when provision=profile' => [
+ [/* answers forcing ProvisionType::PROFILE, rest defaults */],
+ array_diff_key($expected_defaults, [DatabaseDownloadSource::id() => true]),
+ function (AbstractPromptManagerTestCase $test): void {
+ // Force profile provisioning in defaults/mock answers; depends on TUI helper API.
+ // Example if supported: Prompt::answer(ProvisionType::id(), ProvisionType::PROFILE);
+ },
+ ],If the TUI helper doesn’t allow forcing another prompt directly, we can extend AbstractPromptManagerTestCase to support overriding defaults for specific ids.
Committable suggestion skipped: line range outside the PR's diff.
| 'dependency updates provider - discovery - renovate self-hosted - gha' => [ | ||
| [], | ||
| [DependencyUpdatesProvider::id() => DependencyUpdatesProvider::RENOVATEBOT_CI] + $expected_installed, | ||
| function (AbstractPromptManagerTestCase $test, Config $config): void { | ||
| $test->stubVortexProject($config); | ||
| File::dump(static::$sut . '/renovate.json'); | ||
| File::dump(static::$sut . '/.github/workflows/update-dependencies.yml'); | ||
| }, | ||
| ], |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Make CI provider discovery expectations symmetrical (GHA vs CircleCI)
Currently, CircleCI discovery both creates .circleci/config.yml and asserts CiProvider::CIRCLECI, while the GHA case creates only update-dependencies.yml and asserts nothing for CI (leaving CiProvider::NONE from installed defaults). This asymmetry is easy to misread.
Two consistent options:
- Either keep the test focused on DependencyUpdatesProvider and drop the explicit CircleCI expectation, or
- Assert GHA as well by creating the file that CiProvider::discover() actually checks (
.github/workflows/build-test-deploy.yml).
Suggestion below implements the latter for symmetry.
@@
- 'dependency updates provider - discovery - renovate self-hosted - gha' => [
+ 'dependency updates provider - discovery - renovate self-hosted - gha' => [
[],
- [DependencyUpdatesProvider::id() => DependencyUpdatesProvider::RENOVATEBOT_CI] + $expected_installed,
+ [
+ CiProvider::id() => CiProvider::GITHUB_ACTIONS,
+ DependencyUpdatesProvider::id() => DependencyUpdatesProvider::RENOVATEBOT_CI,
+ ] + $expected_installed,
function (AbstractPromptManagerTestCase $test, Config $config): void {
$test->stubVortexProject($config);
File::dump(static::$sut . '/renovate.json');
File::dump(static::$sut . '/.github/workflows/update-dependencies.yml');
+ // Required for CiProvider discovery:
+ File::dump(static::$sut . '/.github/workflows/build-test-deploy.yml');
},
],Also applies to: 37-48
🤖 Prompt for AI Agents
.vortex/installer/tests/Unit/Handlers/DependencyUpdatesProviderPromptManagerTest.php
around lines 27-35 (and similarly 37-48): the GHA test currently only creates
update-dependencies.yml and therefore does not trigger CiProvider discovery,
creating an asymmetry with the CircleCI case; update the GHA test fixture to
create the actual workflow file that CiProvider::discover() checks (e.g. create
.github/workflows/build-test-deploy.yml under the stubbed project) and add the
corresponding expected CI provider entry (the GHA enum/constant) to the
expected_installed array so the test asserts GitHub Actions is discovered, and
make the same change for the other GHA test block at 37-48.
| public static function dataProviderInstall(): array { | ||
| return [ | ||
| 'ciprovider, gha' => [ | ||
| static::cw(function (): void { | ||
| Env::put(PromptManager::makeEnvName(CiProvider::id()), CiProvider::GITHUB_ACTIONS); | ||
| Env::put(PromptManager::makeEnvName(AiCodeInstructions::id()), AiCodeInstructions::CLAUDE); | ||
| }), | ||
| ], | ||
|
|
||
| 'ciprovider, circleci' => [ | ||
| static::cw(function (): void { | ||
| Env::put(PromptManager::makeEnvName(CiProvider::id()), CiProvider::CIRCLECI); | ||
| Env::put(PromptManager::makeEnvName(AiCodeInstructions::id()), AiCodeInstructions::CLAUDE); | ||
| }), | ||
| ], | ||
| ]; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider adding a NONE scenario and post-install assertions
Currently only GHA and CircleCI are covered. Adding a CiProvider::NONE case would validate the “remove both providers” branch in process(). If AbstractInstallTestCase supports after-hooks, assert the correct files/tokens are removed/preserved post-install.
Example addition:
'ciprovider, circleci' => [
static::cw(function (): void {
Env::put(PromptManager::makeEnvName(CiProvider::id()), CiProvider::CIRCLECI);
Env::put(PromptManager::makeEnvName(AiCodeInstructions::id()), AiCodeInstructions::CLAUDE);
}),
],
+ 'ciprovider, none' => [
+ static::cw(function (): void {
+ Env::put(PromptManager::makeEnvName(CiProvider::id()), CiProvider::NONE);
+ }),
+ ],📝 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.
| public static function dataProviderInstall(): array { | |
| return [ | |
| 'ciprovider, gha' => [ | |
| static::cw(function (): void { | |
| Env::put(PromptManager::makeEnvName(CiProvider::id()), CiProvider::GITHUB_ACTIONS); | |
| Env::put(PromptManager::makeEnvName(AiCodeInstructions::id()), AiCodeInstructions::CLAUDE); | |
| }), | |
| ], | |
| 'ciprovider, circleci' => [ | |
| static::cw(function (): void { | |
| Env::put(PromptManager::makeEnvName(CiProvider::id()), CiProvider::CIRCLECI); | |
| Env::put(PromptManager::makeEnvName(AiCodeInstructions::id()), AiCodeInstructions::CLAUDE); | |
| }), | |
| ], | |
| ]; | |
| public static function dataProviderInstall(): array { | |
| return [ | |
| 'ciprovider, gha' => [ | |
| static::cw(function (): void { | |
| Env::put(PromptManager::makeEnvName(CiProvider::id()), CiProvider::GITHUB_ACTIONS); | |
| Env::put(PromptManager::makeEnvName(AiCodeInstructions::id()), AiCodeInstructions::CLAUDE); | |
| }), | |
| ], | |
| 'ciprovider, circleci' => [ | |
| static::cw(function (): void { | |
| Env::put(PromptManager::makeEnvName(CiProvider::id()), CiProvider::CIRCLECI); | |
| Env::put(PromptManager::makeEnvName(AiCodeInstructions::id()), AiCodeInstructions::CLAUDE); | |
| }), | |
| ], | |
| 'ciprovider, none' => [ | |
| static::cw(function (): void { | |
| Env::put(PromptManager::makeEnvName(CiProvider::id()), CiProvider::NONE); | |
| }), | |
| ], | |
| ]; | |
| } |
🤖 Prompt for AI Agents
.vortex/installer/tests/Functional/Handlers/CiProviderInstallTest.php lines
16-31: the data provider only contains GHA and CircleCI cases so the code path
that handles removing both providers isn't exercised; add an additional data
provider entry for 'ciprovider, none' that sets the environment to
CiProvider::NONE (via PromptManager::makeEnvName(CiProvider::id())) and the same
AiCodeInstructions value, and if AbstractInstallTestCase supports after-hooks
add an after-hook or post-install assertions to validate that provider-specific
files/tokens have been removed and shared artifacts preserved (i.e. assert
absence of GHA/CircleCI files and presence/absence of expected tokens).
.vortex/installer/tests/Functional/Handlers/TimezoneInstallTest.php
Outdated
Show resolved
Hide resolved
| protected function stubComposerJsonValue(string $name, mixed $value): string { | ||
| $composer_json = static::$sut . DIRECTORY_SEPARATOR . 'composer.json'; | ||
| file_put_contents($composer_json, json_encode([$name => $value], JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES)); | ||
|
|
||
| return $composer_json; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider adding error handling for JSON encoding.
The stubComposerJsonValue method should handle potential JSON encoding failures.
Apply this improvement:
protected function stubComposerJsonValue(string $name, mixed $value): string {
$composer_json = static::$sut . DIRECTORY_SEPARATOR . 'composer.json';
- file_put_contents($composer_json, json_encode([$name => $value], JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES));
+ $json = json_encode([$name => $value], JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES);
+ if ($json === false) {
+ throw new \RuntimeException('Failed to encode JSON: ' . json_last_error_msg());
+ }
+ file_put_contents($composer_json, $json);
return $composer_json;
}📝 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.
| protected function stubComposerJsonValue(string $name, mixed $value): string { | |
| $composer_json = static::$sut . DIRECTORY_SEPARATOR . 'composer.json'; | |
| file_put_contents($composer_json, json_encode([$name => $value], JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES)); | |
| return $composer_json; | |
| } | |
| protected function stubComposerJsonValue(string $name, mixed $value): string { | |
| $composer_json = static::$sut . DIRECTORY_SEPARATOR . 'composer.json'; | |
| $json = json_encode([$name => $value], JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES); | |
| if ($json === false) { | |
| throw new \RuntimeException('Failed to encode JSON: ' . json_last_error_msg()); | |
| } | |
| file_put_contents($composer_json, $json); | |
| return $composer_json; | |
| } |
🤖 Prompt for AI Agents
.vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php around
lines 214 to 219: the stubComposerJsonValue method currently assumes json_encode
and file_put_contents always succeed; update it to check json_encode result for
false and use json_last_error_msg() to obtain the error, and verify
file_put_contents did not return false, throwing a RuntimeException (or failing
the test) with a descriptive message that includes the JSON error and the target
path if either operation fails; ensure the method returns the path only after
both operations succeed.
| protected function stubComposerJsonDependencies(array $dependencies, bool $is_dev = FALSE): string { | ||
| $composer_json = static::$sut . DIRECTORY_SEPARATOR . 'composer.json'; | ||
| $section = $is_dev ? 'require-dev' : 'require'; | ||
|
|
||
| $data = []; | ||
| if (file_exists($composer_json)) { | ||
| $contents = file_get_contents($composer_json); | ||
| $existing = $contents !== FALSE ? json_decode($contents, TRUE) : NULL; | ||
| if ($existing) { | ||
| $data = $existing; | ||
| } | ||
| } | ||
|
|
||
| if (!isset($data[$section])) { | ||
| $data[$section] = []; | ||
| } | ||
|
|
||
| $data[$section] = array_merge($data[$section], $dependencies); | ||
|
|
||
| file_put_contents($composer_json, json_encode($data, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES)); | ||
|
|
||
| return $composer_json; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider adding JSON validation after merge.
The stubComposerJsonDependencies method should validate the merged JSON structure.
Apply this improvement:
protected function stubComposerJsonDependencies(array $dependencies, bool $is_dev = FALSE): string {
$composer_json = static::$sut . DIRECTORY_SEPARATOR . 'composer.json';
$section = $is_dev ? 'require-dev' : 'require';
$data = [];
if (file_exists($composer_json)) {
$contents = file_get_contents($composer_json);
$existing = $contents !== FALSE ? json_decode($contents, TRUE) : NULL;
if ($existing) {
$data = $existing;
}
}
if (!isset($data[$section])) {
$data[$section] = [];
}
$data[$section] = array_merge($data[$section], $dependencies);
- file_put_contents($composer_json, json_encode($data, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES));
+ $json = json_encode($data, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES);
+ if ($json === false) {
+ throw new \RuntimeException('Failed to encode JSON: ' . json_last_error_msg());
+ }
+ file_put_contents($composer_json, $json);
return $composer_json;
}📝 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.
| protected function stubComposerJsonDependencies(array $dependencies, bool $is_dev = FALSE): string { | |
| $composer_json = static::$sut . DIRECTORY_SEPARATOR . 'composer.json'; | |
| $section = $is_dev ? 'require-dev' : 'require'; | |
| $data = []; | |
| if (file_exists($composer_json)) { | |
| $contents = file_get_contents($composer_json); | |
| $existing = $contents !== FALSE ? json_decode($contents, TRUE) : NULL; | |
| if ($existing) { | |
| $data = $existing; | |
| } | |
| } | |
| if (!isset($data[$section])) { | |
| $data[$section] = []; | |
| } | |
| $data[$section] = array_merge($data[$section], $dependencies); | |
| file_put_contents($composer_json, json_encode($data, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES)); | |
| return $composer_json; | |
| } | |
| protected function stubComposerJsonDependencies(array $dependencies, bool $is_dev = FALSE): string { | |
| $composer_json = static::$sut . DIRECTORY_SEPARATOR . 'composer.json'; | |
| $section = $is_dev ? 'require-dev' : 'require'; | |
| $data = []; | |
| if (file_exists($composer_json)) { | |
| $contents = file_get_contents($composer_json); | |
| $existing = $contents !== FALSE ? json_decode($contents, TRUE) : NULL; | |
| if ($existing) { | |
| $data = $existing; | |
| } | |
| } | |
| if (!isset($data[$section])) { | |
| $data[$section] = []; | |
| } | |
| $data[$section] = array_merge($data[$section], $dependencies); | |
| $json = json_encode($data, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES); | |
| if ($json === false) { | |
| throw new \RuntimeException('Failed to encode JSON: ' . json_last_error_msg()); | |
| } | |
| file_put_contents($composer_json, $json); | |
| return $composer_json; | |
| } |
🤖 Prompt for AI Agents
.vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php lines
221-243: after merging dependencies into the composer.json content you must
validate the JSON read/created and the JSON written; ensure json_decode returned
an array (otherwise treat as empty or fail), ensure the target section is an
array before array_merge, check json_encode succeeded (json_last_error() or
JSON_THROW_ON_ERROR) and that file_put_contents returned a non-false result, and
throw or assert with a clear message if any JSON operation fails so tests fail
fast on invalid JSON.
| protected function stubTheme(string $dir): void { | ||
| File::dump($dir . '/scss/_variables.scss'); | ||
| File::dump($dir . '/Gruntfile.js'); | ||
| File::dump($dir . '/package.json', (string) json_encode(['build-dev' => ''], JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES)); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider validating package.json structure.
The stubTheme method creates a package.json with minimal content. Consider adding a more realistic structure for better test coverage.
Apply this enhancement:
protected function stubTheme(string $dir): void {
File::dump($dir . '/scss/_variables.scss');
File::dump($dir . '/Gruntfile.js');
- File::dump($dir . '/package.json', (string) json_encode(['build-dev' => ''], JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES));
+ File::dump($dir . '/package.json', (string) json_encode([
+ 'scripts' => [
+ 'build-dev' => 'grunt build-dev'
+ ]
+ ], JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES));
}📝 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.
| protected function stubTheme(string $dir): void { | |
| File::dump($dir . '/scss/_variables.scss'); | |
| File::dump($dir . '/Gruntfile.js'); | |
| File::dump($dir . '/package.json', (string) json_encode(['build-dev' => ''], JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES)); | |
| } | |
| protected function stubTheme(string $dir): void { | |
| File::dump($dir . '/scss/_variables.scss'); | |
| File::dump($dir . '/Gruntfile.js'); | |
| File::dump($dir . '/package.json', (string) json_encode([ | |
| 'scripts' => [ | |
| 'build-dev' => 'grunt build-dev' | |
| ] | |
| ], JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES)); | |
| } |
🤖 Prompt for AI Agents
.vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php around
lines 261 to 265: the stubTheme creates a minimal package.json which reduces
test fidelity; update it to write a realistic package.json object (e.g. include
name, version, description, main, scripts with "build-dev" and other common
scripts, author, license, and an empty dependencies/devDependencies objects) and
ensure it is json_encoded with JSON_PRETTY_PRINT|JSON_UNESCAPED_SLASHES so tests
receive a properly structured package.json.
| 'theme - discovery - invalid' => [ | ||
| [], | ||
| $expected_defaults, | ||
| function (AbstractPromptManagerTestCase $test): void { | ||
| // No theme files exist and no DRUPAL_THEME in .env - fall back. | ||
| }, | ||
| ], |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix callable signature: before/after closures must accept both ($test, $config)
This closure also only accepts one parameter but is invoked with two. Update the signature to avoid ArgumentCountError.
- function (AbstractPromptManagerTestCase $test): void {
+ function (AbstractPromptManagerTestCase $test, Config $config): void {
// No theme files exist and no DRUPAL_THEME in .env - fall back.
},🤖 Prompt for AI Agents
.vortex/installer/tests/Unit/Handlers/ThemePromptManagerTest.php around lines 52
to 58: the anonymous test closure currently accepts only one parameter but is
invoked with two, causing an ArgumentCountError; update the signature to accept
both parameters (e.g., function (AbstractPromptManagerTestCase $test, array
$config): void) and, if $config is unused, name it to indicate unused (e.g.,
$_config) or reference it minimally to avoid unused variable warnings.
| return [ | ||
| 'timezone - prompt' => [ | ||
| [Timezone::id() => 'America/New_York'], | ||
| [Timezone::id() => 'America/New_York'] + $expected_defaults, | ||
| ], | ||
|
|
||
| 'timezone - discovery' => [ | ||
| [], | ||
| [Timezone::id() => 'Europe/London'] + $expected_installed, | ||
| function (AbstractPromptManagerTestCase $test, Config $config): void { | ||
| $test->stubVortexProject($config); | ||
| $test->stubDotenvValue('TZ', 'Europe/London'); | ||
| }, | ||
| ], |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Optional: add a process() side-effects check
Consider one scenario that exercises Timezone::process() to assert .env and renovate.json are updated (or tokens removed) as expected. You can use the $after callback to inspect tmpDir files.
| 'timezone - discovery - invalid' => [ | ||
| [], | ||
| $expected_defaults, | ||
| function (AbstractPromptManagerTestCase $test): void { | ||
| // No TZ in .env - should fall back to default. | ||
| }, | ||
| ], |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix callable signature: before/after closures must accept both ($test, $config)
AbstractPromptManagerTestCase::testRunPrompts invokes $before($this, $config). The closure here only accepts one parameter, which will trigger an ArgumentCountError at runtime.
Apply this change:
- function (AbstractPromptManagerTestCase $test): void {
+ function (AbstractPromptManagerTestCase $test, Config $config): void {
// No TZ in .env - should fall back to default.
},📝 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.
| 'timezone - discovery - invalid' => [ | |
| [], | |
| $expected_defaults, | |
| function (AbstractPromptManagerTestCase $test): void { | |
| // No TZ in .env - should fall back to default. | |
| }, | |
| ], | |
| 'timezone - discovery - invalid' => [ | |
| [], | |
| $expected_defaults, | |
| function (AbstractPromptManagerTestCase $test, Config $config): void { | |
| // No TZ in .env - should fall back to default. | |
| }, | |
| ], |
🤖 Prompt for AI Agents
.vortex/installer/tests/Unit/Handlers/TimezonePromptManagerTest.php around lines
33 to 39: the provided before/after closure only accepts one parameter but
AbstractPromptManagerTestCase::testRunPrompts calls it with ($this, $config),
causing an ArgumentCountError; update the closure signature to accept both
parameters (e.g., function (AbstractPromptManagerTestCase $test, array $config):
void) and adjust any body references to use the second $config parameter as
needed.
| 'webroot - prompt' => [ | ||
| [ | ||
| HostingProvider::id() => Key::DOWN . Key::DOWN . Key::DOWN . Key::ENTER, | ||
| Webroot::id() => 'my_webroot', | ||
| ], | ||
| [ | ||
| HostingProvider::id() => HostingProvider::OTHER, | ||
| Webroot::id() => 'my_webroot', | ||
| ] + $expected_defaults, | ||
| ], |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Relying on DOWN×3 assumes the default selection is “none”
This keystroke path depends on HostingProvider::default remaining NONE and the options order staying stable. If either changes, the test breaks. Consider a small helper to emit the minimal key sequence to select “Other” from the current default to reduce coupling to option order.
Example helper (pseudo):
private static function selectHostingProviderOther(): string {
// From default (none) to other: DOWN×3
return Key::DOWN . Key::DOWN . Key::DOWN . Key::ENTER;
}🤖 Prompt for AI Agents
.vortex/installer/tests/Unit/Handlers/WebrootPromptManagerTest.php lines 20-29:
the test currently hardcodes Key::DOWN ×3 to reach the "Other" hosting provider
which assumes the default and option order never change; create a small private
helper (e.g. selectHostingProviderOther()) that computes the minimal key
sequence from the current HostingProvider::default() to HostingProvider::OTHER
by determining the index/position of the default and the "Other" option in the
provider list, emitting the needed number of Key::DOWN moves followed by
Key::ENTER (or Key::UP if shorter), and replace the literal DOWN×3 in the test
with a call to that helper so the test adapts if defaults or order change.
| 'webroot - prompt - capitalization' => [ | ||
| [ | ||
| HostingProvider::id() => Key::DOWN . Key::DOWN . Key::DOWN . Key::ENTER, | ||
| Webroot::id() => 'MyWebroot', | ||
| ], | ||
| [ | ||
| HostingProvider::id() => HostingProvider::OTHER, | ||
| Webroot::id() => 'MyWebroot', | ||
| ] + $expected_defaults, | ||
| ], |
There was a problem hiding this comment.
Test expects uppercase webroot to pass, but handler validation allows only lowercase, digits, and underscores
Webroot::validate returns an error when the value contains uppercase letters. The scenario "webroot - prompt - capitalization" should expect a validation error instead of a successful result.
Apply this diff to make the test consistent with the validator:
- 'webroot - prompt - capitalization' => [
- [
- HostingProvider::id() => Key::DOWN . Key::DOWN . Key::DOWN . Key::ENTER,
- Webroot::id() => 'MyWebroot',
- ],
- [
- HostingProvider::id() => HostingProvider::OTHER,
- Webroot::id() => 'MyWebroot',
- ] + $expected_defaults,
- ],
+ 'webroot - prompt - capitalization (invalid)' => [
+ [
+ HostingProvider::id() => Key::DOWN . Key::DOWN . Key::DOWN . Key::ENTER,
+ Webroot::id() => 'MyWebroot',
+ ],
+ 'Please enter a valid webroot name: only lowercase letters, numbers, and underscores are allowed.',
+ ],📝 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.
| 'webroot - prompt - capitalization' => [ | |
| [ | |
| HostingProvider::id() => Key::DOWN . Key::DOWN . Key::DOWN . Key::ENTER, | |
| Webroot::id() => 'MyWebroot', | |
| ], | |
| [ | |
| HostingProvider::id() => HostingProvider::OTHER, | |
| Webroot::id() => 'MyWebroot', | |
| ] + $expected_defaults, | |
| ], | |
| 'webroot - prompt - capitalization (invalid)' => [ | |
| [ | |
| HostingProvider::id() => Key::DOWN . Key::DOWN . Key::DOWN . Key::ENTER, | |
| Webroot::id() => 'MyWebroot', | |
| ], | |
| 'Please enter a valid webroot name: only lowercase letters, numbers, and underscores are allowed.', | |
| ], |
🤖 Prompt for AI Agents
.vortex/installer/tests/Unit/Handlers/WebrootPromptManagerTest.php around lines
31 to 40: the test case "webroot - prompt - capitalization" currently expects a
successful result for input 'MyWebroot' but the Webroot::validate only allows
lowercase, digits and underscores so this input should trigger a validation
error; update the expected result for that test case to assert the validation
error for the Webroot field (i.e., replace the success/expected_defaults branch
with the validation-failure shape used elsewhere in the file, asserting the
specific validation error for Webroot).
AlexSkrypnyk
force-pushed
the
feature/split-tests-classes-poc
branch
from
9b86f37 to
14de1c8
Compare
There was a problem hiding this comment.
Actionable comments posted: 25
♻️ Duplicate comments (76)
.vortex/CLAUDE.md (2)
608-614: Fix MD032: add blank line before the list under “Key Benefits”Same lint issue here; add an empty line between the heading line and the first bullet.
-**Key Benefits**: -- Run all handler tests: `--filter "Handlers\\\\"` +**Key Benefits**: + +- Run all handler tests: `--filter "Handlers\\\\"` - Run specific handler: `--filter "HandlerNameInstallTest"` - Run specific scenarios: `--filter "HandlerNameInstallTest.*scenario_pattern"` - Consistent structure across all handler test classes - Clear separation between test logic (in base class) and test data (in handler classes)
595-600: Fix MD032: add blank line before the list under “Abstract Base Class”Markdownlint flags “lists should be surrounded by blank lines.” Insert an empty line before the first “-” item.
-**Abstract Base Class**: `AbstractInstallTestCase` provides shared test logic for all installer test scenarios, including: -- Common setup and teardown procedures +**Abstract Base Class**: `AbstractInstallTestCase` provides shared test logic for all installer test scenarios, including: + +- Common setup and teardown procedures - Core `testInstall()` method with data provider integration - Fixture management and assertion helpers - Version replacement utilities.vortex/installer/tests/Unit/Handlers/DependencyUpdatesProviderPromptManagerTest.php (2)
27-35: Make GHA discovery symmetrical with CircleCI (assert CI provider + add workflow file)CircleCI case asserts CiProvider and stubs its config; GHA case should do the same to avoid asymmetry.
- 'dependency updates provider - discovery - renovate self-hosted - gha' => [ + 'dependency updates provider - discovery - renovate self-hosted - gha' => [ [], - [DependencyUpdatesProvider::id() => DependencyUpdatesProvider::RENOVATEBOT_CI] + $expected_installed, + [ + CiProvider::id() => CiProvider::GITHUB_ACTIONS, + DependencyUpdatesProvider::id() => DependencyUpdatesProvider::RENOVATEBOT_CI, + ] + $expected_installed, function (AbstractPromptManagerTestCase $test, Config $config): void { $test->stubVortexProject($config); File::dump(static::$sut . '/renovate.json'); - File::dump(static::$sut . '/.github/workflows/update-dependencies.yml'); + File::dump(static::$sut . '/.github/workflows/update-dependencies.yml'); + // Required for CiProvider discovery: + File::dump(static::$sut . '/.github/workflows/build-test-deploy.yml'); }, ],
67-67: Rename dataset for clarity (“invalid” → “not installed (falls back to default)”)This scenario isn’t invalid; it’s “not installed → default.” Rename for intent clarity.
- 'dependency updates provider - discovery - invalid' => [ + 'dependency updates provider - discovery - not installed (falls back to default)' => [.vortex/installer/tests/Unit/Handlers/DocsPromptManagerTest.php (3)
53-59: Fix callable signature: before/after closures must accept ($test, $config)This closure will be invoked with two params; adjust signature to avoid runtime failure.
- function (AbstractPromptManagerTestCase $test): void { + function (AbstractPromptManagerTestCase $test, Config $config): void { // No docs/README.md and not installed - should fall back to default. },
91-97: Fix callable signature: before/after closures must accept ($test, $config)Same issue for onboarding “invalid” case.
- function (AbstractPromptManagerTestCase $test): void { + function (AbstractPromptManagerTestCase $test, Config $config): void { // No docs/onboarding.md and not installed - fall back to default. },
37-43: Optional: assert process() side effects (docs removal) with $after callbackTo catch regressions in process(), add $after callbacks asserting that docs are removed for FALSE cases (project docs dir removed; onboarding file unlinked). You can use $config->get(Config::TMP) to target tmp dir.
'preserve project documentation - discovery - removed' => [ [], [PreserveDocsProject::id() => FALSE] + $expected_installed, - function (AbstractPromptManagerTestCase $test, Config $config): void { + function (AbstractPromptManagerTestCase $test, Config $config): void { $test->stubVortexProject($config); - }, + }, + function (AbstractPromptManagerTestCase $test, Config $config): void { + $tmp = $config->get(\DrevOps\VortexInstaller\Utils\Config::TMP); + $test->assertFalse(is_dir($tmp . '/docs'), 'docs/ directory should be removed when PreserveDocsProject is FALSE.'); + }, ], @@ 'preserve onboarding checklist - discovery - removed' => [ [], [PreserveDocsOnboarding::id() => FALSE] + $expected_installed, - function (AbstractPromptManagerTestCase $test, Config $config): void { + function (AbstractPromptManagerTestCase $test, Config $config): void { $test->stubVortexProject($config); - }, + }, + function (AbstractPromptManagerTestCase $test, Config $config): void { + $tmp = $config->get(\DrevOps\VortexInstaller\Utils\Config::TMP); + $test->assertFalse(is_file($tmp . '/docs/onboarding.md'), 'docs/onboarding.md should be removed when PreserveDocsOnboarding is FALSE.'); + }, ],Also applies to: 75-81
.vortex/installer/tests/Unit/Handlers/ThemePromptManagerTest.php (2)
25-33: Reduce brittleness: avoid asserting full validation message stringThe test hard-codes the entire message from Theme::validate(); minor copy edits will break it. Prefer asserting via a constant or matching a stable substring.
Proposed approach:
- Extract a public constant in Theme for the validation message and reference it in tests.
Theme.php (handler):
+ public const ERROR_INVALID_MACHINE_NAME = 'Please enter a valid theme machine name: only lowercase letters, numbers, and underscores are allowed.'; @@ - return fn($v): ?string => !empty($v) && Converter::machine($v) !== $v ? 'Please enter a valid theme machine name: only lowercase letters, numbers, and underscores are allowed.' : NULL; + return fn($v): ?string => !empty($v) && Converter::machine($v) !== $v ? self::ERROR_INVALID_MACHINE_NAME : NULL;ThemePromptManagerTest.php (this file):
- 'Please enter a valid theme machine name: only lowercase letters, numbers, and underscores are allowed.', + \DrevOps\VortexInstaller\Prompts\Handlers\Theme::ERROR_INVALID_MACHINE_NAME,
52-58: Fix callable signature: before/after closures must accept ($test, $config)This closure is invoked with two arguments by the harness.
- function (AbstractPromptManagerTestCase $test): void { + function (AbstractPromptManagerTestCase $test, Config $config): void { // No theme files exist and no DRUPAL_THEME in .env - fall back. },.vortex/installer/tests/Unit/Handlers/ModulePrefixPromptManagerTest.php (1)
77-79: Fix before-hook closure arity; also make “invalid discovery” explicit
- The
$beforeclosure takes a single parameter, butAbstractPromptManagerTestCase::testRunPrompts()invokes it with two ($this, $config). This will raise ArgumentCountError.- While here, make the scenario more explicit by creating non-matching module files so it’s clear we’re testing “no *_base/_core present”.
- function (AbstractPromptManagerTestCase $test): void { - // No *_base or *_core modules exist - should fall back to default. - }, + function (AbstractPromptManagerTestCase $test, Config $config): void { + // Create modules that do NOT match *_base/_core to ensure discovery falls back to default. + File::dump(static::$sut . '/web/modules/custom/some_module/some_module.info'); + File::dump(static::$sut . '/web/modules/custom/another/another.info'); + },.vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (3)
214-219: Harden JSON stubbing: check encoding and writesAdd error handling for JSON encoding and file writes so tests fail fast with actionable messages.
protected function stubComposerJsonValue(string $name, mixed $value): string { $composer_json = static::$sut . DIRECTORY_SEPARATOR . 'composer.json'; - file_put_contents($composer_json, json_encode([$name => $value], JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES)); + $json = json_encode([$name => $value], JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES); + if ($json === false) { + throw new \RuntimeException('Failed to encode JSON: ' . json_last_error_msg()); + } + if (file_put_contents($composer_json, $json) === false) { + throw new \RuntimeException(sprintf('Failed to write "%s".', $composer_json)); + } return $composer_json; }
221-243: Validate merged composer.json and file writesApply the same hardening to dependency merging and ensure the target section is an array.
protected function stubComposerJsonDependencies(array $dependencies, bool $is_dev = FALSE): string { $composer_json = static::$sut . DIRECTORY_SEPARATOR . 'composer.json'; $section = $is_dev ? 'require-dev' : 'require'; $data = []; if (file_exists($composer_json)) { $contents = file_get_contents($composer_json); $existing = $contents !== FALSE ? json_decode($contents, TRUE) : NULL; if ($existing) { $data = $existing; } } if (!isset($data[$section])) { $data[$section] = []; } $data[$section] = array_merge($data[$section], $dependencies); - file_put_contents($composer_json, json_encode($data, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES)); + $json = json_encode($data, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES); + if ($json === false) { + throw new \RuntimeException('Failed to encode JSON: ' . json_last_error_msg()); + } + if (file_put_contents($composer_json, $json) === false) { + throw new \RuntimeException(sprintf('Failed to write "%s".', $composer_json)); + } return $composer_json; }
261-265: Make theme stub package.json more realisticA slightly richer structure improves fidelity when handlers or tests inspect scripts.
protected function stubTheme(string $dir): void { File::dump($dir . '/scss/_variables.scss'); File::dump($dir . '/Gruntfile.js'); - File::dump($dir . '/package.json', (string) json_encode(['build-dev' => ''], JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES)); + File::dump($dir . '/package.json', (string) json_encode([ + 'name' => 'theme', + 'version' => '0.0.0', + 'scripts' => [ + 'build-dev' => 'grunt build-dev' + ], + 'dependencies' => new \stdClass(), + 'devDependencies' => new \stdClass(), + ], JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES)); }.vortex/installer/tests/Unit/Handlers/DomainPromptManagerTest.php (4)
7-11: Import Config for typed $before closuresLater closures should accept
Config $config; bring the symbol into scope.use DrevOps\VortexInstaller\Prompts\Handlers\Domain; +use DrevOps\VortexInstaller\Utils\Config; use PHPUnit\Framework\Attributes\CoversClass;
47-53: Fix before-hook closure arity
testRunPrompts()passes two args to$before; update the signature accordingly.- function (AbstractPromptManagerTestCase $test): void { + function (AbstractPromptManagerTestCase $test, Config $config): void { $test->stubDotenvValue('DRUPAL_STAGE_FILE_PROXY_ORIGIN', 'https://discovered-project-dotenv.com'); },
55-61: Fix before-hook closure arity (www case)Same issue for the “www” discovery case.
- function (AbstractPromptManagerTestCase $test): void { + function (AbstractPromptManagerTestCase $test, Config $config): void { $test->stubDotenvValue('DRUPAL_STAGE_FILE_PROXY_ORIGIN', 'https://www.discovered-project-dotenv.com'); },
63-69: Fix before-hook closure arity (invalid discovery)Same arity issue here.
- function (AbstractPromptManagerTestCase $test): void { + function (AbstractPromptManagerTestCase $test, Config $config): void { $test->stubDotenvValue('DRUPAL_STAGE_FILE_PROXY_ORIGIN', ''); },.vortex/installer/tests/Unit/Handlers/ProvisionTypePromptManagerTest.php (1)
41-47: Fix before-hook closure arity and rename case for clarity
- Closure must accept
($this, $config)to match caller.- Name better reflects scenario: falling back to defaults when env var absent.
- 'provision type - discovery - invalid' => [ + 'provision type - discovery - fallback to default' => [ [], $expected_defaults, - function (AbstractPromptManagerTestCase $test): void { + function (AbstractPromptManagerTestCase $test, Config $config): void { // No VORTEX_PROVISION_TYPE in .env - should fall back to default. },.vortex/installer/tests/Unit/Handlers/TimezonePromptManagerTest.php (2)
36-38: Fix callable signature: before/after closures must accept both ($test, $config)This closure is missing the Config parameter and will throw ArgumentCountError when invoked by the harness.
- function (AbstractPromptManagerTestCase $test): void { + function (AbstractPromptManagerTestCase $test, Config $config): void { // No TZ in .env - should fall back to default. },
18-31: Optional: assert process() side-effects (env + renovate) or add a functional testConsider one scenario that exercises Timezone::process() to verify:
- .env has TZ replaced; and
- renovate.json timezone is updated (or tokens removed).
If PromptManager::runPrompts doesn’t invoke process(), prefer adding/expanding a Functional test for Timezone to check these file mutations.
I can draft the after-callback assertions or a focused functional test if you confirm where process() is executed in your flow.
.vortex/installer/tests/Unit/Handlers/DeployTypePromptManagerTest.php (2)
44-46: Fix callable signature: before/after closures must accept both ($test, $config)This setup closure is missing the Config parameter; the harness calls it with two arguments.
- function (AbstractPromptManagerTestCase $test): void { + function (AbstractPromptManagerTestCase $test, Config $config): void { // No VORTEX_DEPLOY_TYPES in .env - should fall back to default. },
16-49: Add hosting-aware defaults scenarios (Lagoon/Acquia) to validate DeployType::default()To prevent regressions in host-coupled defaults, add cases where HostingProvider is preset:
@@ class DeployTypePromptManagerTest extends AbstractPromptManagerTestCase { - public static function dataProviderRunPrompts(): array { + public static function dataProviderRunPrompts(): array { $expected_defaults = static::getExpectedDefaults(); @@ return [ + 'deploy type - default when hosting = Lagoon' => [ + [\DrevOps\VortexInstaller\Prompts\Handlers\HostingProvider::id() => \DrevOps\VortexInstaller\Prompts\Handlers\HostingProvider::LAGOON], + [DeployType::id() => [DeployType::LAGOON]] + $expected_defaults, + ], + 'deploy type - default when hosting = Acquia' => [ + [\DrevOps\VortexInstaller\Prompts\Handlers\HostingProvider::id() => \DrevOps\VortexInstaller\Prompts\Handlers\HostingProvider::ACQUIA], + [DeployType::id() => [DeployType::ARTIFACT]] + $expected_defaults, + ],If you prefer imports, add:
+use DrevOps\VortexInstaller\Prompts\Handlers\HostingProvider;and replace FQCNs above accordingly.
To ensure all such dependencies are covered, run:
#!/bin/bash # Find handlers whose default() depends on HostingProvider to seed additional cases. rg -nP 'function\s+default\([\s\S]*?\)\s*:\s*[^\{]+\{[\s\S]*?HostingProvider::' .vortex/installer/src/Prompts/Handlers --type=php -C2.vortex/installer/tests/Unit/Handlers/HostingProviderPromptManagerTest.php (3)
69-71: Fix callable signature: before/after closures must accept both ($test, $config)The closure for the “invalid” scenario lacks the Config parameter; the harness will pass two arguments.
- function (AbstractPromptManagerTestCase $test): void { + function (AbstractPromptManagerTestCase $test, Config $config): void { // No hooks, .lagoon.yml, or ACQUIA env var - fall back to default. },
54-64: Make Webroot expectation explicit for Lagoon discoveryWebroot::default() returns WEB when HostingProvider is LAGOON. Assert this explicitly to document the coupling.
'hosting provider - discovery - Lagoon' => [ [], [ HostingProvider::id() => HostingProvider::LAGOON, DeployType::id() => [DeployType::LAGOON], + Webroot::id() => Webroot::WEB, DatabaseDownloadSource::id() => DatabaseDownloadSource::LAGOON, ] + $expected_defaults,
22-74: Add precedence scenario: Acquia beats Lagoon when both indicators are presentdiscover() checks Acquia first, then Lagoon. Add a test where hooks/ and .lagoon.yml both exist and assert ACQUIA.
@@ public static function dataProviderRunPrompts(): array { return [ + 'hosting provider - discovery - both present prefers Acquia' => [ + [], + [ + HostingProvider::id() => HostingProvider::ACQUIA, + Webroot::id() => Webroot::DOCROOT, + DeployType::id() => [DeployType::ARTIFACT], + DatabaseDownloadSource::id() => DatabaseDownloadSource::ACQUIA, + ] + $expected_defaults, + function (AbstractPromptManagerTestCase $test, Config $config): void { + File::dump(static::$sut . '/hooks/somehook'); + File::dump(static::$sut . '/.lagoon.yml'); + }, + ],.vortex/installer/tests/Unit/Handlers/DatabaseDownloadSourcePromptManagerTest.php (3)
26-29: Fix callable signature: before/after closures must accept both ($test, $config)- function (AbstractPromptManagerTestCase $test): void { + function (AbstractPromptManagerTestCase $test, Config $config): void { $test->stubDotenvValue('VORTEX_DB_DOWNLOAD_SOURCE', DatabaseDownloadSource::FTP); },
34-36: Fix callable signature: before/after closures must accept both ($test, $config)- function (AbstractPromptManagerTestCase $test): void { + function (AbstractPromptManagerTestCase $test, Config $config): void { $test->stubDotenvValue('VORTEX_DB_DOWNLOAD_SOURCE', 'invalid_source'); },
14-21: Add a test for skipping the DatabaseDownloadSource prompt when provision type is PROFILETo ensure that
DatabaseDownloadSource::shouldRun()correctly skips the prompt forProvisionType::PROFILE, please add a data-provider scenario inDatabaseDownloadSourcePromptManagerTest.phpcovering that case.• File:
.vortex/installer/tests/Unit/Handlers/DatabaseDownloadSourcePromptManagerTest.php
• Method:dataProviderRunPrompts()Proposed diff:
@@ public static function dataProviderRunPrompts(): array { $expected_defaults = static::getExpectedDefaults(); + // When provision type is PROFILE, DatabaseDownloadSource::shouldRun() is false => prompt skipped + $expected_skipped = $expected_defaults; + unset($expected_skipped[\DrevOps\VortexInstaller\Prompts\Handlers\DatabaseDownloadSource::id()]); @@ return [ + 'database download source – skipped for provision=PROFILE' => [ + [\DrevOps\VortexInstaller\Prompts\Handlers\ProvisionType::id() => \DrevOps\VortexInstaller\Prompts\Handlers\ProvisionType::PROFILE], + $expected_skipped, + ], 'database download source - prompt' => [ [DatabaseDownloadSource::id() => Key::ENTER], [DatabaseDownloadSource::id() => DatabaseDownloadSource::URL] + $expected_defaults, ],If you’d rather use imports at the top, add:
+use DrevOps\VortexInstaller\Prompts\Handlers\ProvisionType;Optional: If the TUI helper in
AbstractPromptManagerTestCasedoesn’t accept direct-value answers for radio prompts, you can overridedefaultTuiAnswers()for this scenario to supply the PROFILE value. Let me know if you’d like a snippet for that override..vortex/installer/tests/Unit/Handlers/CodeProviderPromptManagerTest.php (2)
11-11: Avoid external dependency in unit tests: replace Git::init() with a stubbed .git dirShelling out to Git in unit tests is brittle and slow. You only need the
.gitdirectory to exist for discovery. Create it directly and drop the Git import.--- a/.vortex/installer/tests/Unit/Handlers/CodeProviderPromptManagerTest.php +++ b/.vortex/installer/tests/Unit/Handlers/CodeProviderPromptManagerTest.php @@ -use DrevOps\VortexInstaller\Utils\Git; @@ - function (AbstractPromptManagerTestCase $test, Config $config): void { + function (AbstractPromptManagerTestCase $test, Config $config): void { $test->stubVortexProject($config); - Git::init(static::$sut); + File::mkdir(static::$sut . '/.git'); },Also applies to: 49-52
55-61: Fix: $before closure signature mismatch will throw ArgumentCountError
AbstractPromptManagerTestCase::testRunPrompts()always calls$before($this, $config). This closure accepts only one arg and will fail.- function (AbstractPromptManagerTestCase $test): void { + function (AbstractPromptManagerTestCase $test, Config $config): void { // No .github directory and no .git directory - fall back to default. },.vortex/installer/tests/Unit/Handlers/CiProviderPromptManagerTest.php (1)
52-58: Fix: $before closure signature mismatch will throw ArgumentCountErrorConsistent with the base test runner, the closure must accept both
$testand$config.- function (AbstractPromptManagerTestCase $test): void { + function (AbstractPromptManagerTestCase $test, Config $config): void { // No CI files and not installed - should fall back to default. },.vortex/installer/tests/Unit/Handlers/WebrootPromptManagerTest.php (2)
31-40: Test expects invalid input to pass; handler validation rejects uppercase
Webroot::validate()only allows lowercase letters, digits, and underscores. The "capitalization" case should assert a validation error, not success.- 'webroot - prompt - capitalization' => [ + 'webroot - prompt - capitalization (invalid)' => [ [ - HostingProvider::id() => Key::DOWN . Key::DOWN . Key::DOWN . Key::ENTER, + HostingProvider::id() => Key::DOWN . Key::DOWN . Key::DOWN . Key::ENTER, Webroot::id() => 'MyWebroot', ], - [ - HostingProvider::id() => HostingProvider::OTHER, - Webroot::id() => 'MyWebroot', - ] + $expected_defaults, + 'Please enter a valid webroot name: only lowercase letters, numbers, and underscores are allowed.', ],
22-22: Reduce brittleness: avoid hardcoding DOWN×3 to select HostingProvider::OTHERThis couples the test to option order and default. Add a small helper to emit the minimal keystrokes from the current default to “Other,” then use it in datasets.
--- a/.vortex/installer/tests/Unit/Handlers/WebrootPromptManagerTest.php +++ b/.vortex/installer/tests/Unit/Handlers/WebrootPromptManagerTest.php @@ class WebrootPromptManagerTest extends AbstractPromptManagerTestCase { public static function dataProviderRunPrompts(): array { @@ - HostingProvider::id() => Key::DOWN . Key::DOWN . Key::DOWN . Key::ENTER, + HostingProvider::id() => self::selectHostingProviderOther(), @@ - HostingProvider::id() => Key::DOWN . Key::DOWN . Key::DOWN . Key::ENTER, + HostingProvider::id() => self::selectHostingProviderOther(), @@ - HostingProvider::id() => Key::DOWN . Key::DOWN . Key::DOWN . Key::ENTER, + HostingProvider::id() => self::selectHostingProviderOther(), @@ } + + /** + * Compute keystrokes to select HostingProvider::OTHER from the current default. + */ + private static function selectHostingProviderOther(): string { + // Current default is NONE; options order: ACQUIA, LAGOON, OTHER, NONE. + // From NONE, three DOWNs wraps to OTHER in current UI. + // If defaults/options change, adjust this helper in one place. + return Key::DOWN . Key::DOWN . Key::DOWN . Key::ENTER; + }Also applies to: 33-33, 44-44
.vortex/installer/tests/Unit/Handlers/ServicesPromptManagerTest.php (1)
30-33: DRY the docker-compose scaffolding with a tiny helperWriting docker-compose.yml inline in every case is repetitive. Add a
writeCompose(array $services)helper and reuse it across scenarios.--- a/.vortex/installer/tests/Unit/Handlers/ServicesPromptManagerTest.php +++ b/.vortex/installer/tests/Unit/Handlers/ServicesPromptManagerTest.php @@ class ServicesPromptManagerTest extends AbstractPromptManagerTestCase { public static function dataProviderRunPrompts(): array { @@ - $test->stubVortexProject($config); - File::dump(static::$sut . '/docker-compose.yml', Yaml::dump(['services' => [Services::SOLR => []]])); + $test->stubVortexProject($config); + self::writeCompose([Services::SOLR => []]); @@ - $test->stubVortexProject($config); - File::dump(static::$sut . '/docker-compose.yml', Yaml::dump(['services' => [Services::VALKEY => []]])); + $test->stubVortexProject($config); + self::writeCompose([Services::VALKEY => []]); @@ - $test->stubVortexProject($config); - File::dump(static::$sut . '/docker-compose.yml', Yaml::dump(['services' => [Services::CLAMAV => []]])); + $test->stubVortexProject($config); + self::writeCompose([Services::CLAMAV => []]); @@ - $test->stubVortexProject($config); - File::dump(static::$sut . '/docker-compose.yml', Yaml::dump(['services' => [Services::CLAMAV => [], Services::VALKEY => [], Services::SOLR => []]])); + $test->stubVortexProject($config); + self::writeCompose([Services::CLAMAV => [], Services::VALKEY => [], Services::SOLR => []]); @@ - $test->stubVortexProject($config); - File::dump(static::$sut . '/docker-compose.yml', Yaml::dump(['services' => ['other_service' => []]])); + $test->stubVortexProject($config); + self::writeCompose(['other_service' => []]); @@ } + + private static function writeCompose(array $services): void { + File::dump(static::$sut . '/docker-compose.yml', Yaml::dump(['services' => $services])); + }Also applies to: 40-42, 49-51, 61-62, 69-71
.vortex/installer/tests/Unit/Handlers/NamesPromptManagerTest.php (1)
161-164: Minor: trailing space in dataset labelThe key “org machine name - invalid ” has a trailing space. Clean it up for sanity when filtering tests by name.
- 'org machine name - invalid ' => [ + 'org machine name - invalid' => [.vortex/installer/tests/Unit/Handlers/ProfilePromptManagerTest.php (3)
26-29: Fix expected mapping for “custom” selection to match handler behaviorProfile::process() does not mutate the responses array; “custom” stays under Profile::id() and the actual name lives under 'profile_custom'. Adjust the expectation accordingly.
'profile - prompt - custom' => [ [Profile::id() => Key::DOWN . Key::DOWN . Key::DOWN . Key::ENTER . 'myprofile'], - [Profile::id() => 'myprofile'] + $expected_defaults, + [ + Profile::id() => Profile::CUSTOM, + 'profile_custom' => 'myprofile', + ] + $expected_defaults, ],
45-51: Align discovery expectation for non‑Vortex projectsdiscover() returns Profile::CUSTOM for unknown names; the discovered name should be placed under 'profile_custom'. Update the expected responses.
'profile - discovery - non-Vortex project' => [ [], - [Profile::id() => 'discovered_profile'] + $expected_defaults, + [ + Profile::id() => Profile::CUSTOM, + 'profile_custom' => 'discovered_profile', + ] + $expected_defaults, function (AbstractPromptManagerTestCase $test, Config $config): void { File::dump(static::$sut . '/web/profiles/discovered_profile/discovered_profile.info'); }, ],
56-58: Before-callback signature mismatch will throw ArgumentCountErrortestRunPrompts() calls $before($this, $config). This closure only accepts one parameter.
- function (AbstractPromptManagerTestCase $test): void { + function (AbstractPromptManagerTestCase $test, Config $config): void { // No .env file and no profile info files - fall back to default. },.vortex/installer/tests/Unit/Handlers/AiCodeInstructionsPromptManagerTest.php (3)
22-24: Prefer harness constant over raw Key::ENTERUse the shared TUI_DEFAULT to avoid coupling to Laravel\Prompts internals.
- [AiCodeInstructions::id() => Key::ENTER], + [AiCodeInstructions::id() => static::TUI_DEFAULT],
51-57: Before-callback signature mismatch will throw ArgumentCountErrorThe closure must accept both ($test, $config).
- function (AbstractPromptManagerTestCase $test): void { + function (AbstractPromptManagerTestCase $test, Config $config): void { // No CLAUDE.md and not installed - should fall back to default. },
20-34: Optional: add an explicit “prompt selects CLAUDE” scenarioThis complements discovery and hardens prompt option mapping coverage.
return [ 'ai instructions - prompt' => [ [AiCodeInstructions::id() => static::TUI_DEFAULT], [AiCodeInstructions::id() => AiCodeInstructions::NONE] + $expected_defaults, ], + + // Explicit selection of CLAUDE from the prompt. + 'ai instructions - prompt selects CLAUDE' => [ + // Default is NONE; one UP should select CLAUDE given options order [CLAUDE, NONE]. + [AiCodeInstructions::id() => Key::UP . Key::ENTER], + [AiCodeInstructions::id() => AiCodeInstructions::CLAUDE] + $expected_defaults, + ],.vortex/installer/tests/Functional/Handlers/ToolsInstallTest.php (4)
270-283: Decide whether to remove the “test:” aggregator when no BE tests are selectedIf “test:” aggregates only backend tests in your template, assert its removal as well. If not, keep as-is.
static::cw(fn(FunctionalTestCase $test) => $test->assertSutNotContains([ 'phpunit', 'ahoy test-unit', 'ahoy test-kernel', 'ahoy test-functional', 'behat', 'behat/behat', 'drupal/drupal-extension', 'ahoy test-bdd', 'gherkinlint', 'gherkin-lint', 'gherkin', + # If "test:" aggregates only BE tests, include it: + # 'test:', ])),To confirm intent, search the template’s .ahoy.yml:
#!/bin/bash # Find the "test:" target and inspect its dependencies. rg -nP -C3 '^\s*test:\s*$' . | sed -n '1,200p'
226-239: Strengthen group removal checks for backend lintingAlso assert that group-level Ahoy targets are removed when no BE lint tools are selected.
static::cw(fn(FunctionalTestCase $test) => $test->assertSutNotContains([ 'phpcs', 'phpcbf', 'dealerdirect/phpcodesniffer-composer-installer', 'drupal/coder', 'squizlabs/php_codesniffer', 'phpmd', 'phpmd/phpmd', 'phpstan', 'phpstan/phpstan', 'mglaman/phpstan-drupal', 'rector', 'rector/rector', + '/\blint-be:/', + '/\blint-be-fix:/', ])),
248-261: Mirror lint group assertions in CircleCI variantKeep both CI paths equally strict.
static::cw(fn(FunctionalTestCase $test) => $test->assertSutNotContains([ 'phpcs', 'phpcbf', 'dealerdirect/phpcodesniffer-composer-installer', 'drupal/coder', 'squizlabs/php_codesniffer', 'phpmd', 'phpmd/phpmd', 'phpstan', 'phpstan/phpstan', 'mglaman/phpstan-drupal', 'rector', 'rector/rector', + '/\blint-be:/', + '/\blint-be-fix:/', ])),
53-58: Reduce duplication with helpers for tool selection and CIYou recompute the tools list and set env values in many cases. Add small helpers to improve clarity and maintainability, then use them across scenarios.
class ToolsInstallTest extends AbstractInstallTestCase { public static function dataProviderInstall(): array { return [ @@ - 'tools, no phpcs' => [ - static::cw(function (): void { - $tools = array_keys(Tools::getToolDefinitions('tools')); - Env::put(PromptManager::makeEnvName(Tools::id()), Converter::toList(array_diff($tools, [Tools::PHPCS]))); - Env::put(PromptManager::makeEnvName(CiProvider::id()), CiProvider::GITHUB_ACTIONS); - }), + 'tools, no phpcs' => [ + static::cw(function (): void { + self::putTools(array_diff(self::allTools(), [Tools::PHPCS]), CiProvider::GITHUB_ACTIONS); + }), static::cw(fn(FunctionalTestCase $test) => $test->assertSutNotContains([ 'phpcs', 'phpcbf', 'dealerdirect/phpcodesniffer-composer-installer', 'drupal/coder', 'squizlabs/php_codesniffer', ])), ], @@ } + /** + * Return all tool IDs. + * + * @return array<int,string> + */ + private static function allTools(): array { + return array_keys(Tools::getToolDefinitions('tools')); + } + + /** + * Put selected tools and optional CI into the environment. + */ + private static function putTools(array $keep, ?string $ci = null): void { + Env::put(PromptManager::makeEnvName(Tools::id()), Converter::toList($keep)); + if ($ci !== null) { + Env::put(PromptManager::makeEnvName(CiProvider::id()), $ci); + } + } + }Apply similar replacements in other scenarios that build $tools and set both env vars.
.vortex/installer/tests/Functional/Handlers/DocsInstallTest.php (1)
7-15: Add assertions to make intent explicitAdd lightweight checks so disabled flags actually remove docs; helps catch regressions in handler processing.
use DrevOps\VortexInstaller\Prompts\PromptManager; use DrevOps\VortexInstaller\Utils\Env; use PHPUnit\Framework\Attributes\CoversClass; +use DrevOps\VortexInstaller\Tests\Functional\FunctionalTestCase; @@ public static function dataProviderInstall(): array { return [ 'preserve docs project, enabled' => [ static::cw(fn() => Env::put(PromptManager::makeEnvName(PreserveDocsProject::id()), Env::TRUE)), + // Optional: assert project docs remain. + // static::cw(fn(FunctionalTestCase $test) => $test->assertFileExists(static::$sut . '/docs/README.md')), ], @@ 'preserve docs project, disabled' => [ static::cw(fn() => Env::put(PromptManager::makeEnvName(PreserveDocsProject::id()), Env::FALSE)), + static::cw(fn(FunctionalTestCase $test) => $test->assertFileDoesNotExist(static::$sut . '/docs/README.md')), ], @@ 'preserve docs onboarding, enabled' => [ static::cw(fn() => Env::put(PromptManager::makeEnvName(PreserveDocsOnboarding::id()), Env::TRUE)), + // Optional: assert onboarding doc remains. + // static::cw(fn(FunctionalTestCase $test) => $test->assertFileExists(static::$sut . '/docs/onboarding.md')), ], @@ 'preserve docs onboarding, disabled' => [ static::cw(fn() => Env::put(PromptManager::makeEnvName(PreserveDocsOnboarding::id()), Env::FALSE)), + static::cw(fn(FunctionalTestCase $test) => $test->assertFileDoesNotExist(static::$sut . '/docs/onboarding.md')), ],.vortex/installer/tests/Functional/Handlers/NamesInstallTest.php (2)
5-5: Namespace corrected — resolved prior inconsistencyThe namespace now matches the project structure and imports.
18-19: Add coverage annotations for all handlers seeded by this testThis class seeds responses for multiple handlers but attributes coverage only to Name. Attribute coverage to all actually exercised handlers to keep attribution accurate.
Apply this diff:
#[CoversClass(Name::class)] +#[CoversClass(MachineName::class)] +#[CoversClass(Org::class)] +#[CoversClass(OrgMachineName::class)] +#[CoversClass(Domain::class)] +#[CoversClass(ModulePrefix::class)] +#[CoversClass(Theme::class)] class NamesInstallTest extends AbstractInstallTestCase {.vortex/installer/tests/Functional/Handlers/DatabaseDownloadSourceInstallTest.php (1)
18-44: Cover the “none” path and assert .env mutations for container registry
- Add a dataset for NONE to keep parity with available options.
- For container registry, assert .env keys written by DatabaseDownloadSource and DatabaseImage to guard against regressions.
Apply this diff:
public static function dataProviderInstall(): array { return [ 'db download source, url' => [ static::cw(fn() => Env::put(PromptManager::makeEnvName(DatabaseDownloadSource::id()), DatabaseDownloadSource::URL)), ], 'db download source, ftp' => [ static::cw(fn() => Env::put(PromptManager::makeEnvName(DatabaseDownloadSource::id()), DatabaseDownloadSource::FTP)), ], 'db download source, acquia' => [ static::cw(fn() => Env::put(PromptManager::makeEnvName(DatabaseDownloadSource::id()), DatabaseDownloadSource::ACQUIA)), ], 'db download source, lagoon' => [ static::cw(fn() => Env::put(PromptManager::makeEnvName(DatabaseDownloadSource::id()), DatabaseDownloadSource::LAGOON)), ], + 'db download source, none' => [ + static::cw(fn() => Env::put(PromptManager::makeEnvName(DatabaseDownloadSource::id()), DatabaseDownloadSource::NONE)), + ], + 'db download source, container_registry' => [ static::cw(function (): void { Env::put(PromptManager::makeEnvName(DatabaseDownloadSource::id()), DatabaseDownloadSource::CONTAINER_REGISTRY); Env::put(PromptManager::makeEnvName(DatabaseImage::id()), 'the_empire/star_wars:latest'); Env::put(PromptManager::makeEnvName(AiCodeInstructions::id()), AiCodeInstructions::CLAUDE); }), + static::cw(function (\DrevOps\VortexInstaller\Tests\Functional\FunctionalTestCase $test): void { + $env = static::$sut . '/.env'; + $test->assertFileContainsString('VORTEX_DB_DOWNLOAD_SOURCE=container_registry', $env); + $test->assertFileContainsString('VORTEX_DB_IMAGE=the_empire/star_wars:latest', $env); + }), ], ]; }.vortex/installer/tests/Functional/Handlers/AiCodeInstructionsInstallTest.php (1)
15-24: Assert presence/absence of CLAUDE.md for both pathsMake outcomes explicit:
- CLAUDE selected => CLAUDE.md present.
- NONE selected => CLAUDE.md absent.
Apply this diff:
public static function dataProviderInstall(): array { return [ 'ai instructions, claude' => [ - static::cw(fn() => Env::put(PromptManager::makeEnvName(AiCodeInstructions::id()), AiCodeInstructions::CLAUDE)), + static::cw(fn() => Env::put(PromptManager::makeEnvName(AiCodeInstructions::id()), AiCodeInstructions::CLAUDE)), + static::cw(function (\DrevOps\VortexInstaller\Tests\Functional\FunctionalTestCase $t): void { + $t->assertSutContains('CLAUDE.md'); + }), ], 'ai instructions, none' => [ - static::cw(fn() => Env::put(PromptManager::makeEnvName(AiCodeInstructions::id()), AiCodeInstructions::NONE)), + static::cw(fn() => Env::put(PromptManager::makeEnvName(AiCodeInstructions::id()), AiCodeInstructions::NONE)), + // Ensure CLAUDE.md is not left in the SUT. + static::cw(fn(\DrevOps\VortexInstaller\Tests\Functional\FunctionalTestCase $t) => $t->assertSutNotContains('CLAUDE.md')), ], ]; }.vortex/installer/tests/Functional/Handlers/BaselineInstallTest.php (2)
38-67: Trim CoversClass to what this test actually exercisesBaseline should not attribute coverage to per-handler classes that have their own focused tests. Keep only essentials touched by the baseline flow (e.g., Config, Downloader, Git, Tui) and drop the rest to avoid inflated metrics.
Apply this diff:
#[CoversClass(AssignAuthorPr::class)] -#[CoversClass(CiProvider::class)] -#[CoversClass(CodeProvider::class)] -#[CoversClass(DatabaseDownloadSource::class)] -#[CoversClass(DatabaseImage::class)] -#[CoversClass(DependencyUpdatesProvider::class)] -#[CoversClass(DeployType::class)] -#[CoversClass(Domain::class)] -#[CoversClass(HostingProvider::class)] -#[CoversClass(Internal::class)] -#[CoversClass(LabelMergeConflictsPr::class)] -#[CoversClass(MachineName::class)] -#[CoversClass(ModulePrefix::class)] -#[CoversClass(Name::class)] -#[CoversClass(Org::class)] -#[CoversClass(OrgMachineName::class)] -#[CoversClass(PreserveDocsOnboarding::class)] -#[CoversClass(PreserveDocsProject::class)] -#[CoversClass(Profile::class)] -#[CoversClass(ProvisionType::class)] -#[CoversClass(Services::class)] -#[CoversClass(Theme::class)] -#[CoversClass(Timezone::class)] -#[CoversClass(Webroot::class)] -#[CoversClass(PromptManager::class)] #[CoversClass(Downloader::class)] #[CoversClass(Config::class)] #[CoversClass(Git::class)] #[CoversClass(Tui::class)] class BaselineInstallTest extends AbstractInstallTestCase {Follow-up: if you accept this, remove now-unused corresponding use statements at the top of the file.
77-81: Remove duplicate datasetThe 'non-interactive' dataset repeats the baseline case verbatim and adds no signal.
Apply this diff:
- 'non-interactive' => [ - NULL, - NULL, - ['Welcome to the Vortex non-interactive installer'], - ],.vortex/installer/tests/Functional/Handlers/CodeProviderInstallTest.php (1)
21-23: Reduce brittleness: assert file presence instead of specific template textAsserting on a specific phrase makes the test flaky when the template copy changes. It’s sufficient to verify the file got renamed into place.
- $test->assertFileContainsString('Checklist before requesting a review', static::$sut . '/.github/PULL_REQUEST_TEMPLATE.md'); + $test->assertFileExists(static::$sut . '/.github/PULL_REQUEST_TEMPLATE.md');.vortex/installer/tests/Functional/Handlers/DependencyUpdatesProviderInstallTest.php (4)
18-21: Add post-install assertions for Renovate CI (GHA) flowValidate that renovate.json and the GHA workflow are created to catch regressions in file/token handling.
'deps updates provider, ci, gha' => [ static::cw(fn() => Env::put(PromptManager::makeEnvName(DependencyUpdatesProvider::id()), DependencyUpdatesProvider::RENOVATEBOT_CI)), + static::cw(function (\DrevOps\VortexInstaller\Tests\Functional\FunctionalTestCase $test): void { + $test->assertFileExists(static::$sut . '/renovate.json'); + $test->assertFileExists(static::$sut . '/.github/workflows/update-dependencies.yml'); + }), ],
22-27: Add post-install assertions for Renovate CI (CircleCI) flowFor CircleCI, the GHA workflow should not exist; renovate.json should exist; CircleCI config should reference the job.
'deps updates provider, ci, circleci' => [ static::cw(function (): void { Env::put(PromptManager::makeEnvName(DependencyUpdatesProvider::id()), DependencyUpdatesProvider::RENOVATEBOT_CI); Env::put(PromptManager::makeEnvName(CiProvider::id()), CiProvider::CIRCLECI); }), + static::cw(function (\DrevOps\VortexInstaller\Tests\Functional\FunctionalTestCase $test): void { + $test->assertFileExists(static::$sut . '/renovate.json'); + $test->assertFileDoesNotExist(static::$sut . '/.github/workflows/update-dependencies.yml'); + $test->assertFileContainsString('update-dependencies', static::$sut . '/.circleci/config.yml'); + }), ],
29-31: Add post-install assertions for Renovate GitHub App flowThe app flow should keep configuration but remove the CI workflow.
'deps updates provider, app' => [ static::cw(fn() => Env::put(PromptManager::makeEnvName(DependencyUpdatesProvider::id()), DependencyUpdatesProvider::RENOVATEBOT_APP)), + static::cw(function (\DrevOps\VortexInstaller\Tests\Functional\FunctionalTestCase $test): void { + $test->assertFileExists(static::$sut . '/renovate.json'); + $test->assertFileDoesNotExist(static::$sut . '/.github/workflows/update-dependencies.yml'); + }), ],
33-35: Add post-install assertions for “none” flowAssert both renovate.json and CI artifacts are removed when provider is NONE.
'deps updates provider, none' => [ static::cw(fn() => Env::put(PromptManager::makeEnvName(DependencyUpdatesProvider::id()), DependencyUpdatesProvider::NONE)), + static::cw(function (\DrevOps\VortexInstaller\Tests\Functional\FunctionalTestCase $test): void { + $test->assertFileDoesNotExist(static::$sut . '/renovate.json'); + $test->assertFileDoesNotExist(static::$sut . '/.github/workflows/update-dependencies.yml'); + }), ],.vortex/installer/tests/Functional/Handlers/CiProviderInstallTest.php (1)
18-30: Exercise the NONE branch and add post-install assertions per providerCurrently only GHA and CircleCI paths are covered; the “remove both providers” branch is untested. Also add minimal post-assertions for existing cases to verify correct artifacts are present/absent.
'ciprovider, gha' => [ static::cw(function (): void { Env::put(PromptManager::makeEnvName(CiProvider::id()), CiProvider::GITHUB_ACTIONS); Env::put(PromptManager::makeEnvName(AiCodeInstructions::id()), AiCodeInstructions::CLAUDE); }), + static::cw(function (\DrevOps\VortexInstaller\Tests\Functional\FunctionalTestCase $test): void { + $test->assertFileExists(static::$sut . '/.github/workflows/build-test-deploy.yml'); + $test->assertDirectoryDoesNotExist(static::$sut . '/.circleci'); + }), ], 'ciprovider, circleci' => [ static::cw(function (): void { Env::put(PromptManager::makeEnvName(CiProvider::id()), CiProvider::CIRCLECI); Env::put(PromptManager::makeEnvName(AiCodeInstructions::id()), AiCodeInstructions::CLAUDE); }), + static::cw(function (\DrevOps\VortexInstaller\Tests\Functional\FunctionalTestCase $test): void { + $test->assertDirectoryExists(static::$sut . '/.circleci'); + $test->assertFileDoesNotExist(static::$sut . '/.github/workflows/build-test-deploy.yml'); + }), ], + 'ciprovider, none' => [ + static::cw(function (): void { + Env::put(PromptManager::makeEnvName(CiProvider::id()), CiProvider::NONE); + }), + static::cw(function (\DrevOps\VortexInstaller\Tests\Functional\FunctionalTestCase $test): void { + $test->assertFileDoesNotExist(static::$sut . '/.github/workflows/build-test-deploy.yml'); + $test->assertDirectoryDoesNotExist(static::$sut . '/.circleci'); + $test->assertFileDoesNotExist(static::$sut . '/docs/ci.md'); + }), + ],.vortex/installer/tests/Functional/Handlers/ServicesInstallTest.php (1)
44-46: Prefer explicit empty-list encoding over a bare commaUsing ',' relies on Converter::fromList(',') semantics. Encode “none selected” explicitly for clarity and to avoid accidental truthy parsing.
- static::cw(fn() => Env::put(PromptManager::makeEnvName(Services::id()), ',')), + static::cw(fn() => Env::put(PromptManager::makeEnvName(Services::id()), Converter::toList([]))),.vortex/installer/tests/Functional/Handlers/ThemeInstallTest.php (2)
24-29: Broaden assertions to cover all files Theme::process() mutatesTheme::process() touches several configs; verify all are cleaned when theme is absent.
- static::cw(fn(FunctionalTestCase $test) => $test->assertDirectoryNotContainsString('themes/custom', static::$sut, [ - '.gitignore', - 'scripts/vortex', - 'composer.json', - 'CLAUDE.md', - ])), + static::cw(fn(FunctionalTestCase $test) => $test->assertDirectoryNotContainsString('themes/custom', static::$sut, [ + '.gitignore', + 'scripts/vortex', + 'composer.json', + 'phpcs.xml', + 'phpstan.neon', + 'phpmd.xml', + 'phpunit.xml', + 'rector.php', + '.twig-cs-fixer.php', + '.ahoy.yml', + 'CLAUDE.md', + ])),
33-35: Assert PascalCase placeholder removal and directory rename for custom themeCover both snake_case and PascalCase replacements and verify directory rename succeeded.
'theme, custom' => [ static::cw(fn() => Env::put(PromptManager::makeEnvName(Theme::id()), 'light_saber')), - static::cw(fn(FunctionalTestCase $test) => $test->assertDirectoryNotContainsString('your_site_theme', static::$sut)), + static::cw(fn(FunctionalTestCase $test) => $test->assertDirectoryNotContainsString('your_site_theme', static::$sut)), + static::cw(fn(FunctionalTestCase $test) => $test->assertDirectoryNotContainsString('YourSiteTheme', static::$sut)), + static::cw(fn(FunctionalTestCase $test) => $test->assertDirectoryExists(static::$sut . '/web/themes/custom/light_saber')), ],.vortex/installer/tests/Functional/Handlers/DeployTypeInstallTest.php (5)
19-21: Add assertions for ARTIFACT selection: preserve .gitignore files and docs/deployment.md.DeployType::process() keeps .gitignore.{artifact,deployment} and docs/deployment.md when ARTIFACT is chosen. Assert these files exist to prevent regressions.
'deploy type, artifact' => [ - static::cw(fn() => Env::put(PromptManager::makeEnvName(DeployType::id()), Converter::toList([DeployType::ARTIFACT], ',', TRUE))), + static::cw(fn() => Env::put(PromptManager::makeEnvName(DeployType::id()), Converter::toList([DeployType::ARTIFACT]))), + static::cw(fn() => self::assertFileExists(static::$sut . '/.gitignore.artifact')), + static::cw(fn() => self::assertFileExists(static::$sut . '/.gitignore.deployment')), + static::cw(fn() => self::assertFileExists(static::$sut . '/docs/deployment.md')), ],
23-33: Use consistent Converter::toList signature (avoid optional params).Remove explicit separator/strict flags for consistency with the rest of the suite.
'deploy type, lagoon' => [ - static::cw(fn() => Env::put(PromptManager::makeEnvName(DeployType::id()), Converter::toList([DeployType::LAGOON], ',', TRUE))), + static::cw(fn() => Env::put(PromptManager::makeEnvName(DeployType::id()), Converter::toList([DeployType::LAGOON]))), ], 'deploy type, container_image' => [ - static::cw(fn() => Env::put(PromptManager::makeEnvName(DeployType::id()), Converter::toList([DeployType::CONTAINER_IMAGE], ',', TRUE))), + static::cw(fn() => Env::put(PromptManager::makeEnvName(DeployType::id()), Converter::toList([DeployType::CONTAINER_IMAGE]))), ], 'deploy type, webhook' => [ - static::cw(fn() => Env::put(PromptManager::makeEnvName(DeployType::id()), Converter::toList([DeployType::WEBHOOK], ',', TRUE))), + static::cw(fn() => Env::put(PromptManager::makeEnvName(DeployType::id()), Converter::toList([DeployType::WEBHOOK]))), ],
39-44: Strengthen “all, circleci” with cross-handler assertions (GHA removed, CircleCI kept, docs present).Validates interplay with CiProvider::process().
'deploy type, all, circleci' => [ static::cw(function (): void { Env::put(PromptManager::makeEnvName(DeployType::id()), Converter::toList([DeployType::WEBHOOK, DeployType::CONTAINER_IMAGE, DeployType::LAGOON, DeployType::ARTIFACT])); Env::put(PromptManager::makeEnvName(CiProvider::id()), CiProvider::CIRCLECI); }), + static::cw(fn() => self::assertFileDoesNotExist(static::$sut . '/.github/workflows/build-test-deploy.yml')), + static::cw(fn() => self::assertFileExists(static::$sut . '/.circleci/config.yml')), + static::cw(fn() => self::assertFileExists(static::$sut . '/docs/ci.md')), ],
46-55: Assert “none” behavior: docs and both .gitignore files are removed (GHA and CircleCI variants).When DeployType is empty, DeployType::process() deletes docs/deployment.md and both .gitignore files. Make it explicit.
'deploy type, none, gha' => [ - static::cw(fn() => Env::put(PromptManager::makeEnvName(DeployType::id()), ',')), + static::cw(fn() => Env::put(PromptManager::makeEnvName(DeployType::id()), Converter::toList([]))), + static::cw(fn() => self::assertFileDoesNotExist(static::$sut . '/docs/deployment.md')), + static::cw(fn() => self::assertFileDoesNotExist(static::$sut . '/.gitignore.artifact')), + static::cw(fn() => self::assertFileDoesNotExist(static::$sut . '/.gitignore.deployment')), ], 'deploy type, none, circleci' => [ static::cw(function (): void { - Env::put(PromptManager::makeEnvName(DeployType::id()), ','); + Env::put(PromptManager::makeEnvName(DeployType::id()), Converter::toList([]))); Env::put(PromptManager::makeEnvName(CiProvider::id()), CiProvider::CIRCLECI); }), + static::cw(fn() => self::assertFileDoesNotExist(static::$sut . '/docs/deployment.md')), + static::cw(fn() => self::assertFileDoesNotExist(static::$sut . '/.gitignore.artifact')), + static::cw(fn() => self::assertFileDoesNotExist(static::$sut . '/.gitignore.deployment')), ],
46-48: Prefer explicit empty list over “,” sentinel for readability.Using "," to signal “no deploy types” is non-obvious; Converter::toList([]) communicates intent better and avoids parsing quirks.
.vortex/installer/tests/Functional/Handlers/TimezoneInstallTest.php (1)
5-5: Namespace corrected — LGTM.Matches file path and project conventions.
.vortex/installer/tests/Functional/Handlers/HostingProviderInstallTest.php (2)
19-24: Add targeted FS assertions for ACQUIA selection (Lagoon artifacts removed).HostingProvider::process() removes Lagoon artifacts when ACQUIA is chosen. Assert their absence.
'hosting, acquia' => [ static::cw(function (): void { Env::put(PromptManager::makeEnvName(HostingProvider::id()), HostingProvider::ACQUIA); Env::put(PromptManager::makeEnvName(AiCodeInstructions::id()), AiCodeInstructions::CLAUDE); }), + static::cw(fn() => self::assertFileDoesNotExist(static::$sut . '/.lagoon.yml')), + static::cw(fn() => self::assertFileDoesNotExist(static::$sut . '/.github/workflows/close-pull-request.yml')), + static::cw(fn() => self::assertFileDoesNotExist(static::$sut . '/drush/sites/lagoon.site.yml')), ],
26-32: Replace broad text search with concrete assertions; verify Lagoon artifacts and Acquia cleanup.FS checks are more reliable and intention-revealing; also assert .htaccess removal for Lagoon.
'hosting, lagoon' => [ static::cw(function (): void { Env::put(PromptManager::makeEnvName(HostingProvider::id()), HostingProvider::LAGOON); Env::put(PromptManager::makeEnvName(AiCodeInstructions::id()), AiCodeInstructions::CLAUDE); }), - static::cw(fn(FunctionalTestCase $test) => $test->assertSutNotContains('acquia')), + static::cw(fn() => self::assertFileExists(static::$sut . '/.lagoon.yml')), + static::cw(fn() => self::assertFileDoesNotExist(static::$sut . '/hooks')), + static::cw(fn() => self::assertFileDoesNotExist(static::$sut . '/web/.htaccess')), ],.vortex/installer/tests/Functional/Handlers/ProvisionTypeInstallTest.php (3)
19-21: Assert .env rewrite for DATABASE provisioning.ProvisionType::process() rewrites VORTEX_PROVISION_TYPE. Add a focused guard.
'provision, database' => [ static::cw(fn() => Env::put(PromptManager::makeEnvName(ProvisionType::id()), ProvisionType::DATABASE)), + static::cw(fn() => self::assertStringContainsString('VORTEX_PROVISION_TYPE=database', file_get_contents(static::$sut . '/.env'))), ],
23-29: Lagoon + DATABASE: assert .htaccess removed as per HostingProvider::process().Validates the cross-handler side effect.
'provision, database, lagoon' => [ static::cw(function (): void { Env::put(PromptManager::makeEnvName(ProvisionType::id()), ProvisionType::DATABASE); Env::put(PromptManager::makeEnvName(HostingProvider::id()), HostingProvider::LAGOON); Env::put(PromptManager::makeEnvName(AiCodeInstructions::id()), AiCodeInstructions::CLAUDE); }), + static::cw(fn() => self::assertFileDoesNotExist(static::$sut . '/web/.htaccess')), ],
31-36: Assert .env rewrite for PROFILE provisioning.Mirror the DATABASE case to catch regressions.
'provision, profile' => [ static::cw(function (): void { Env::put(PromptManager::makeEnvName(ProvisionType::id()), ProvisionType::PROFILE); Env::put(PromptManager::makeEnvName(AiCodeInstructions::id()), AiCodeInstructions::CLAUDE); }), + static::cw(fn() => self::assertStringContainsString('VORTEX_PROVISION_TYPE=profile', file_get_contents(static::$sut . '/.env'))), ],.vortex/installer/tests/Unit/Handlers/PullRequestPromptManagerTest.php (3)
56-59: Fix closure signature: AbstractPromptManagerTestCase::testRunPrompts() passes ($test, $config).Current single-arg closure will throw ArgumentCountError. Add the missing Config parameter (even if unused).
- function (AbstractPromptManagerTestCase $test): void { + function (AbstractPromptManagerTestCase $test, Config $config): void { // No assign-author.yml workflow and not installed - fall back. },
91-97: Fix closure signature (second occurrence).Same issue for the label-merge-conflict invalid-discovery case.
- function (AbstractPromptManagerTestCase $test): void { + function (AbstractPromptManagerTestCase $test, Config $config): void { // No label-merge-conflict.yml workflow and not installed - fall back. },
23-26: Nit: Remove redundant expected-array overrides that mirror base expectations.Keys already match $expected_defaults/$expected_installed; the explicit overrides add noise without signal.
'auto assign pr - prompt' => [ [AssignAuthorPr::id() => Key::ENTER], - [AssignAuthorPr::id() => TRUE] + $expected_defaults, + $expected_defaults, ], 'label merge conflicts - prompt' => [ [LabelMergeConflictsPr::id() => Key::ENTER], - [LabelMergeConflictsPr::id() => TRUE] + $expected_defaults, + $expected_defaults, ], 'auto assign pr - discovery' => [ [], - [AssignAuthorPr::id() => TRUE] + $expected_installed, + $expected_installed, function (AbstractPromptManagerTestCase $test, Config $config): void { $test->stubVortexProject($config); File::dump(static::$sut . '/.github/workflows/assign-author.yml'); }, ], 'auto assign pr - discovery - non-Vortex' => [ [], - [AssignAuthorPr::id() => TRUE] + $expected_defaults, + $expected_defaults, function (AbstractPromptManagerTestCase $test, Config $config): void { File::dump(static::$sut . '/.github/workflows/assign-author.yml'); }, ], 'label merge conflicts - discovery' => [ [], - [LabelMergeConflictsPr::id() => TRUE] + $expected_installed, + $expected_installed, function (AbstractPromptManagerTestCase $test, Config $config): void { $test->stubVortexProject($config); File::dump(static::$sut . '/.github/workflows/label-merge-conflict.yml'); }, ], 'label merge conflicts - discovery - non-Vortex' => [ [], - [LabelMergeConflictsPr::id() => TRUE] + $expected_defaults, + $expected_defaults, function (AbstractPromptManagerTestCase $test, Config $config): void { File::dump(static::$sut . '/.github/workflows/label-merge-conflict.yml'); }, ],Also applies to: 61-64, 66-73, 75-81, 83-89
.vortex/installer/tests/Functional/Handlers/AbstractInstallTestCase.php (1)
5-5: Namespace inconsistency resolved — thanksThe namespace now matches the
VortexInstallersubtree and other tests in this PR. This addresses the earlier review.
| // Use a two-words name for the sut directory. | ||
| static::$sut = File::mkdir(static::$workspace . DIRECTORY_SEPARATOR . 'star_wars'); | ||
|
|
||
| // Change the current working directory to the 'system under test'. | ||
| chdir(static::$sut); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Minor wording + robustness nit: fix comment and check chdir() result
- Grammar/terminology: "two-word" and "SUT".
- Optional: assert
chdir()succeeded to fail fast with a clear message if the directory cannot be entered.
Apply this diff:
- // Use a two-words name for the sut directory.
+ // Use a two-word name for the SUT directory.
@@
- // Change the current working directory to the 'system under test'.
- chdir(static::$sut);
+ // Change the current working directory to the SUT.
+ $this->assertTrue(@chdir(static::$sut), sprintf('Failed to chdir() into SUT: %s', static::$sut));📝 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.
| // Use a two-words name for the sut directory. | |
| static::$sut = File::mkdir(static::$workspace . DIRECTORY_SEPARATOR . 'star_wars'); | |
| // Change the current working directory to the 'system under test'. | |
| chdir(static::$sut); | |
| } | |
| // Use a two-word name for the SUT directory. | |
| static::$sut = File::mkdir(static::$workspace . DIRECTORY_SEPARATOR . 'star_wars'); | |
| // Change the current working directory to the SUT. | |
| $this->assertTrue( | |
| @chdir(static::$sut), | |
| sprintf('Failed to chdir() into SUT: %s', static::$sut) | |
| ); |
🤖 Prompt for AI Agents
.vortex/installer/tests/Functional/Handlers/AbstractInstallTestCase.php lines
24-29: update the inline comment to use "two-word" and "SUT" terminology and
replace the bare chdir(...) call with a check that chdir succeeded so the test
fails fast with a clear message; specifically, change the comment to mention a
"two-word name for the SUT directory" and assert the return value of chdir
(failing the test or throwing with a descriptive message if it returns false).
| if ($before instanceof SerializableClosure) { | ||
| $before = static::cu($before); | ||
| $before($this); | ||
| } | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Verify availability and contract of static::cu() with SerializableClosure
The test relies on static::cu() to unwrap SerializableClosure to an invokable closure. Ensure it’s available on the hierarchy (likely in a trait) and its signature is stable across PHPUnit process isolation. If not guaranteed, add a small adapter here.
I can provide a tiny adapter like private static function toClosure(?SerializableClosure $c): ?\Closure and use it here for clarity and type-safety.
Also applies to: 56-60
🤖 Prompt for AI Agents
.vortex/installer/tests/Functional/Handlers/AbstractInstallTestCase.php around
lines 40-44 (also apply same change at 56-60): the code uses static::cu() to
unwrap SerializableClosure but that helper may not exist or have a stable
signature under PHPUnit process isolation; add a private static adapter like
toClosure(?SerializableClosure $c): ?\Closure in this class and replace calls to
static::cu($before) with self::toClosure($before) (and similarly for the other
occurrence) so the conversion is explicit, type-safe, and guaranteed to work
regardless of trait availability.
| $expected = empty($expected) ? ['Welcome to the Vortex non-interactive installer'] : $expected; | ||
| $this->assertApplicationOutputContains($expected); | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Prefer strict empty-array check over empty() for typed parameter
$expected is typed as array; using empty() can hide bugs if a non-empty but "falsy" value sneaks in later. A direct === [] keeps intent explicit.
Apply this diff:
- $expected = empty($expected) ? ['Welcome to the Vortex non-interactive installer'] : $expected;
+ if ($expected === []) {
+ $expected = ['Welcome to the Vortex non-interactive installer'];
+ }📝 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.
| $expected = empty($expected) ? ['Welcome to the Vortex non-interactive installer'] : $expected; | |
| $this->assertApplicationOutputContains($expected); | |
| if ($expected === []) { | |
| $expected = ['Welcome to the Vortex non-interactive installer']; | |
| } | |
| $this->assertApplicationOutputContains($expected); |
🤖 Prompt for AI Agents
.vortex/installer/tests/Functional/Handlers/AbstractInstallTestCase.php around
lines 47-49: the code uses empty($expected) to decide defaulting which can mask
non-empty but falsy values despite $expected being typed as array; replace the
empty() check with a strict comparison $expected === [] so the intent is
explicit and only a true empty array triggers the default message, keeping the
rest of the logic unchanged.
| $baseline = File::dir(static::$fixtures . '/../' . self::BASELINE_DIR); | ||
| static::replaceVersions(static::$sut); | ||
| $this->assertDirectoryEqualsPatchedBaseline(static::$sut, $baseline, static::$fixtures); |
There was a problem hiding this comment.
Fix undefined class constant reference: use late static binding for BASELINE_DIR
BASELINE_DIR is defined upstream (e.g., in a parent test case). Referencing it via self:: here will trigger "Undefined class constant" at runtime. Use static:: to honor inheritance/overrides.
Apply this diff:
- $baseline = File::dir(static::$fixtures . '/../' . self::BASELINE_DIR);
+ $baseline = File::dir(static::$fixtures . '/../' . static::BASELINE_DIR);📝 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.
| $baseline = File::dir(static::$fixtures . '/../' . self::BASELINE_DIR); | |
| static::replaceVersions(static::$sut); | |
| $this->assertDirectoryEqualsPatchedBaseline(static::$sut, $baseline, static::$fixtures); | |
| $baseline = File::dir(static::$fixtures . '/../' . static::BASELINE_DIR); | |
| static::replaceVersions(static::$sut); | |
| $this->assertDirectoryEqualsPatchedBaseline(static::$sut, $baseline, static::$fixtures); |
🤖 Prompt for AI Agents
.vortex/installer/tests/Functional/Handlers/AbstractInstallTestCase.php around
lines 50 to 52: the code references the BASELINE_DIR constant using
self::BASELINE_DIR which can cause an "Undefined class constant" at runtime when
subclasses define/override the constant; change the reference to use late static
binding (static::BASELINE_DIR) so the constant is resolved on the actual runtime
class; update that single token from self::BASELINE_DIR to static::BASELINE_DIR
in the line constructing $baseline.
| protected function assertCommon(): void { | ||
| $this->assertDirectoryEqualsDirectory(static::$root . '/scripts/vortex', static::$sut . '/scripts/vortex', 'Vortex scripts were not modified.'); | ||
| if (file_exists(static::$root . '/scripts/vortex.yml')) { | ||
| $this->assertFileEquals(static::$root . '/tests/behat/fixtures/image.jpg', static::$sut . '/tests/behat/fixtures/image.jpg', 'Binary files were not modified.'); | ||
| } | ||
|
|
||
| $this->assertYamlFileIsValid('.ahoy.yml'); | ||
| $this->assertJsonFileIsValid('composer.json'); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Avoid relying on CWD for file validation; use absolute paths
assertYamlFileIsValid() and assertJsonFileIsValid() operate on paths; passing absolute paths avoids accidental coupling to the working directory and makes the assertions resilient if chdir() behavior changes.
Apply this diff:
- $this->assertYamlFileIsValid('.ahoy.yml');
- $this->assertJsonFileIsValid('composer.json');
+ $this->assertYamlFileIsValid(static::$sut . DIRECTORY_SEPARATOR . '.ahoy.yml');
+ $this->assertJsonFileIsValid(static::$sut . DIRECTORY_SEPARATOR . 'composer.json');📝 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.
| protected function assertCommon(): void { | |
| $this->assertDirectoryEqualsDirectory(static::$root . '/scripts/vortex', static::$sut . '/scripts/vortex', 'Vortex scripts were not modified.'); | |
| if (file_exists(static::$root . '/scripts/vortex.yml')) { | |
| $this->assertFileEquals(static::$root . '/tests/behat/fixtures/image.jpg', static::$sut . '/tests/behat/fixtures/image.jpg', 'Binary files were not modified.'); | |
| } | |
| $this->assertYamlFileIsValid('.ahoy.yml'); | |
| $this->assertJsonFileIsValid('composer.json'); | |
| } | |
| protected function assertCommon(): void { | |
| $this->assertDirectoryEqualsDirectory(static::$root . '/scripts/vortex', static::$sut . '/scripts/vortex', 'Vortex scripts were not modified.'); | |
| if (file_exists(static::$root . '/scripts/vortex.yml')) { | |
| $this->assertFileEquals(static::$root . '/tests/behat/fixtures/image.jpg', static::$sut . '/tests/behat/fixtures/image.jpg', 'Binary files were not modified.'); | |
| } | |
| $this->assertYamlFileIsValid(static::$sut . DIRECTORY_SEPARATOR . '.ahoy.yml'); | |
| $this->assertJsonFileIsValid(static::$sut . DIRECTORY_SEPARATOR . 'composer.json'); | |
| } |
🤖 Prompt for AI Agents
.vortex/installer/tests/Functional/Handlers/AbstractInstallTestCase.php around
lines 64 to 72: the calls to assertYamlFileIsValid('.ahoy.yml') and
assertJsonFileIsValid('composer.json') rely on the current working directory;
change them to use absolute paths under the system-under-test (e.g. static::$sut
. '/.ahoy.yml' and static::$sut . '/composer.json') so the assertions validate
the intended files regardless of CWD.
| 'services - discovery - invalid' => [ | ||
| [], | ||
| $expected_defaults, | ||
| function (AbstractPromptManagerTestCase $test): void { | ||
| // Invalid YAML causes discovery to fail and fall back to defaults. | ||
| File::dump(static::$sut . '/docker-compose.yml', <<<'YAML' | ||
| - !text | | ||
| first line | ||
| YAML | ||
| ); | ||
| }, | ||
| ], |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Strengthen the “invalid YAML” scenario to actually exercise discovery on an installed project
As written, the project isn’t marked installed, so Services::discover() returns NULL before parsing YAML. Consider marking it installed to hit the exception path.
Proposed tweak:
'services - discovery - invalid' => [
[],
$expected_defaults,
- function (AbstractPromptManagerTestCase $test): void {
+ function (AbstractPromptManagerTestCase $test, Config $config): void {
+ $test->stubVortexProject($config);
// Invalid YAML causes discovery to fail and fall back to defaults.
File::dump(static::$sut . '/docker-compose.yml', <<<'YAML'
- !text |
first line
YAML
);
},To confirm similar issues elsewhere, you can scan for single-arg $before closures in unit tests:
🏁 Script executed:
#!/bin/bash
# Find handler unit tests that define one-arg $before closures
rg -nP --type=php -C1 $'function\\s*\\(\\s*AbstractPromptManagerTestCase\\s*\\$test\\s*\\)\\s*:\\s*void' .vortex/installer/tests/Unit/HandlersLength of output: 14258
🏁 Script executed:
#!/bin/bash
# Locate stubVortexProject usage and its definition signature
rg -n stubVortexProject .
# Find the AbstractPromptManagerTestCase class definition to inspect available stubbing helpers
rg -nP --type=php -C3 'class\s+AbstractPromptManagerTestCase' .Length of output: 93
🏁 Script executed:
#!/bin/bash
# Show imports and context in Webroot test
sed -n '1,50p' .vortex/installer/tests/Unit/Handlers/WebrootPromptManagerTest.php
# Locate AbstractPromptManagerTestCase class definition
grep -R "class .*AbstractPromptManagerTestCase" -n .
# List files and directories in Unit tests root
find .vortex/installer/tests/Unit -maxdepth 1 -type f -print
find .vortex/installer/tests/Unit -maxdepth 1 -type d -print
# Find stubDotenvValue definition
grep -R "function stubDotenvValue" -n .Length of output: 5929
🏁 Script executed:
#!/bin/bash
# Display stubVortexProject definition and related stubbing helpers in AbstractPromptManagerTestCase
grep -R "stubVortexProject" -n .vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php -C5
# Show surrounding context for stubDotenvValue to compare signatures
grep -nP "protected function stubDotenvValue" -n .vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php -C3Length of output: 1102
🏁 Script executed:
#!/bin/bash
# Display the top portion of AbstractPromptManagerTestCase to inspect callback handling
sed -n '1,200p' .vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.phpLength of output: 7129
🏁 Script executed:
#!/bin/bash
# Display the ServicesPromptManagerTest data provider to confirm the invalid YAML scenario.
sed -n '1,200p' .vortex/installer/tests/Unit/Handlers/ServicesPromptManagerTest.phpLength of output: 3614
Ensure the “invalid YAML” test actually exercises discovery by marking the project as installed
You need to stub a Vortex project in the “services – discovery – invalid” case so that discovery runs (and then fails on the bad YAML), rather than immediately falling back because the project isn’t marked installed.
– In ServicesPromptManagerTest.php, update the closure signature and add the stub call:
'services - discovery - invalid' => [
[],
$expected_defaults,
- function (AbstractPromptManagerTestCase $test): void {
+ function (AbstractPromptManagerTestCase $test, Config $config): void {
+ // Mark as an installed Vortex project so discovery will run.
+ $test->stubVortexProject($config);
// Invalid YAML causes discovery to fail and fall back to defaults.
File::dump(static::$sut . '/docker-compose.yml', <<<'YAML'
- !text |
first line
YAML
);
},This aligns the invalid-YAML case with the other discovery tests and ensures you hit the exception path in Services::discover().
📝 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.
| 'services - discovery - invalid' => [ | |
| [], | |
| $expected_defaults, | |
| function (AbstractPromptManagerTestCase $test): void { | |
| // Invalid YAML causes discovery to fail and fall back to defaults. | |
| File::dump(static::$sut . '/docker-compose.yml', <<<'YAML' | |
| - !text | | |
| first line | |
| YAML | |
| ); | |
| }, | |
| ], | |
| 'services - discovery - invalid' => [ | |
| [], | |
| $expected_defaults, | |
| function (AbstractPromptManagerTestCase $test, Config $config): void { | |
| // Mark as an installed Vortex project so discovery will run. | |
| $test->stubVortexProject($config); | |
| // Invalid YAML causes discovery to fail and fall back to defaults. | |
| File::dump(static::$sut . '/docker-compose.yml', <<<'YAML' | |
| - !text | | |
| first line | |
| YAML | |
| ); | |
| }, | |
| ], |
🤖 Prompt for AI Agents
In .vortex/installer/tests/Unit/Handlers/ServicesPromptManagerTest.php around
lines 74-85, the "services - discovery - invalid" test closure needs to actually
stub a Vortex project as installed so discovery runs; change the closure
signature to accept the concrete test class (e.g. ServicesPromptManagerTestCase
$test) and inside the closure call the test helper to mark the project installed
(e.g. $test->stubVortexProjectInstalled();) before writing the invalid
docker-compose.yml so the Services::discover() path is exercised and fails on
the bad YAML.
| function (AbstractPromptManagerTestCase $test): void { | ||
| // Invalid YAML causes discovery to fail and fall back to defaults. | ||
| File::dump(static::$sut . '/docker-compose.yml', <<<'YAML' | ||
| - !text | | ||
| first line | ||
| YAML | ||
| ); | ||
| }, | ||
| ], |
There was a problem hiding this comment.
Fix: $before closure signature mismatch will throw ArgumentCountError
Align the invalid YAML scenario’s closure with the runner’s two-argument call.
- function (AbstractPromptManagerTestCase $test): void {
+ function (AbstractPromptManagerTestCase $test, Config $config): void {
// Invalid YAML causes discovery to fail and fall back to defaults.
File::dump(static::$sut . '/docker-compose.yml', <<<'YAML'
- !text |
first line
YAML
);
},📝 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.
| function (AbstractPromptManagerTestCase $test): void { | |
| // Invalid YAML causes discovery to fail and fall back to defaults. | |
| File::dump(static::$sut . '/docker-compose.yml', <<<'YAML' | |
| - !text | | |
| first line | |
| YAML | |
| ); | |
| }, | |
| ], | |
| function (AbstractPromptManagerTestCase $test, Config $config): void { | |
| // Invalid YAML causes discovery to fail and fall back to defaults. | |
| File::dump(static::$sut . '/docker-compose.yml', <<<'YAML' | |
| - !text | | |
| first line | |
| YAML | |
| ); | |
| }, |
🤖 Prompt for AI Agents
.vortex/installer/tests/Unit/Handlers/ServicesPromptManagerTest.php around lines
77 to 85: the closure used for the "Invalid YAML" scenario currently accepts one
parameter but the test runner invokes it with two arguments, which leads to an
ArgumentCountError; update the closure signature to accept the second parameter
(e.g., add a second parameter like $data or $runnerArgs with a proper type or
nullable type) and update any internal references if needed so the closure
matches the runner's two-argument call.
| 'tools - discovery - all tools' => [ | ||
| [], | ||
| [Tools::id() => [Tools::BEHAT, Tools::PHPCS, Tools::PHPMD, Tools::PHPSTAN, Tools::PHPUNIT, Tools::RECTOR]] + $expected_installed, | ||
| function (AbstractPromptManagerTestCase $test, Config $config): void { | ||
| $test->stubVortexProject($config); | ||
| $dependencies = [ | ||
| 'squizlabs/php_codesniffer' => '*', | ||
| 'phpmd/phpmd' => '*', | ||
| 'phpstan/phpstan' => '*', | ||
| 'rector/rector' => '*', | ||
| 'phpunit/phpunit' => '*', | ||
| 'behat/behat' => '*', | ||
| ]; | ||
| $test->stubComposerJsonDependencies($dependencies, TRUE); | ||
| }, | ||
| ], | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Optional: add regression coverage for tool “groups” logic
The handler’s processGroup logic (and a known ‘goups’ key typo risk) isn’t exercised here. Consider a dataset that deselects all tools on an installed project and asserts group-side effects (e.g., Ahoy command removals or tokens), plus a contrasting case where at least one tool in a group remains. This would guard against regressions in group processing.
Happy to draft an additional dataset with an $after callback asserting expected file/token changes.
| 'tools - discovery - non-Vortex project' => [ | ||
| [], | ||
| $expected_defaults, | ||
| function (AbstractPromptManagerTestCase $test, Config $config): void { | ||
| $dependencies = [ | ||
| 'squizlabs/php_codesniffer' => '*', | ||
| 'phpmd/phpmd' => '*', | ||
| 'phpstan/phpstan' => '*', | ||
| 'rector/rector' => '*', | ||
| 'phpunit/phpunit' => '*', | ||
| 'behat/behat' => '*', | ||
| ]; | ||
| $test->stubComposerJsonDependencies($dependencies, TRUE); | ||
| }, | ||
| ], | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Dataset naming nit: clarify “non-Vortex project” behavior
Minor: consider “discovery - not installed (falls back to defaults)” to align with semantics used in other handlers.
- 'tools - discovery - non-Vortex project' => [
+ 'tools - discovery - not installed (falls back to defaults)' => [📝 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.
| 'tools - discovery - non-Vortex project' => [ | |
| [], | |
| $expected_defaults, | |
| function (AbstractPromptManagerTestCase $test, Config $config): void { | |
| $dependencies = [ | |
| 'squizlabs/php_codesniffer' => '*', | |
| 'phpmd/phpmd' => '*', | |
| 'phpstan/phpstan' => '*', | |
| 'rector/rector' => '*', | |
| 'phpunit/phpunit' => '*', | |
| 'behat/behat' => '*', | |
| ]; | |
| $test->stubComposerJsonDependencies($dependencies, TRUE); | |
| }, | |
| ], | |
| 'tools - discovery - not installed (falls back to defaults)' => [ | |
| [], | |
| $expected_defaults, | |
| function (AbstractPromptManagerTestCase $test, Config $config): void { | |
| $dependencies = [ | |
| 'squizlabs/php_codesniffer' => '*', | |
| 'phpmd/phpmd' => '*', | |
| 'phpstan/phpstan' => '*', | |
| 'rector/rector' => '*', | |
| 'phpunit/phpunit' => '*', | |
| 'behat/behat' => '*', | |
| ]; | |
| $test->stubComposerJsonDependencies($dependencies, TRUE); | |
| }, | |
| ], |
🤖 Prompt for AI Agents
.vortex/installer/tests/Unit/Handlers/ToolsPromptManagerTest.php around lines 51
to 66: the dataset key string "tools - discovery - non-Vortex project" should be
renamed to match existing test semantics; change it to "tools - discovery - not
installed (falls back to defaults)" so it aligns with other handlers and
clarifies that discovery falls back to defaults when tools are not installed.
| 'webroot - discovery - invalid' => [ | ||
| [], | ||
| $expected_defaults, | ||
| function (AbstractPromptManagerTestCase $test): void { | ||
| // No WEBROOT in .env and no composer.json scaffold - fall back. | ||
| }, | ||
| ], |
There was a problem hiding this comment.
Fix: $before closure signature mismatch will throw ArgumentCountError
The invalid-discovery case defines a one-arg closure; the runner passes two.
- function (AbstractPromptManagerTestCase $test): void {
+ function (AbstractPromptManagerTestCase $test, Config $config): void {
// No WEBROOT in .env and no composer.json scaffold - fall back.
},📝 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.
| 'webroot - discovery - invalid' => [ | |
| [], | |
| $expected_defaults, | |
| function (AbstractPromptManagerTestCase $test): void { | |
| // No WEBROOT in .env and no composer.json scaffold - fall back. | |
| }, | |
| ], | |
| 'webroot - discovery - invalid' => [ | |
| [], | |
| $expected_defaults, | |
| function (AbstractPromptManagerTestCase $test, Config $config): void { | |
| // No WEBROOT in .env and no composer.json scaffold - fall back. | |
| }, | |
| ], |
🤖 Prompt for AI Agents
.vortex/installer/tests/Unit/Handlers/WebrootPromptManagerTest.php around lines
66 to 72: the 'webroot - discovery - invalid' case provides a closure that
accepts one argument but the test runner invokes it with two, causing an
ArgumentCountError; change the closure signature to accept two parameters (e.g.,
function (AbstractPromptManagerTestCase $test, $second = null): void) or
explicitly type the second param (with a default) and ignore it inside the body
so the closure matches the runner's call signature.
Summary by CodeRabbit
Documentation
Tests