branch [#1560] Converted BATS functional tests to PHPUnit functional tests. #2007
Conversation
WalkthroughAdds extensive PHPUnit test infrastructure under .vortex/tests: composer dev dependency/config tweaks, FunctionalTestCase enhancements (process wrappers, wildcard assertions, lifecycle logging), many new Step* traits (Ahoy/DB/Env/FE/Services/Testing), new functional tests, and related trait adjustments. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant FT as FunctionalTestCase
participant PT as ProcessTrait (aliased)
participant Env as Environment
Note over FT: processRun wrapper injects AHOY_CONFIRM vars and timeouts
FT->>Env: merge env + {AHOY_CONFIRM_RESPONSE, AHOY_CONFIRM_WAIT_SKIP}
FT->>PT: traitProcessRun(command, args, inputs, mergedEnv, timeouts)
PT-->>FT: Process
Note right of FT: processRunInContainer prefixes "ahoy cli --" then delegates
sequenceDiagram
autonumber
participant WT as WorkflowInstallDbTest
participant ST as Step Traits (Ahoy/DB/Env/FE/Services/Testing)
WT->>WT: setUp (parent + stepDownloadDb)
WT->>ST: stepBuild -> stepAhoyLogin -> assertFilesTrackedInGit
WT->>ST: stepAhoyCli -> stepEnvChanges -> stepTimezone
WT->>ST: stepAhoyComposer -> stepAhoyDrush -> stepAhoyInfo -> stepAhoyContainerLogs
WT->>ST: stepAhoyImportDb / stepAhoyExportDb -> stepAhoyProvision
WT->>ST: stepAhoyLint -> stepAhoyTest -> stepAhoyFei -> stepAhoyFe -> stepAhoyDebug
WT->>ST: stepSolr -> stepRedis
WT-->>WT: tearDown (log summary)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1).vortex/tests/phpunit/Functional/FunctionalTestCase.php (1)
⏰ 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). (12)
🔇 Additional comments (1)
Pre-merge checks (4 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2007 +/- ##
========================================
Coverage 78.02% 78.02%
========================================
Files 89 89
Lines 5542 5542
Branches 44 44
========================================
Hits 4324 4324
Misses 1218 1218 ☔ 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: 21
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.vortex/tests/phpunit/Functional/FunctionalTestCase.php (1)
121-123: Use strict comparison for env flag parsing.Avoid loose comparisons with getenv.
Apply:
- return getenv('VORTEX_DEV_VOLUMES_SKIP_MOUNT') != 1; + return getenv('VORTEX_DEV_VOLUMES_SKIP_MOUNT') === '0' || getenv('VORTEX_DEV_VOLUMES_SKIP_MOUNT') === false;Or, if the intent is “mounted when flag != '1'”:
- return getenv('VORTEX_DEV_VOLUMES_SKIP_MOUNT') != 1; + return getenv('VORTEX_DEV_VOLUMES_SKIP_MOUNT') !== '1';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
.vortex/tests/composer.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
.vortex/tests/composer.json(2 hunks).vortex/tests/phpunit/Functional/FunctionalTestCase.php(3 hunks).vortex/tests/phpunit/Functional/SelfTest.php(1 hunks).vortex/tests/phpunit/Functional/WorkflowInstallDbTest.php(1 hunks).vortex/tests/phpunit/Traits/AssertFilesTrait.php(3 hunks).vortex/tests/phpunit/Traits/GitTrait.php(1 hunks).vortex/tests/phpunit/Traits/Steps/StepAhoyTrait.php(1 hunks).vortex/tests/phpunit/Traits/Steps/StepDatabaseTrait.php(1 hunks).vortex/tests/phpunit/Traits/Steps/StepEnvironmentTrait.php(1 hunks).vortex/tests/phpunit/Traits/Steps/StepFrontendTrait.php(1 hunks).vortex/tests/phpunit/Traits/Steps/StepServicesTrait.php(1 hunks).vortex/tests/phpunit/Traits/Steps/StepTestingTrait.php(1 hunks)web/themes/custom/your_site_theme/Gruntfile.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
web/themes/custom/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place all custom Drupal themes under web/themes/custom/[theme_name]/
Files:
web/themes/custom/your_site_theme/Gruntfile.js
🧠 Learnings (1)
📚 Learning: 2025-08-08T12:02:24.652Z
Learnt from: AlexSkrypnyk
PR: drevops/vortex#1896
File: .vortex/tests/bats/unit/download-db-lagoon.bats:24-25
Timestamp: 2025-08-08T12:02:24.652Z
Learning: In .vortex/tests/bats/unit Bats tests using ../_helper.bash (run_steps), prefixing a STEPS entry with "- " denotes a negative assertion (the substring must NOT appear in output). Unprefixed entries are positive assertions. Example: "- Database dump refresh requested. Will create a new dump." asserts absence; "Database dump refresh requested. Will create a new dump." asserts presence.
Applied to files:
.vortex/tests/phpunit/Traits/Steps/StepEnvironmentTrait.php.vortex/tests/phpunit/Traits/Steps/StepTestingTrait.php.vortex/tests/phpunit/Traits/Steps/StepDatabaseTrait.php
🧬 Code graph analysis (9)
.vortex/tests/phpunit/Traits/AssertFilesTrait.php (2)
.vortex/tests/phpunit/Traits/GitTrait.php (1)
gitAssertFilesNotTracked(358-371).vortex/tests/phpunit/Traits/LoggerTrait.php (1)
logSubstep(32-34)
.vortex/tests/phpunit/Traits/Steps/StepAhoyTrait.php (2)
.vortex/tests/phpunit/Traits/LoggerTrait.php (3)
logStepStart(16-22)logSubstep(32-34)logStepFinish(24-30).vortex/tests/phpunit/Functional/FunctionalTestCase.php (1)
processRun(76-90)
.vortex/tests/phpunit/Functional/WorkflowInstallDbTest.php (10)
.vortex/tests/phpunit/Functional/FunctionalTestCase.php (3)
FunctionalTestCase(26-185)setUp(43-55)tearDown(57-64).vortex/tests/phpunit/Traits/Steps/StepPrepareSutTrait.php (1)
stepPrepareSut(18-40).vortex/tests/phpunit/Traits/Steps/StepDownloadDbTrait.php (1)
stepDownloadDb(22-37).vortex/tests/phpunit/Traits/Steps/StepBuildTrait.php (1)
stepBuild(16-63).vortex/tests/phpunit/Traits/Steps/StepAhoyTrait.php (6)
stepAhoyLogin(106-115)stepAhoyCli(16-38)stepAhoyComposer(40-50)stepAhoyDrush(52-61)stepAhoyInfo(63-93)stepAhoyContainerLogs(95-104).vortex/tests/phpunit/Traits/AssertFilesTrait.php (1)
assertFilesTrackedInGit(293-322).vortex/tests/phpunit/Traits/Steps/StepDatabaseTrait.php (3)
stepAhoyImportDb(38-49)stepAhoyExportDb(17-36)stepAhoyProvision(51-123).vortex/tests/phpunit/Traits/Steps/StepTestingTrait.php (1)
stepAhoyLint(17-29).vortex/tests/phpunit/Traits/Steps/StepFrontendTrait.php (2)
stepAhoyFei(17-32)stepAhoyFe(34-74).vortex/tests/phpunit/Traits/Steps/StepServicesTrait.php (2)
stepSolr(16-25)stepRedis(27-82)
.vortex/tests/phpunit/Traits/Steps/StepServicesTrait.php (3)
.vortex/tests/phpunit/Traits/LoggerTrait.php (3)
logStepStart(16-22)logSubstep(32-34)logStepFinish(24-30).vortex/tests/phpunit/Functional/FunctionalTestCase.php (2)
processRun(76-90)syncToContainer(112-119).vortex/tests/phpunit/Traits/Steps/StepEnvironmentTrait.php (2)
addVarToFile(259-265)restoreFile(275-280)
.vortex/tests/phpunit/Traits/Steps/StepEnvironmentTrait.php (4)
.vortex/installer/src/Utils/File.php (1)
File(10-121).vortex/tests/phpunit/Traits/LoggerTrait.php (3)
logStepStart(16-22)logSubstep(32-34)logStepFinish(24-30).vortex/tests/phpunit/Functional/FunctionalTestCase.php (2)
processRun(76-90)syncToContainer(112-119).vortex/tests/phpunit/Traits/AssertFilesTrait.php (1)
assertCommonFilesPresent(20-39)
.vortex/tests/phpunit/Traits/Steps/StepFrontendTrait.php (3)
.vortex/installer/src/Utils/File.php (1)
File(10-121).vortex/tests/phpunit/Traits/LoggerTrait.php (3)
logStepStart(16-22)logSubstep(32-34)logStepFinish(24-30).vortex/tests/phpunit/Functional/FunctionalTestCase.php (3)
syncToContainer(112-119)processRun(76-90)syncToHost(103-110)
.vortex/tests/phpunit/Traits/Steps/StepTestingTrait.php (4)
.vortex/installer/src/Utils/File.php (1)
File(10-121).vortex/tests/phpunit/Traits/LoggerTrait.php (3)
logStepStart(16-22)logSubstep(32-34)logStepFinish(24-30).vortex/tests/phpunit/Functional/FunctionalTestCase.php (4)
processRun(76-90)syncToContainer(112-119)syncToHost(103-110)assertFilesWildcardExists(137-156).vortex/tests/phpunit/Traits/Steps/StepEnvironmentTrait.php (1)
restoreFile(275-280)
.vortex/tests/phpunit/Traits/Steps/StepDatabaseTrait.php (3)
.vortex/installer/src/Utils/File.php (1)
File(10-121).vortex/tests/phpunit/Traits/LoggerTrait.php (3)
logStepStart(16-22)logSubstep(32-34)logStepFinish(24-30).vortex/tests/phpunit/Functional/FunctionalTestCase.php (4)
processRun(76-90)syncToHost(103-110)assertFilesWildcardExists(137-156)syncToContainer(112-119)
.vortex/tests/phpunit/Functional/SelfTest.php (2)
.vortex/installer/src/Utils/File.php (1)
File(10-121).vortex/tests/phpunit/Functional/FunctionalTestCase.php (4)
FunctionalTestCase(26-185)setUp(43-55)assertFilesWildcardExists(137-156)assertFilesWildcardDoNotExist(164-183)
⏰ 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). (12)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-workflow (4)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: build (0)
- GitHub Check: build (1)
🔇 Additional comments (23)
.vortex/tests/composer.json (1)
15-16: Dev deps bump looks fine; ensure lockfile coherence.After merging, run composer update in the tests workspace and commit composer.lock to avoid CI drift.
.vortex/tests/phpunit/Traits/AssertFilesTrait.php (1)
321-321: Local settings workflow rename and behavior LGTM.Renaming to removeLocalDrupalSettings() and copying example.* to local files with clear logging improves intent and mirrors real setups.
Also applies to: 325-334, 336-341
.vortex/tests/phpunit/Traits/Steps/StepServicesTrait.php (1)
16-25: Solr smoke test is concise and good..vortex/tests/phpunit/Traits/Steps/StepDatabaseTrait.php (2)
17-36: Overall DB steps flow reads well and covers key paths.Also applies to: 38-49, 51-123
79-84: Issued scripts to locate regex-based assertion methods and TestCase definitions for verification..vortex/tests/phpunit/Traits/Steps/StepFrontendTrait.php (2)
51-57: Ensure prod build actually minifies; tests rely on 'background:' (no space).This depends on Gruntfile sass:prod being configured to outputStyle: 'compressed'. Please align Gruntfile as noted in its review.
Also applies to: 66-72
17-32: Nice end-to-end FE flow with install/build and verification.Also applies to: 34-75
.vortex/tests/phpunit/Functional/FunctionalTestCase.php (3)
34-36: Trait aliasing for processRun is clean and future-proof.
73-90: Wrapper injecting AHOY env defaults is correct (uses $env += to allow overrides).
131-156: Wildcard file assertions are robust and well-factored.Also applies to: 158-183
.vortex/tests/phpunit/Functional/WorkflowInstallDbTest.php (1)
14-17: LGTM! Clear and informative class documentation.The PHPDoc comment effectively communicates the test's purpose.
.vortex/tests/phpunit/Traits/Steps/StepEnvironmentTrait.php (12)
1-13: Well-structured trait with appropriate namespace and imports.The trait follows PHP best practices with proper strict typing declaration, clear namespace, and appropriate imports. The LoggerTrait usage aligns with the existing logging pattern shown in the codebase.
17-28: LGTM: Clean git status verification.The stepGitignore implementation correctly checks git tracking behavior and verifies the presence of the .gitignore file. The logging pattern is consistent with other step methods.
30-80: Comprehensive environment variable testing with proper cleanup.This method thoroughly tests environment variable lifecycle - from absence verification, through addition and container sync, to restoration. The backup/restore pattern ensures test isolation. The dual approach of testing both environment variable presence and actual value availability in containers is thorough.
82-119: Timezone testing covers multiple services appropriately.The stepTimezone method tests timezone configuration across all relevant Docker services (cli, php, nginx, database). The use of specific timezone abbreviations (UTC, AWST) for verification is more reliable than checking full timezone names.
Note: The hardcoded sleep(10) at line 116 might be necessary for container stabilization, but consider if this duration is optimal for CI environments.
121-177: Thorough Xdebug state management testing.The stepAhoyDebug method comprehensively tests Xdebug enable/disable functionality, including verification from both inside and outside containers. The pattern matching approach using "reat" from "Create"/"Creating" is clever for detecting container restarts.
179-220: Reset functionality testing with appropriate file preservation checks.The stepAhoyReset method tests the reset command while verifying that development files and IDE configurations are preserved. The test creates realistic scenarios with untracked files and development settings.
The hardcoded sleep(10) at line 194 should be consistent with container startup times.
222-257: Hard reset testing verifies complete cleanup behavior.The stepAhoyResetHard method appropriately tests the more aggressive reset functionality, ensuring development settings and untracked files are removed while preserving IDE configurations.
259-265: Simple and effective variable addition helper.The addVarToFile method provides a clean way to add environment variables with proper backup handling. The format string approach ensures consistent formatting.
267-280: Reliable backup and restore mechanism.The backup/restore file pattern using /tmp/bkp provides good test isolation. The directory creation with proper permissions and the existence check in restoreFile are appropriate safety measures.
282-297: Development settings management helpers are well-implemented.The createDevelopmentSettings and removeDevelopmentSettings methods properly handle the standard Drupal development configuration files with immediate assertions to verify the operations.
299-306: Assert helpers provide appropriate abstractions.The assertFilesPresent and assertGitRepo methods provide clean abstractions for common assertions, with assertFilesPresent correctly delegating to the existing assertCommonFilesPresent method from AssertFilesTrait.
268-268: Verify backup directory permissions and cleanup.The backup directory /tmp/bkp is created but never cleaned up. Consider whether test isolation requires cleaning up backup files between tests.
Should the backup directory be cleaned up after tests complete, or does the test framework handle /tmp cleanup automatically?
| }, | ||
| "discard-changes": true, | ||
| "process-timeout": 600, | ||
| "process-timeout": 0, |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Avoid unbounded Composer process-timeout (use a large finite value).
Setting "process-timeout": 0 can mask hung installs and stall CI. Prefer a generous finite value (e.g., 1800–3600s).
Apply:
- "process-timeout": 0,
+ "process-timeout": 3600,📝 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.
| "process-timeout": 0, | |
| "process-timeout": 3600, |
🤖 Prompt for AI Agents
.vortex/tests/composer.json around line 41: the Composer "process-timeout" is
set to 0 (unbounded) which can stall CI; change the value to a large finite
timeout (for example 1800 or 3600 seconds) by replacing 0 with the chosen
integer, commit the change, and ensure any test/CI configs that rely on this
file are updated and validated by running the relevant test job locally or in CI
to confirm behavior.
| public function processRunInContainer( | ||
| string $command, | ||
| array $arguments = [], | ||
| array $inputs = [], | ||
| array $env = [], | ||
| int $timeout = 60, | ||
| int $idle_timeout = 30, | ||
| ): Process { | ||
| return static::traitProcessRun('ahoy cli -- ' . $command, $arguments, $inputs, $env, $timeout, $idle_timeout); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Prefer $this->traitProcessRun over static::traitProcessRun.
Using $this preserves late bindings/mocks; static calls can be harder to stub.
Apply:
- return static::traitProcessRun('ahoy cli -- ' . $command, $arguments, $inputs, $env, $timeout, $idle_timeout);
+ return $this->traitProcessRun('ahoy cli -- ' . $command, $arguments, $inputs, $env, $timeout, $idle_timeout);📝 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 function processRunInContainer( | |
| string $command, | |
| array $arguments = [], | |
| array $inputs = [], | |
| array $env = [], | |
| int $timeout = 60, | |
| int $idle_timeout = 30, | |
| ): Process { | |
| return static::traitProcessRun('ahoy cli -- ' . $command, $arguments, $inputs, $env, $timeout, $idle_timeout); | |
| } | |
| public function processRunInContainer( | |
| string $command, | |
| array $arguments = [], | |
| array $inputs = [], | |
| array $env = [], | |
| int $timeout = 60, | |
| int $idle_timeout = 30, | |
| ): Process { | |
| return $this->traitProcessRun('ahoy cli -- ' . $command, $arguments, $inputs, $env, $timeout, $idle_timeout); | |
| } |
🤖 Prompt for AI Agents
.vortex/tests/phpunit/Functional/FunctionalTestCase.php around lines 92 to 101:
The method processRunInContainer calls static::traitProcessRun which prevents
instance-level overrides/mocks; change the call to $this->traitProcessRun('ahoy
cli -- ' . $command, $arguments, $inputs, $env, $timeout, $idle_timeout) so the
instance can be properly stubbed or mocked; keep the method signature and
argument order identical and run the test suite to ensure no regressions.
| $this->processRun('ahoy cli curl -L -s "http://nginx:8080" >/dev/null'); | ||
| $this->processRun('docker compose exec redis redis-cli --scan'); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Shell redirection inside processRun may be ignored depending on how commands are executed.
If processRun doesn’t use a shell, >/dev/null won’t be applied. Prefer passing curl’s -o /dev/null -sS flags rather than shell redirection.
Also applies to: 67-68
🤖 Prompt for AI Agents
In .vortex/tests/phpunit/Traits/Steps/StepServicesTrait.php around lines 45-46
(and similarly at 67-68), the commands rely on shell redirection (>/dev/null)
which will be ignored if processRun executes commands without a shell; change
the curl invocation to use curl's built-in output/quiet flags instead (e.g. add
-o /dev/null -sS) so suppression works regardless of shell, and leave the
redis-cli --scan invocation as-is (or ensure it does not rely on shell features)
— update both occurrences accordingly.
| // @todo Add restoring of the file. | ||
| $this->logStepFinish(); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Incomplete implementation with TODO.
The TODO comment indicates missing file restoration logic, which could leave the system in an inconsistent state for subsequent tests.
The original file content should be preserved and restored. Would you like me to implement the file restoration logic to ensure proper cleanup?
protected function stepAhoyLintBe(string $webroot = 'web'): void {
$this->logStepStart();
+ $phpcs_file = 'phpcs.xml';
+ $original_phpcs_content = File::read($phpcs_file);
+ $module_file = $webroot . '/modules/custom/sw_base/sw_base.module';
+ $original_module_content = File::read($module_file);
+
$this->logSubstep('Assert that BE lint failure works');
- File::dump($webroot . '/modules/custom/sw_base/sw_base.module', File::read($webroot . '/modules/custom/sw_base/sw_base.module') . '$a=1;');
+ File::dump($module_file, $original_module_content . '$a=1;');
$this->syncToContainer();
$this->processRun('ahoy lint-be', timeout: 120, idle_timeout: 90);
$this->assertProcessFailed();
$this->logSubstep('Assert that BE lint tool disabling works');
// Replace with some valid XML element to avoid XML parsing errors.
- File::replaceContentInFile('phpcs.xml', '<file>' . $webroot . '/modules/custom</file>', '<exclude-pattern>somefile</exclude-pattern>');
+ File::replaceContentInFile($phpcs_file, '<file>' . $webroot . '/modules/custom</file>', '<exclude-pattern>somefile</exclude-pattern>');
$this->syncToContainer();
$this->processRun('ahoy lint-be', timeout: 120, idle_timeout: 90);
$this->assertProcessSuccessful();
- // @todo Add restoring of the file.
+ // Restore original files
+ File::dump($phpcs_file, $original_phpcs_content);
+ File::dump($module_file, $original_module_content);
+ $this->syncToContainer();
+
$this->logStepFinish();
}📝 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.
| // @todo Add restoring of the file. | |
| $this->logStepFinish(); | |
| protected function stepAhoyLintBe(string $webroot = 'web'): void { | |
| $this->logStepStart(); | |
| // Capture original file contents for restoration | |
| $phpcs_file = 'phpcs.xml'; | |
| $original_phpcs_content = File::read($phpcs_file); | |
| $module_file = $webroot . '/modules/custom/sw_base/sw_base.module'; | |
| $original_module_content = File::read($module_file); | |
| $this->logSubstep('Assert that BE lint failure works'); | |
| File::dump($module_file, $original_module_content . '$a=1;'); | |
| $this->syncToContainer(); | |
| $this->processRun('ahoy lint-be', timeout: 120, idle_timeout: 90); | |
| $this->assertProcessFailed(); | |
| $this->logSubstep('Assert that BE lint tool disabling works'); | |
| // Replace with some valid XML element to avoid XML parsing errors. | |
| File::replaceContentInFile( | |
| $phpcs_file, | |
| '<file>' . $webroot . '/modules/custom</file>', | |
| '<exclude-pattern>somefile</exclude-pattern>' | |
| ); | |
| $this->syncToContainer(); | |
| $this->processRun('ahoy lint-be', timeout: 120, idle_timeout: 90); | |
| $this->assertProcessSuccessful(); | |
| // Restore original files | |
| File::dump($phpcs_file, $original_phpcs_content); | |
| File::dump($module_file, $original_module_content); | |
| $this->syncToContainer(); | |
| $this->logStepFinish(); | |
| } |
🤖 Prompt for AI Agents
.vortex/tests/phpunit/Traits/Steps/StepTestingTrait.php around lines 47-48: the
TODO notes missing file restoration after tests which can leave persistent
changes; implement saving the original file state before modification (read and
store its contents and file permissions/ownership if applicable, or record that
it didn't exist), then ensure restoration in a teardown/finally path by writing
back the original contents (or deleting the created file if it didn’t exist),
restoring permissions, and handling exceptions (log or rethrow) so cleanup
always runs even on test failures.
| // Sometimes, tests fail for random reasons. A workaround is to run BDD | ||
| // tests to "cache" the environment and then run the tests again. | ||
| $this->processRun('ahoy test-bdd || true'); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Error suppression pattern could mask real failures.
Using || true to suppress errors in the initial BDD test run (lines 208 and 236) could hide legitimate setup issues. Consider handling specific known transient failures more explicitly.
Consider implementing a retry mechanism with specific error detection:
- // Sometimes, tests fail for random reasons. A workaround is to run BDD
- // tests to "cache" the environment and then run the tests again.
- $this->processRun('ahoy test-bdd || true');
+ // Warm up the test environment with a retry mechanism
+ $max_retries = 2;
+ $retry_count = 0;
+ $success = false;
+
+ while ($retry_count < $max_retries && !$success) {
+ $this->processRun('ahoy test-bdd');
+ if ($this->getProcessExitCode() === 0) {
+ $success = true;
+ } else if ($retry_count < $max_retries - 1) {
+ $this->logSubstep('BDD test warm-up failed, retrying...');
+ sleep(2);
+ }
+ $retry_count++;
+ }Also applies to: 234-237
🤖 Prompt for AI Agents
In .vortex/tests/phpunit/Traits/Steps/StepTestingTrait.php around lines 206-209
and 234-237, the use of "ahoy test-bdd || true" blindly suppresses all errors;
replace this with a small retry loop (e.g., 2-3 attempts with short backoff)
that runs the command normally, inspects the exit code and/or stderr for known
transient failure signatures, and only retries/suppresses when those specific
transient conditions are detected; on other failures log the full output and
return a non-zero status so the test run fails fast.
| $this->restoreFile('.env'); | ||
| $this->processRun('ahoy up cli'); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Undefined method call detected.
The restoreFile method is called but not defined in this trait. Based on the code snippets, this method exists in StepEnvironmentTrait.
🏁 Script executed:
#!/bin/bash
# Verify that StepEnvironmentTrait is properly included when StepTestingTrait is used
# Check if WorkflowInstallDbTest uses both traits
rg -n "use StepEnvironmentTrait|use StepTestingTrait" .vortex/tests/phpunit/Functional/WorkflowInstallDbTest.php
# Check if there are other test classes using StepTestingTrait without StepEnvironmentTrait
fd -e php -x grep -l "use.*StepTestingTrait" {} \; | while read -r file; do
echo "=== $file ==="
grep -n "use StepEnvironmentTrait\|use StepTestingTrait" "$file" || echo "Missing StepEnvironmentTrait"
doneLength of output: 57
🏁 Script executed:
#!/bin/bash
# Verify that restoreFile is defined in StepEnvironmentTrait
rg -n "function restoreFile" -C2 -g ".vortex/tests/phpunit/Traits/Steps/StepEnvironmentTrait.php" || echo "No restoreFile definition found in StepEnvironmentTrait"Length of output: 204
🏁 Script executed:
#!/bin/bash
# Locate the StepEnvironmentTrait definition
fd -e php StepEnvironmentTrait.php
# Search for restoreFile in the located file
rg -n "restoreFile" -C3 $(fd -e php StepEnvironmentTrait.php)
# Verify StepTestingTrait imports StepEnvironmentTrait
rg -n "use StepEnvironmentTrait" .vortex/tests/phpunit/Traits/Steps/StepTestingTrait.php
# Confirm restoreFile is invoked in StepTestingTrait
rg -n "restoreFile" .vortex/tests/phpunit/Traits/Steps/StepTestingTrait.phpLength of output: 35
🏁 Script executed:
#!/bin/bash
# Locate the StepEnvironmentTrait file
env_trait_file=$(fd -e php StepEnvironmentTrait.php)
# Show the definition of restoreFile in StepEnvironmentTrait
rg -n "function restoreFile" -C3 "$env_trait_file" || echo "restoreFile not found in $env_trait_file"
# Confirm StepTestingTrait includes StepEnvironmentTrait
rg -n "use .*StepEnvironmentTrait" .vortex/tests/phpunit/Traits/Steps/StepTestingTrait.phpLength of output: 66
🏁 Script executed:
#!/bin/bash
# Locate StepEnvironmentTrait file in the repository
find . -type f -name "StepEnvironmentTrait.php"
# Once located, search for restoreFile definition within that file
FILE=$(find . -type f -name "StepEnvironmentTrait.php")
if [ -n "$FILE" ]; then
rg -n "function restoreFile" -C3 "$FILE"
else
echo "StepEnvironmentTrait.php not found"
fi
# Confirm StepTestingTrait includes StepEnvironmentTrait
rg -n "use .*StepEnvironmentTrait" .vortex/tests/phpunit/Traits/Steps/StepTestingTrait.phpLength of output: 335
🏁 Script executed:
#!/bin/bash
# Locate StepTestingTrait file
FILE=$(find . -type f -name "StepTestingTrait.php")
echo "File: $FILE"
# Check if StepTestingTrait imports StepEnvironmentTrait
rg -n "use .*StepEnvironmentTrait" -C2 "$FILE" || echo "StepTestingTrait does not import StepEnvironmentTrait"
# Confirm restoreFile invocation
rg -n "restoreFile" -C2 "$FILE"Length of output: 329
Add missing StepEnvironmentTrait to StepTestingTrait
StepTestingTrait calls restoreFile() (defined in StepEnvironmentTrait) but doesn’t include that trait. In .vortex/tests/phpunit/Traits/Steps/StepTestingTrait.php add:
use StepEnvironmentTrait;after the other use statements.
🤖 Prompt for AI Agents
.vortex/tests/phpunit/Traits/Steps/StepTestingTrait.php around lines 290-291:
StepTestingTrait calls restoreFile() which is defined in StepEnvironmentTrait
but that trait is not imported; add a `use StepEnvironmentTrait;` line among the
other trait `use` statements in this file (place it immediately after the
existing use lines) so the restoreFile() method is available to
StepTestingTrait.
| implementation: require('sass'), | ||
| sourceMap: false, | ||
| outputStyle: 'compressed' | ||
| style: 'compressed' |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Sass option name likely incorrect — use outputStyle for grunt-sass.
grunt-sass expects outputStyle, not style. Using style may skip minification and break FE tests that assert minified output.
Apply:
- style: 'compressed'
+ outputStyle: 'compressed'Optionally also align dev:
// In sass:dev options:
outputStyle: 'expanded'🏁 Script executed:
#!/bin/bash
echo "package.json files:"
find . -type f -name "package.json"
echo
echo "Searching for 'grunt-sass' and 'grunt-contrib-sass' in each package.json:"
for f in $(find . -type f -name "package.json"); do
echo "=== $f ==="
grep -n '"grunt-sass"' "$f" || echo " grunt-sass not found"
grep -n '"grunt-contrib-sass"' "$f" || echo " grunt-contrib-sass not found"
echo
done
echo "Locating Gruntfile.js:"
find . -type f -name "Gruntfile.js"Length of output: 37760
Replace style with outputStyle for grunt-sass
grunt-sass v0.9+ only recognizes outputStyle; style: 'compressed' is ignored and prevents CSS minification.
In web/themes/custom/your_site_theme/Gruntfile.js (line 97) change:
- style: 'compressed'
+ outputStyle: 'compressed'Also update the dev target to use outputStyle: 'expanded'.
📝 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.
| style: 'compressed' | |
| outputStyle: 'compressed' |
🤖 Prompt for AI Agents
In web/themes/custom/your_site_theme/Gruntfile.js around line 97, the grunt-sass
config uses the deprecated option style: 'compressed' which is ignored by v0.9+
and prevents CSS minification; replace style with outputStyle for both targets
(set outputStyle: 'compressed' for the production/build target) and update the
dev target to use outputStyle: 'expanded' so sourcemaps and readable CSS are
produced.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
.vortex/tests/composer.json (1)
41-41: Avoid unbounded Composer process-timeout (can stall CI).Set a large finite value (e.g., 3600s) instead of 0.
Apply:
- "process-timeout": 0, + "process-timeout": 3600,.vortex/tests/phpunit/Functional/FunctionalTestCase.php (1)
94-103: Delegate to processRun to inherit non-interactive env defaults; also avoid static call.processRunInContainer bypasses the AHOY_* env defaults and uses static::traitProcessRun, making prompts possible and harder to mock.
Apply:
public function processRunInContainer( string $command, array $arguments = [], array $inputs = [], array $env = [], int $timeout = 60, int $idle_timeout = 30, ): Process { - return static::traitProcessRun('ahoy cli -- ' . $command, $arguments, $inputs, $env, $timeout, $idle_timeout); + // Reuse wrapper to apply env defaults and streaming config. + return $this->processRun('ahoy cli -- ' . $command, $arguments, $inputs, $env, $timeout, $idle_timeout); }If you must keep trait direct call, at minimum switch to $this->traitProcessRun.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
.vortex/tests/composer.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
.vortex/tests/composer.json(2 hunks).vortex/tests/phpunit/Functional/FunctionalTestCase.php(4 hunks).vortex/tests/phpunit/Traits/LoggerTrait.php(0 hunks)
💤 Files with no reviewable changes (1)
- .vortex/tests/phpunit/Traits/LoggerTrait.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: build (1)
- GitHub Check: build (0)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-workflow (4)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-docs
🔇 Additional comments (2)
.vortex/tests/composer.json (1)
15-16: Dev deps bump verified. composer.lock shows alexskrypnyk/file@0.9.0, phpunit-helpers@0.9.0, phpunit@12.3.8, and symfony/process@7.3.3 present..vortex/tests/phpunit/Functional/FunctionalTestCase.php (1)
34-36: Good use of trait alias to preserve original ProcessTrait::processRun.
| static::logStepSummary(); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider instance call for logging for easier stubbing.
If LoggerTrait provides an instance method, prefer $this->logStepSummary() over static:: for testability. If it’s intentionally static, ignore.
Apply (only if non-static):
- static::logStepSummary();
+ $this->logStepSummary();📝 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.
| static::logStepSummary(); | |
| } | |
| $this->logStepSummary(); | |
| } |
🤖 Prompt for AI Agents
.vortex/tests/phpunit/Functional/FunctionalTestCase.php around lines 65-66: the
test currently calls static::logStepSummary(); change this to an instance call
$this->logStepSummary() if LoggerTrait defines logStepSummary() as a non-static
method; update any test doubles/setup to stub/spy on $this (or the injected
logger) instead of expecting a static call, and run tests to ensure no static
references remain.
| /** | ||
| * {@inheritdoc} | ||
| */ | ||
| public function processRun( | ||
| string $command, | ||
| array $arguments = [], | ||
| array $inputs = [], | ||
| array $env = [], | ||
| int $timeout = 60, | ||
| int $idle_timeout = 60, | ||
| ): Process { | ||
| $env += [ | ||
| 'AHOY_CONFIRM_RESPONSE' => 'y', | ||
| 'AHOY_CONFIRM_WAIT_SKIP' => 1, | ||
| ]; | ||
|
|
||
| return $this->traitProcessRun($command, $arguments, $inputs, $env, $timeout, $idle_timeout); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Normalize env var types to strings.
Some process runners expect string env values; cast 1 to '1' for consistency.
Apply:
- $env += [
- 'AHOY_CONFIRM_RESPONSE' => 'y',
- 'AHOY_CONFIRM_WAIT_SKIP' => 1,
- ];
+ $env += [
+ 'AHOY_CONFIRM_RESPONSE' => 'y',
+ 'AHOY_CONFIRM_WAIT_SKIP' => '1',
+ ];📝 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.
| /** | |
| * {@inheritdoc} | |
| */ | |
| public function processRun( | |
| string $command, | |
| array $arguments = [], | |
| array $inputs = [], | |
| array $env = [], | |
| int $timeout = 60, | |
| int $idle_timeout = 60, | |
| ): Process { | |
| $env += [ | |
| 'AHOY_CONFIRM_RESPONSE' => 'y', | |
| 'AHOY_CONFIRM_WAIT_SKIP' => 1, | |
| ]; | |
| return $this->traitProcessRun($command, $arguments, $inputs, $env, $timeout, $idle_timeout); | |
| } | |
| /** | |
| * {@inheritdoc} | |
| */ | |
| public function processRun( | |
| string $command, | |
| array $arguments = [], | |
| array $inputs = [], | |
| array $env = [], | |
| int $timeout = 60, | |
| int $idle_timeout = 60, | |
| ): Process { | |
| $env += [ | |
| 'AHOY_CONFIRM_RESPONSE' => 'y', | |
| 'AHOY_CONFIRM_WAIT_SKIP' => '1', | |
| ]; | |
| return $this->traitProcessRun($command, $arguments, $inputs, $env, $timeout, $idle_timeout); | |
| } |
🤖 Prompt for AI Agents
.vortex/tests/phpunit/Functional/FunctionalTestCase.php around lines 75 to 92:
the environment array uses an integer value for 'AHOY_CONFIRM_WAIT_SKIP' which
may cause issues with process runners that expect string env values; change the
value to the string '1' (and ensure any other env values are strings) so the
merged $env contains only string values before passing to traitProcessRun.
| /** | ||
| * Assert that files matching wildcard pattern(s) exist. | ||
| * | ||
| * @param string|array $patterns | ||
| * Wildcard pattern(s) to match files against. | ||
| */ | ||
| protected function assertFilesWildcardExists(string|array $patterns): void { | ||
| $patterns = is_array($patterns) ? $patterns : [$patterns]; | ||
|
|
||
| if (empty($patterns)) { | ||
| throw new \InvalidArgumentException('Empty patterns - no files to check'); | ||
| } | ||
|
|
||
| foreach ($patterns as $pattern) { | ||
| $matches = glob($pattern); | ||
|
|
||
| if ($matches === FALSE) { | ||
| throw new \RuntimeException(sprintf('Failed to read files matching wildcard pattern: %s', $pattern)); | ||
| } | ||
|
|
||
| $this->assertNotEmpty( | ||
| $matches, | ||
| sprintf('No files found matching wildcard pattern: %s', $pattern) | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Wildcard helper: allow flags and brace expansion.
Support advanced patterns and future needs by exposing glob flags (default 0).
Apply:
- protected function assertFilesWildcardExists(string|array $patterns): void {
+ protected function assertFilesWildcardExists(string|array $patterns, int $flags = 0): void {
$patterns = is_array($patterns) ? $patterns : [$patterns];
if (empty($patterns)) {
throw new \InvalidArgumentException('Empty patterns - no files to check');
}
foreach ($patterns as $pattern) {
- $matches = glob($pattern);
+ $matches = glob($pattern, $flags);Example usage in tests: assertFilesWildcardExists('build/{app,assets}/*', GLOB_BRACE).
📝 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.
| /** | |
| * Assert that files matching wildcard pattern(s) exist. | |
| * | |
| * @param string|array $patterns | |
| * Wildcard pattern(s) to match files against. | |
| */ | |
| protected function assertFilesWildcardExists(string|array $patterns): void { | |
| $patterns = is_array($patterns) ? $patterns : [$patterns]; | |
| if (empty($patterns)) { | |
| throw new \InvalidArgumentException('Empty patterns - no files to check'); | |
| } | |
| foreach ($patterns as $pattern) { | |
| $matches = glob($pattern); | |
| if ($matches === FALSE) { | |
| throw new \RuntimeException(sprintf('Failed to read files matching wildcard pattern: %s', $pattern)); | |
| } | |
| $this->assertNotEmpty( | |
| $matches, | |
| sprintf('No files found matching wildcard pattern: %s', $pattern) | |
| ); | |
| } | |
| } | |
| /** | |
| * Assert that files matching wildcard pattern(s) exist. | |
| * | |
| * @param string|array $patterns | |
| * Wildcard pattern(s) to match files against. | |
| * @param int $flags | |
| * Glob flags to control pattern matching (default 0). | |
| */ | |
| protected function assertFilesWildcardExists(string|array $patterns, int $flags = 0): void { | |
| $patterns = is_array($patterns) ? $patterns : [$patterns]; | |
| if (empty($patterns)) { | |
| throw new \InvalidArgumentException('Empty patterns - no files to check'); | |
| } | |
| foreach ($patterns as $pattern) { | |
| $matches = glob($pattern, $flags); | |
| if ($matches === FALSE) { | |
| throw new \RuntimeException(sprintf('Failed to read files matching wildcard pattern: %s', $pattern)); | |
| } | |
| $this->assertNotEmpty( | |
| $matches, | |
| sprintf('No files found matching wildcard pattern: %s', $pattern) | |
| ); | |
| } | |
| } |
🤖 Prompt for AI Agents
.vortex/tests/phpunit/Functional/FunctionalTestCase.php around lines 133 to 158:
the helper currently calls glob($pattern) which prevents using flags like
GLOB_BRACE; change the method signature to accept an optional int $flags = 0
(update the docblock to document $flags) and pass $flags to glob as
glob($pattern, $flags); keep existing behavior when flags are omitted and
preserve the same exceptions and assertions; ensure any test call sites that
need brace expansion pass GLOB_BRACE.
| /** | ||
| * Assert that files matching wildcard pattern(s) do not exist. | ||
| * | ||
| * @param string|array $patterns | ||
| * Wildcard pattern(s) to match files against. | ||
| */ | ||
| protected function assertFilesWildcardDoNotExist(string|array $patterns): void { | ||
| $patterns = is_array($patterns) ? $patterns : [$patterns]; | ||
|
|
||
| if (empty($patterns)) { | ||
| throw new \InvalidArgumentException('Empty patterns - no files to check'); | ||
| } | ||
|
|
||
| foreach ($patterns as $pattern) { | ||
| $matches = glob($pattern); | ||
|
|
||
| if ($matches === FALSE) { | ||
| throw new \RuntimeException(sprintf('Failed to read files matching wildcard pattern: %s', $pattern)); | ||
| } | ||
|
|
||
| $this->assertEmpty( | ||
| $matches, | ||
| sprintf('Found %d file(s) matching wildcard pattern that should not exist: %s', count($matches), $pattern) | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Mirror flags support in negative assertion.
Keep both helpers symmetric.
Apply:
- protected function assertFilesWildcardDoNotExist(string|array $patterns): void {
+ protected function assertFilesWildcardDoNotExist(string|array $patterns, int $flags = 0): void {
$patterns = is_array($patterns) ? $patterns : [$patterns];
if (empty($patterns)) {
throw new \InvalidArgumentException('Empty patterns - no files to check');
}
foreach ($patterns as $pattern) {
- $matches = glob($pattern);
+ $matches = glob($pattern, $flags);📝 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.
| /** | |
| * Assert that files matching wildcard pattern(s) do not exist. | |
| * | |
| * @param string|array $patterns | |
| * Wildcard pattern(s) to match files against. | |
| */ | |
| protected function assertFilesWildcardDoNotExist(string|array $patterns): void { | |
| $patterns = is_array($patterns) ? $patterns : [$patterns]; | |
| if (empty($patterns)) { | |
| throw new \InvalidArgumentException('Empty patterns - no files to check'); | |
| } | |
| foreach ($patterns as $pattern) { | |
| $matches = glob($pattern); | |
| if ($matches === FALSE) { | |
| throw new \RuntimeException(sprintf('Failed to read files matching wildcard pattern: %s', $pattern)); | |
| } | |
| $this->assertEmpty( | |
| $matches, | |
| sprintf('Found %d file(s) matching wildcard pattern that should not exist: %s', count($matches), $pattern) | |
| ); | |
| } | |
| } | |
| /** | |
| * Assert that files matching wildcard pattern(s) do not exist. | |
| * | |
| * @param string|array $patterns | |
| * Wildcard pattern(s) to match files against. | |
| */ | |
| protected function assertFilesWildcardDoNotExist(string|array $patterns, int $flags = 0): void { | |
| $patterns = is_array($patterns) ? $patterns : [$patterns]; | |
| if (empty($patterns)) { | |
| throw new \InvalidArgumentException('Empty patterns - no files to check'); | |
| } | |
| foreach ($patterns as $pattern) { | |
| $matches = glob($pattern, $flags); | |
| if ($matches === FALSE) { | |
| throw new \RuntimeException(sprintf('Failed to read files matching wildcard pattern: %s', $pattern)); | |
| } | |
| $this->assertEmpty( | |
| $matches, | |
| sprintf('Found %d file(s) matching wildcard pattern that should not exist: %s', count($matches), $pattern) | |
| ); | |
| } | |
| } |
🤖 Prompt for AI Agents
.vortex/tests/phpunit/Functional/FunctionalTestCase.php lines 160-185: The
negative wildcard assertion lacks support for glob flags, making it asymmetric
with the positive helper; update the method signature and phpdoc to accept an
optional int $flags = 0, validate patterns as before, and pass $flags into the
glob(...) call (handle the FALSE return as now). Ensure the error messages and
assertions remain the same and only the extra $flags parameter is added and
forwarded to glob.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.vortex/tests/composer.json (1)
41-41: Avoid unbounded Composer process-timeout (set a large finite value).0 can mask hangs and stall CI; prefer 1800–3600s.
- "process-timeout": 0, + "process-timeout": 3600,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
.vortex/tests/composer.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
.vortex/tests/composer.json(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: build (1)
- GitHub Check: build (0)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (4)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-docs
| "alexskrypnyk/file": "^0.9", | ||
| "alexskrypnyk/phpunit-helpers": "main-dev", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Fix invalid Composer constraint for phpunit-helpers; pin dev dependency and verify BC with file ^0.9.
- "main-dev" is not a valid Composer version selector. Use "dev-main" (optionally pinned to a commit) or a tagged release to keep builds reproducible. Also confirm this package is compatible with PHPUnit 12.
- Since you’re depending on a dev branch, consider keeping minimum-stability at stable and using an explicit dev constraint for just this package to reduce churn.
Apply:
- "alexskrypnyk/phpunit-helpers": "main-dev",
+ "alexskrypnyk/phpunit-helpers": "dev-main",🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo
echo "Find composer.json files:"
find . -type f -iname composer.json -print || true
echo
if [ -f ".vortex/tests/composer.json" ]; then
echo "---- .vortex/tests/composer.json ----"
nl -ba .vortex/tests/composer.json | sed -n '1,240p'
else
echo "---- .vortex/tests/composer.json: NOT FOUND ----"
fi
echo
if [ -f "composer.json" ]; then
echo "---- root composer.json ----"
nl -ba composer.json | sed -n '1,240p'
else
echo "---- root composer.json: NOT FOUND ----"
fi
echo
echo "Search for 'alexskrypnyk' in repo:"
if command -v rg >/dev/null 2>&1; then
rg -n "alexskrypnyk" || true
else
grep -RIn --exclude-dir=.git "alexskrypnyk" || true
fi
echo
echo "Search for 'minimum-stability' in repo:"
if command -v rg >/dev/null 2>&1; then
rg -n "\"minimum-stability\"" || true
else
grep -RIn --exclude-dir=.git "\"minimum-stability\"" || true
fiLength of output: 247
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "cwd: $(pwd)"
echo
echo "=== .vortex/tests/composer.json ==="
if [ -f ".vortex/tests/composer.json" ]; then
awk '{printf "%4d: %s\n", NR, $0}' .vortex/tests/composer.json
else
echo "MISSING: .vortex/tests/composer.json"
fi
echo
echo "=== Search for 'alexskrypnyk' across repo (rg fallback to grep) ==="
if command -v rg >/dev/null 2>&1; then
rg -n "alexskrypnyk" || true
else
grep -RIn --exclude-dir=.git "alexskrypnyk" || true
fi
echo
echo "=== Files mentioning minimum-stability or require-dev ==="
if command -v rg >/dev/null 2>&1; then
rg -n "\"minimum-stability\"|\"require-dev\"" || true
else
grep -RIn --exclude-dir=.git -e "\"minimum-stability\"" -e "\"require-dev\"" || true
fiLength of output: 2192
Fix Composer constraint: change main-dev → dev-main (or pin to a tagged release 0.8.2).
- Replace the invalid "main-dev" with "dev-main" — Composer requires branch constraints to be prefixed with "dev-". (getcomposer.org)
- Prefer pinning to a tagged release for reproducible builds (0.8.2 exists); phpunit-helpers declares phpunit ^11 || ^12, so PHPUnit 12 is supported. (packagist.org)
- File to change: .vortex/tests/composer.json (require-dev entry). It currently sets "minimum-stability": "alpha"; if you want to keep minimum-stability "stable", set that at the root and use an explicit dev constraint for only this package or pin the tag.
Apply:
- "alexskrypnyk/phpunit-helpers": "main-dev",
+ "alexskrypnyk/phpunit-helpers": "dev-main",📝 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.
| "alexskrypnyk/file": "^0.9", | |
| "alexskrypnyk/phpunit-helpers": "main-dev", | |
| "alexskrypnyk/file": "^0.9", | |
| "alexskrypnyk/phpunit-helpers": "dev-main", |
🤖 Prompt for AI Agents
.vortex/tests/composer.json lines 15-16: the require-dev constraint
"alexskrypnyk/phpunit-helpers": "main-dev" is invalid and should be changed;
replace "main-dev" with the correct branch format "dev-main" or, preferably, pin
to a stable tag such as "0.8.2" for reproducible builds; if you keep using a dev
branch, use the "dev-" prefix and consider setting an explicit version alias or
adjusting minimum-stability only for this package instead of changing the root
stability.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (6)
.vortex/tests/phpunit/Functional/FunctionalTestCase.php (5)
71-72: Prefer instance call for logStepSummary (repeat).Switch to $this->logStepSummary() for testability and consistency.
- static::logStepSummary(); + $this->logStepSummary();#!/bin/bash rg -nC2 -e 'trait\s+LoggerTrait' -e '\b(static\s+)?function\s+logStepSummary\s*\(' .vortex/tests/phpunit/Traits/LoggerTrait.php
92-95: Normalize env values to strings (repeat).Some runners expect strings; cast 1 to '1'.
$env += [ 'AHOY_CONFIRM_RESPONSE' => 'y', - 'AHOY_CONFIRM_WAIT_SKIP' => 1, + 'AHOY_CONFIRM_WAIT_SKIP' => '1', ];
100-109: Delegate to processRun to reuse env injection and make mocking easier (repeat, plus functional impact).Calling the trait directly bypasses the AHOY_CONFIRM_* env setup and any future hooks in processRun. Delegate instead.
public function processRunInContainer( string $command, array $arguments = [], array $inputs = [], array $env = [], int $timeout = 60, int $idle_timeout = 30, ): Process { - return static::traitProcessRun('ahoy cli -- ' . $command, $arguments, $inputs, $env, $timeout, $idle_timeout); + return $this->processRun('ahoy cli -- ' . $command, $arguments, $inputs, $env, $timeout, $idle_timeout); }#!/bin/bash # Check if processRunInContainer is used anywhere (impact of bypassing env injection). rg -n 'processRunInContainer\s*\('
139-164: Wildcard helper: accept glob flags (repeat).Expose flags (e.g., GLOB_BRACE) for future-proofing.
- protected function assertFilesWildcardExists(string|array $patterns): void { + protected function assertFilesWildcardExists(string|array $patterns, int $flags = 0): void { $patterns = is_array($patterns) ? $patterns : [$patterns]; if (empty($patterns)) { throw new \InvalidArgumentException('Empty patterns - no files to check'); } foreach ($patterns as $pattern) { - $matches = glob($pattern); + $matches = glob($pattern, $flags);
172-191: Mirror flags support in negative assertion (repeat).Keep API symmetry with the positive helper.
- protected function assertFilesWildcardDoNotExist(string|array $patterns): void { + protected function assertFilesWildcardDoNotExist(string|array $patterns, int $flags = 0): void { $patterns = is_array($patterns) ? $patterns : [$patterns]; if (empty($patterns)) { throw new \InvalidArgumentException('Empty patterns - no files to check'); } foreach ($patterns as $pattern) { - $matches = glob($pattern); + $matches = glob($pattern, $flags);.vortex/tests/phpunit/Functional/WorkflowInstallDbTest.php (1)
35-87: Split monolithic workflow test into focused tests (repeat).Diagnosability and flakiness improve when steps are isolated with @Depends.
- public function testDbDrivenWorkflow(): void { - $this->stepBuild(); - $this->stepAhoyLogin(); - // State-less tests. - $this->assertFilesTrackedInGit(); - $this->stepAhoyCli(); - $this->stepEnvChanges(); - $this->stepTimezone(); - $this->stepAhoyComposer(); - $this->stepAhoyDrush(); - $this->stepAhoyInfo(); - $this->stepAhoyContainerLogs(); - // State-full tests. - $this->stepAhoyImportDb(); - $this->stepAhoyExportDb(); - $this->stepAhoyExportDb('mydb.sql'); - $this->stepAhoyImportDb('.data/mydb.sql'); - $this->stepAhoyProvision(); - $this->stepAhoyLint(); - $this->stepAhoyTest(); - $this->stepAhoyFei(); - $this->stepAhoyFe(); - $this->stepAhoyDebug(); - $this->stepSolr(); - // Run this test as a last one to make sure that there is no concurrency - // issues with enabled Valkey. - $this->stepRedis(); - $this->stepAhoyReset(); - $this->stepAhoyResetHard(); - } + public function testStatelessOperations(): void { + $this->stepBuild(); + $this->stepAhoyLogin(); + $this->assertFilesTrackedInGit(); + $this->stepAhoyCli(); + $this->stepEnvChanges(); + $this->stepTimezone(); + $this->stepAhoyComposer(); + $this->stepAhoyDrush(); + $this->stepAhoyInfo(); + $this->stepAhoyContainerLogs(); + } + + #[\PHPUnit\Framework\Attributes\Depends('testStatelessOperations')] + public function testDatabaseOperations(): void { + $this->stepAhoyImportDb(); + $this->stepAhoyExportDb(); + $this->stepAhoyExportDb('mydb.sql'); + $this->stepAhoyImportDb('.data/mydb.sql'); + $this->stepAhoyProvision(); + } + + #[\PHPUnit\Framework\Attributes\Depends('testDatabaseOperations')] + public function testQualityAndTesting(): void { + $this->stepAhoyLint(); + $this->stepAhoyTest(); + } + + #[\PHPUnit\Framework\Attributes\Depends('testQualityAndTesting')] + public function testFrontendAndServices(): void { + $this->stepAhoyFei(); + $this->stepAhoyFe(); + $this->stepAhoyDebug(); + $this->stepSolr(); + // Keep Redis last to avoid concurrency issues. + $this->stepRedis(); + } + + #[\PHPUnit\Framework\Attributes\Depends('testFrontendAndServices')] + public function testCleanup(): void { + $this->stepAhoyReset(); + $this->stepAhoyResetHard(); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
.vortex/tests/composer.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
.vortex/tests/phpunit/Functional/FunctionalTestCase.php(5 hunks).vortex/tests/phpunit/Functional/SelfTest.php(1 hunks).vortex/tests/phpunit/Functional/WorkflowInstallDbTest.php(1 hunks).vortex/tests/phpunit/Functional/WorkflowTest.php(0 hunks).vortex/tests/phpunit/Functional/WorkflowUtilitiesTest.php(0 hunks)
💤 Files with no reviewable changes (2)
- .vortex/tests/phpunit/Functional/WorkflowUtilitiesTest.php
- .vortex/tests/phpunit/Functional/WorkflowTest.php
🧰 Additional context used
🧬 Code graph analysis (3)
.vortex/tests/phpunit/Functional/WorkflowInstallDbTest.php (10)
.vortex/tests/phpunit/Functional/FunctionalTestCase.php (2)
FunctionalTestCase(26-193)setUp(43-61).vortex/tests/phpunit/Traits/Steps/StepDownloadDbTrait.php (1)
stepDownloadDb(22-37).vortex/tests/phpunit/Traits/Steps/StepBuildTrait.php (1)
stepBuild(16-63).vortex/tests/phpunit/Traits/Steps/StepAhoyTrait.php (2)
stepAhoyLogin(106-115)stepAhoyCli(16-38).vortex/tests/phpunit/Traits/AssertFilesTrait.php (1)
assertFilesTrackedInGit(293-322).vortex/tests/phpunit/Traits/Steps/StepEnvironmentTrait.php (2)
stepEnvChanges(30-80)stepTimezone(82-119).vortex/tests/phpunit/Traits/Steps/StepDatabaseTrait.php (3)
stepAhoyImportDb(38-49)stepAhoyExportDb(17-36)stepAhoyProvision(51-123).vortex/tests/phpunit/Traits/Steps/StepTestingTrait.php (2)
stepAhoyLint(17-29)stepAhoyTest(99-114).vortex/tests/phpunit/Traits/Steps/StepFrontendTrait.php (2)
stepAhoyFei(17-32)stepAhoyFe(34-74).vortex/tests/phpunit/Traits/Steps/StepServicesTrait.php (2)
stepSolr(16-25)stepRedis(27-82)
.vortex/tests/phpunit/Functional/FunctionalTestCase.php (1)
.vortex/tests/phpunit/Traits/Steps/StepPrepareSutTrait.php (1)
stepPrepareSut(18-40)
.vortex/tests/phpunit/Functional/SelfTest.php (1)
.vortex/tests/phpunit/Functional/FunctionalTestCase.php (4)
FunctionalTestCase(26-193)setUp(43-61)assertFilesWildcardExists(145-164)assertFilesWildcardDoNotExist(172-191)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: build (1)
- GitHub Check: build (0)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (4)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-docs
🔇 Additional comments (5)
.vortex/tests/phpunit/Functional/FunctionalTestCase.php (2)
34-36: LGTM on ProcessTrait aliasing.Good use of method alias to keep a clean public API while retaining direct access to the trait impl.
56-61: Verify whether logSection is instance or static before switching to $this->logSectionLoggerTrait only "uses" AlexSkrypnyk\PhpunitHelpers\Traits\LoggerTrait and no local logSection is present in the repo — keep static::logSection(...) unless the upstream trait defines a non‑static logSection; if it does, prefer $this->logSection(...) for stubbability/late binding.
Location: .vortex/tests/phpunit/Functional/FunctionalTestCase.php:56–61.
Check upstream signature, e.g.:
rg -n --hidden -S '\bfunction\s+logSection\s*(' vendor || true.vortex/tests/phpunit/Functional/WorkflowInstallDbTest.php (1)
26-30: LGTM on DB download in setUp.Reasonable to seed the workflow early; complements build/provision steps.
.vortex/tests/phpunit/Functional/SelfTest.php (2)
72-105: LGTM: data provider covers positive, negative, mixed, nested, and invalid cases.
142-171: LGTM: negative cases exercise counts and empty-pattern error path.
| protected function setUp(): void { | ||
| parent::setUp(); | ||
|
|
||
| // Create test directory structure for wildcard testing. | ||
| $test_dir = static::$workspace . DIRECTORY_SEPARATOR . 'wildcard_test'; | ||
| File::mkdir($test_dir); | ||
|
|
||
| // Create test files. | ||
| File::dump($test_dir . DIRECTORY_SEPARATOR . 'test1.html', 'test content'); | ||
| File::dump($test_dir . DIRECTORY_SEPARATOR . 'test2.html', 'test content'); | ||
| File::dump($test_dir . DIRECTORY_SEPARATOR . 'test1.log', 'test content'); | ||
| File::dump($test_dir . DIRECTORY_SEPARATOR . 'report.txt', 'test content'); | ||
|
|
||
| // Create subdirectory with files. | ||
| $sub_dir = $test_dir . DIRECTORY_SEPARATOR . 'subdir'; | ||
| File::mkdir($sub_dir); | ||
| File::dump($sub_dir . DIRECTORY_SEPARATOR . 'nested.html', 'test content'); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Avoid heavy parent::setUp for filesystem-only checks.
This test doesn’t need containers/SUT; consider a lighter base or bypassing stepPrepareSut to speed up the suite.
🤖 Prompt for AI Agents
.vortex/tests/phpunit/Functional/SelfTest.php lines 18-35: the test calls
parent::setUp() which performs heavy container/SUT setup unnecessary for pure
filesystem checks; remove or avoid invoking the full parent::setUp() and instead
perform only the minimal initialization needed (create workspace dir or call a
lightweight helper) or explicitly bypass stepPrepareSut (e.g., call the parent
setup alternative that skips SUT initialization or mock/override stepPrepareSut
in this test class) so the test creates the test_dir, files and subdir without
starting containers or SUT components.
| #[DataProvider('dataProviderAssertFilesWildcardExists')] | ||
| public function testAssertFilesWildcardExists(string|array $patterns, bool $should_pass, ?string $expected_exception = NULL, ?string $expected_message = NULL): void { | ||
| // Convert relative patterns to absolute paths. | ||
| $workspace = static::$workspace; | ||
| if (is_array($patterns)) { | ||
| $patterns = array_map(function (string $pattern) use ($workspace): string { | ||
| return $workspace . DIRECTORY_SEPARATOR . $pattern; | ||
| }, $patterns); | ||
| } | ||
| elseif (!empty($patterns)) { | ||
| $patterns = $workspace . DIRECTORY_SEPARATOR . $patterns; | ||
| } | ||
|
|
||
| if ($should_pass) { | ||
| $this->assertFilesWildcardExists($patterns); | ||
| } | ||
| else { | ||
| if (!is_string($expected_exception) || !class_exists($expected_exception)) { | ||
| throw new \RuntimeException('Expected a string, got a ' . gettype($expected_exception)); | ||
| } | ||
| /** @var class-string<\Throwable> $expected_exception */ | ||
| $this->expectException($expected_exception); | ||
| if ($expected_message !== NULL) { | ||
| $this->expectExceptionMessage($expected_message); | ||
| } | ||
| $this->assertFilesWildcardExists($patterns); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use regex for exception message to avoid brittleness.
Actual assertion messages include extra context (e.g., pattern/path); match substrings safely.
- if ($expected_message !== NULL) {
- $this->expectExceptionMessage($expected_message);
- }
+ if ($expected_message !== NULL) {
+ $this->expectExceptionMessageMatches('/' . preg_quote($expected_message, '/') . '/');
+ }📝 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.
| #[DataProvider('dataProviderAssertFilesWildcardExists')] | |
| public function testAssertFilesWildcardExists(string|array $patterns, bool $should_pass, ?string $expected_exception = NULL, ?string $expected_message = NULL): void { | |
| // Convert relative patterns to absolute paths. | |
| $workspace = static::$workspace; | |
| if (is_array($patterns)) { | |
| $patterns = array_map(function (string $pattern) use ($workspace): string { | |
| return $workspace . DIRECTORY_SEPARATOR . $pattern; | |
| }, $patterns); | |
| } | |
| elseif (!empty($patterns)) { | |
| $patterns = $workspace . DIRECTORY_SEPARATOR . $patterns; | |
| } | |
| if ($should_pass) { | |
| $this->assertFilesWildcardExists($patterns); | |
| } | |
| else { | |
| if (!is_string($expected_exception) || !class_exists($expected_exception)) { | |
| throw new \RuntimeException('Expected a string, got a ' . gettype($expected_exception)); | |
| } | |
| /** @var class-string<\Throwable> $expected_exception */ | |
| $this->expectException($expected_exception); | |
| if ($expected_message !== NULL) { | |
| $this->expectExceptionMessage($expected_message); | |
| } | |
| $this->assertFilesWildcardExists($patterns); | |
| } | |
| } | |
| #[DataProvider('dataProviderAssertFilesWildcardExists')] | |
| public function testAssertFilesWildcardExists(string|array $patterns, bool $should_pass, ?string $expected_exception = NULL, ?string $expected_message = NULL): void { | |
| // Convert relative patterns to absolute paths. | |
| $workspace = static::$workspace; | |
| if (is_array($patterns)) { | |
| $patterns = array_map(function (string $pattern) use ($workspace): string { | |
| return $workspace . DIRECTORY_SEPARATOR . $pattern; | |
| }, $patterns); | |
| } | |
| elseif (!empty($patterns)) { | |
| $patterns = $workspace . DIRECTORY_SEPARATOR . $patterns; | |
| } | |
| if ($should_pass) { | |
| $this->assertFilesWildcardExists($patterns); | |
| } | |
| else { | |
| if (!is_string($expected_exception) || !class_exists($expected_exception)) { | |
| throw new \RuntimeException('Expected a string, got a ' . gettype($expected_exception)); | |
| } | |
| /** @var class-string<\Throwable> $expected_exception */ | |
| $this->expectException($expected_exception); | |
| if ($expected_message !== NULL) { | |
| $this->expectExceptionMessageMatches('/' . preg_quote($expected_message, '/') . '/'); | |
| } | |
| $this->assertFilesWildcardExists($patterns); | |
| } | |
| } |
🤖 Prompt for AI Agents
.vortex/tests/phpunit/Functional/SelfTest.php lines 40-67: the test currently
calls expectExceptionMessage with an exact string which is brittle; change it to
use PHPUnit's regex-based exception message assertion instead, constructing a
regex from the provided expected_message (escape it with preg_quote and wrap
with delimiters, and include wildcards if you want substring matching) and call
the regex matcher (e.g., expectExceptionMessageMatches) so the test matches the
message safely even when extra context is present.
| #[DataProvider('dataProviderAssertFilesWildcardDoNotExist')] | ||
| public function testAssertFilesWildcardDoNotExist(string|array $patterns, bool $should_pass, ?string $expected_exception = NULL, ?string $expected_message = NULL): void { | ||
| // Convert relative patterns to absolute paths. | ||
| $workspace = static::$workspace; | ||
| if (is_array($patterns)) { | ||
| $patterns = array_map(function (string $pattern) use ($workspace): string { | ||
| return $workspace . DIRECTORY_SEPARATOR . $pattern; | ||
| }, $patterns); | ||
| } | ||
| elseif (!empty($patterns)) { | ||
| $patterns = $workspace . DIRECTORY_SEPARATOR . $patterns; | ||
| } | ||
|
|
||
| if ($should_pass) { | ||
| $this->assertFilesWildcardDoNotExist($patterns); | ||
| } | ||
| else { | ||
| if (!is_string($expected_exception) || !class_exists($expected_exception)) { | ||
| throw new \RuntimeException('Expected a string, got a ' . gettype($expected_exception)); | ||
| } | ||
| /** @var class-string<\Throwable> $expected_exception */ | ||
| $this->expectException($expected_exception); | ||
| if ($expected_message !== NULL) { | ||
| $this->expectExceptionMessage($expected_message); | ||
| } | ||
| $this->assertFilesWildcardDoNotExist($patterns); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Mirror regex-based message matching in negative helper test.
- if ($expected_message !== NULL) {
- $this->expectExceptionMessage($expected_message);
- }
+ if ($expected_message !== NULL) {
+ $this->expectExceptionMessageMatches('/' . preg_quote($expected_message, '/') . '/');
+ }📝 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.
| #[DataProvider('dataProviderAssertFilesWildcardDoNotExist')] | |
| public function testAssertFilesWildcardDoNotExist(string|array $patterns, bool $should_pass, ?string $expected_exception = NULL, ?string $expected_message = NULL): void { | |
| // Convert relative patterns to absolute paths. | |
| $workspace = static::$workspace; | |
| if (is_array($patterns)) { | |
| $patterns = array_map(function (string $pattern) use ($workspace): string { | |
| return $workspace . DIRECTORY_SEPARATOR . $pattern; | |
| }, $patterns); | |
| } | |
| elseif (!empty($patterns)) { | |
| $patterns = $workspace . DIRECTORY_SEPARATOR . $patterns; | |
| } | |
| if ($should_pass) { | |
| $this->assertFilesWildcardDoNotExist($patterns); | |
| } | |
| else { | |
| if (!is_string($expected_exception) || !class_exists($expected_exception)) { | |
| throw new \RuntimeException('Expected a string, got a ' . gettype($expected_exception)); | |
| } | |
| /** @var class-string<\Throwable> $expected_exception */ | |
| $this->expectException($expected_exception); | |
| if ($expected_message !== NULL) { | |
| $this->expectExceptionMessage($expected_message); | |
| } | |
| $this->assertFilesWildcardDoNotExist($patterns); | |
| } | |
| } | |
| #[DataProvider('dataProviderAssertFilesWildcardDoNotExist')] | |
| public function testAssertFilesWildcardDoNotExist(string|array $patterns, bool $should_pass, ?string $expected_exception = NULL, ?string $expected_message = NULL): void { | |
| // Convert relative patterns to absolute paths. | |
| $workspace = static::$workspace; | |
| if (is_array($patterns)) { | |
| $patterns = array_map(function (string $pattern) use ($workspace): string { | |
| return $workspace . DIRECTORY_SEPARATOR . $pattern; | |
| }, $patterns); | |
| } | |
| elseif (!empty($patterns)) { | |
| $patterns = $workspace . DIRECTORY_SEPARATOR . $patterns; | |
| } | |
| if ($should_pass) { | |
| $this->assertFilesWildcardDoNotExist($patterns); | |
| } | |
| else { | |
| if (!is_string($expected_exception) || !class_exists($expected_exception)) { | |
| throw new \RuntimeException('Expected a string, got a ' . gettype($expected_exception)); | |
| } | |
| /** @var class-string<\Throwable> $expected_exception */ | |
| $this->expectException($expected_exception); | |
| if ($expected_message !== NULL) { | |
| $this->expectExceptionMessageMatches('/' . preg_quote($expected_message, '/') . '/'); | |
| } | |
| $this->assertFilesWildcardDoNotExist($patterns); | |
| } | |
| } |
🤖 Prompt for AI Agents
.vortex/tests/phpunit/Functional/SelfTest.php around lines 110-137: the negative
test currently calls $this->expectExceptionMessage($expected_message) which does
exact-string match, but we need regex-based message matching like the helper;
replace that call with $this->expectExceptionMessageMatches(...) and ensure the
passed value is a valid regex — if $expected_message is not already a regex,
wrap it with delimiters and preg_quote() so it is treated as a literal match.
| // Run this test as a last one to make sure that there is no concurrency | ||
| // issues with enabled Valkey. | ||
| $this->stepRedis(); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Align wording: Redis vs Valkey.
The comment says Valkey but the step targets Redis; pick one consistently (likely “Redis” here).
- // Run this test as a last one to make sure that there is no concurrency
- // issues with enabled Valkey.
+ // Run this test last to avoid concurrency issues related to Redis.📝 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.
| // Run this test as a last one to make sure that there is no concurrency | |
| // issues with enabled Valkey. | |
| $this->stepRedis(); | |
| // Run this test last to avoid concurrency issues related to Redis. | |
| $this->stepRedis(); |
🤖 Prompt for AI Agents
.vortex/tests/phpunit/Functional/WorkflowInstallDbTest.php around lines 80 to
82: the inline comment references "Valkey" while the code calls
$this->stepRedis(); update the comment to consistently refer to "Redis" (e.g.,
"Run this test as a last one to make sure that there are no concurrency issues
with enabled Redis.") so the wording matches the step being executed.
AlexSkrypnyk
changed the title
Closes #1560
Summary by CodeRabbit
Tests
Refactor
Chores