[#2444] Added 'CodeCoverageProvider' installer prompt with Codecov option.#2464
Merged
AlexSkrypnyk merged 3 commits intomainfrom Apr 20, 2026
Merged
[#2444] Added 'CodeCoverageProvider' installer prompt with Codecov option.#2464AlexSkrypnyk merged 3 commits intomainfrom
AlexSkrypnyk merged 3 commits intomainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
WalkthroughAdds a Code Coverage Provider prompt to the Vortex installer (options: Changes
Sequence DiagramsequenceDiagram
participant Installer as Installer
participant PromptMgr as Prompt Manager
participant Handler as CodeCoverageProvider Handler
participant CIConfig as CI Config Files (GH Actions / CircleCI)
Installer->>PromptMgr: init prompts
PromptMgr->>Handler: discover()
Handler->>CIConfig: scan workflows / circleci config for Codecov
CIConfig-->>Handler: detection result
Handler-->>PromptMgr: discovery result (codecov / none / null)
PromptMgr->>Installer: present selection (default based on discovery)
Installer-->>PromptMgr: user selection
PromptMgr->>Handler: process(selection)
Handler->>CIConfig: add/remove Vortex markers (`CODE_COVERAGE_PROVIDER_CODECOV`)
CIConfig-->>Handler: files updated
Handler-->>PromptMgr: processing complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.vortex/installer/src/Prompts/Handlers/CodeCoverageProvider.php:
- Around line 54-62: The current detection in CodeCoverageProvider.php is too
narrow—replace the hardcoded checks against a single workflow file ($gha) and a
specific CircleCI substring ($circle) with broader searches: scan all files
under .github/workflows/*.y*ml for any occurrence of "codecov" (or
"codecov/codecov-action") and check .circleci/config.yml (if present) for the
substring "codecov" rather than the exact "codecov -Z -s"; use glob() to
enumerate workflow files and File::contains() or similar to search contents, and
keep returning self::CODECOV when any match is found so existing Codecov setups
aren’t silently missed.
In
@.vortex/installer/tests/Functional/Handlers/CodeCoverageProviderHandlerProcessTest.php:
- Around line 14-23: Add a new data provider case in dataProviderHandlerProcess
to exercise the "none" branch: yield a case named like
'code_coverage_provider_none' that uses the same static::cw helper and sets
$test->prompts[CodeCoverageProvider::id()] to the "none" constant (e.g.,
CodeCoverageProvider::NONE) so the handler's else-branch is executed; keep the
pattern consistent with the existing 'codecov' yields and include no CiProvider
prompt for this scenario.
In
@.vortex/installer/tests/Unit/Handlers/CodeCoverageProviderHandlerDiscoveryTest.php:
- Line 63: The invalid discovery test's setup closure currently accepts only one
parameter (AbstractHandlerDiscoveryTestCase $test) but the test harness calls
setup callbacks with two arguments (the test instance and a Config object);
update the closure used in CodeCoverageProviderHandlerDiscoveryTest's "invalid"
case to accept two parameters (e.g., function(AbstractHandlerDiscoveryTestCase
$test, Config $config): void) and, if necessary, import or fully qualify the
Config type and adjust the closure body to use or ignore the $config parameter
so the arity matches the harness invocation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 334913e8-0f67-4144-acb7-64b65ad5e0aa
⛔ Files ignored due to path filters (69)
.vortex/installer/tests/Fixtures/handler_process/_baseline/.github/workflows/build-test-deploy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/_baseline/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/ciprovider_circleci/.circleci/config.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/ciprovider_circleci/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/code_coverage_provider_codecov/.github/workflows/build-test-deploy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/code_coverage_provider_codecov/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/code_coverage_provider_codecov_circleci/.circleci/config.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/code_coverage_provider_codecov_circleci/.circleci/post-coverage-comment.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/code_coverage_provider_codecov_circleci/.github/workflows/-build-test-deploy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/code_coverage_provider_codecov_circleci/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/code_coverage_provider_codecov_circleci/docs/ci.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/code_coverage_provider_codecov_circleci/tests/phpunit/CircleCiConfigTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/code_coverage_provider_codecov_circleci/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/code_coverage_provider_codecov_circleci/web/sites/default/includes/providers/-settings.gha.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/code_coverage_provider_codecov_circleci/web/sites/default/includes/providers/settings.circleci.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/deploy_types_all_circleci/.circleci/config.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/deploy_types_all_circleci/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/deploy_types_none_circleci/.circleci/config.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/deploy_types_none_circleci/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/deploy_types_none_gha/.github/workflows/build-test-deploy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/deps_updates_provider_ci_circleci/.circleci/config.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/deps_updates_provider_ci_circleci/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/deps_updates_provider_none/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/hosting_acquia/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/hosting_lagoon/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/hosting_project_name___acquia/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/hosting_project_name___lagoon/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_disabled_circleci/.circleci/config.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_disabled_circleci/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_disabled_lagoon/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_enabled_circleci/.circleci/config.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_enabled_circleci/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_enabled_lagoon/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/names/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/non_interactive_config_file/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/non_interactive_config_string/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/provision_database_lagoon/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/provision_profile/.github/workflows/build-test-deploy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/provision_profile/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/timezone_circleci/.circleci/config.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/timezone_circleci/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_groups_no_be_lint_circleci/.circleci/config.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_groups_no_be_lint_circleci/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_groups_no_be_tests/.github/workflows/build-test-deploy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_groups_no_be_tests_circleci/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_groups_no_fe_lint_circleci/.circleci/config.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_groups_no_fe_lint_circleci/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_groups_no_fe_lint_no_theme_circleci/.circleci/config.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_groups_no_fe_lint_no_theme_circleci/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_behat/.github/workflows/build-test-deploy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_behat_circleci/.circleci/config.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_behat_circleci/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_eslint_circleci/.circleci/config.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_eslint_circleci/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_jest_circleci/.circleci/config.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_jest_circleci/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_phpcs_circleci/.circleci/config.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_phpcs_circleci/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_phpmd_circleci/.circleci/config.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_phpmd_circleci/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_phpstan_circleci/.circleci/config.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_phpstan_circleci/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_phpunit/.github/workflows/build-test-deploy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_phpunit_circleci/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_rector_circleci/.circleci/config.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_rector_circleci/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_stylelint_circleci/.circleci/config.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_no_stylelint_circleci/README.mdis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/tools_none/.github/workflows/build-test-deploy.ymlis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (9)
.circleci/config.yml.github/workflows/build-test-deploy.yml.vortex/installer/src/Prompts/Handlers/CodeCoverageProvider.php.vortex/installer/src/Prompts/PromptManager.php.vortex/installer/tests/Functional/Handlers/CodeCoverageProviderHandlerProcessTest.php.vortex/installer/tests/Unit/Handlers/AbstractHandlerDiscoveryTestCase.php.vortex/installer/tests/Unit/Handlers/AbstractHandlerTypeTest.php.vortex/installer/tests/Unit/Handlers/CodeCoverageProviderHandlerDiscoveryTest.phpREADME.dist.md
Comment on lines
+14
to
+23
| public static function dataProviderHandlerProcess(): \Iterator { | ||
| yield 'code_coverage_provider_codecov' => [ | ||
| static::cw(fn($test): string => $test->prompts[CodeCoverageProvider::id()] = CodeCoverageProvider::CODECOV), | ||
| ]; | ||
| yield 'code_coverage_provider_codecov_circleci' => [ | ||
| static::cw(function ($test): void { | ||
| $test->prompts[CodeCoverageProvider::id()] = CodeCoverageProvider::CODECOV; | ||
| $test->prompts[CiProvider::id()] = CiProvider::CIRCLECI; | ||
| }), | ||
| ]; |
There was a problem hiding this comment.
Add an explicit none process scenario to cover the else-branch.
This provider has two process paths; this test currently validates only codecov variants.
✅ Suggested test addition
public static function dataProviderHandlerProcess(): \Iterator {
+ yield 'code_coverage_provider_none' => [
+ static::cw(function ($test): void {
+ $test->prompts[CodeCoverageProvider::id()] = CodeCoverageProvider::NONE;
+ }),
+ ];
+
yield 'code_coverage_provider_codecov' => [
static::cw(fn($test): string => $test->prompts[CodeCoverageProvider::id()] = CodeCoverageProvider::CODECOV),
];📝 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.
Suggested change
| public static function dataProviderHandlerProcess(): \Iterator { | |
| yield 'code_coverage_provider_codecov' => [ | |
| static::cw(fn($test): string => $test->prompts[CodeCoverageProvider::id()] = CodeCoverageProvider::CODECOV), | |
| ]; | |
| yield 'code_coverage_provider_codecov_circleci' => [ | |
| static::cw(function ($test): void { | |
| $test->prompts[CodeCoverageProvider::id()] = CodeCoverageProvider::CODECOV; | |
| $test->prompts[CiProvider::id()] = CiProvider::CIRCLECI; | |
| }), | |
| ]; | |
| public static function dataProviderHandlerProcess(): \Iterator { | |
| yield 'code_coverage_provider_none' => [ | |
| static::cw(function ($test): void { | |
| $test->prompts[CodeCoverageProvider::id()] = CodeCoverageProvider::NONE; | |
| }), | |
| ]; | |
| yield 'code_coverage_provider_codecov' => [ | |
| static::cw(fn($test): string => $test->prompts[CodeCoverageProvider::id()] = CodeCoverageProvider::CODECOV), | |
| ]; | |
| yield 'code_coverage_provider_codecov_circleci' => [ | |
| static::cw(function ($test): void { | |
| $test->prompts[CodeCoverageProvider::id()] = CodeCoverageProvider::CODECOV; | |
| $test->prompts[CiProvider::id()] = CiProvider::CIRCLECI; | |
| }), | |
| ]; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
@.vortex/installer/tests/Functional/Handlers/CodeCoverageProviderHandlerProcessTest.php
around lines 14 - 23, Add a new data provider case in dataProviderHandlerProcess
to exercise the "none" branch: yield a case named like
'code_coverage_provider_none' that uses the same static::cw helper and sets
$test->prompts[CodeCoverageProvider::id()] to the "none" constant (e.g.,
CodeCoverageProvider::NONE) so the handler's else-branch is executed; keep the
pattern consistent with the existing 'codecov' yields and include no CiProvider
prompt for this scenario.
Member
Author
There was a problem hiding this comment.
Not adding this scenario. NONE is the default for this handler, so the baseline fixture already exercises the else-branch end-to-end (no codecov content anywhere in the generated project). A code_coverage_provider_none fixture would be a zero-diff copy of _baseline/ and add no coverage on top of what the baseline snapshot tests already assert.
If the default later flips to CODECOV, that assumption no longer holds and the none fixture becomes meaningful - happy to add it at that point.
There was a problem hiding this comment.
@AlexSkrypnyk, that's a clear and reasonable explanation — the baseline fixture serving as the implicit none coverage makes perfect sense given NONE is the default. I'll withdraw the suggestion.
(ᵔᴥᵔ)
✏️ Learnings added
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 2464
File: .vortex/installer/tests/Functional/Handlers/CodeCoverageProviderHandlerProcessTest.php:14-23
Timestamp: 2026-04-20T01:43:31.229Z
Learning: In `.vortex/installer/tests/Functional/Handlers/`, the `_baseline/` fixture implicitly exercises the "else-branch" (i.e. default value) of every handler. For `CodeCoverageProvider` (in `CodeCoverageProviderHandlerProcessTest.php`), `NONE` is the default, so no explicit `code_coverage_provider_none` fixture is needed — it would be a zero-diff copy of the baseline. A dedicated `none` fixture only becomes meaningful if the default is later changed to `CODECOV`.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 2011
File: .vortex/tests/phpunit/Traits/Steps/StepAhoyTrait.php:20-23
Timestamp: 2025-09-13T04:14:41.765Z
Learning: In .vortex/tests/phpunit PHPUnit tests using cmd/cmdFail helpers, patterns in the expected output arrays support '*' markers for positive assertions (text must be present) and '!' markers for negative assertions (text must NOT be present). The '*' markers are NOT literal text matches but rather indicate expected presence.
| yield 'code coverage provider - discovery - invalid' => [ | ||
| [], | ||
| $expected_defaults, | ||
| function (AbstractHandlerDiscoveryTestCase $test): void { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Confirm how setup callbacks are invoked in the shared discovery harness.
rg -n -C4 --type=php 'call_user_func|->\(|\$config|dataProviderRunPrompts|\[\s*2\s*\]' \
.vortex/installer/tests/Unit/Handlers/AbstractHandlerDiscoveryTestCase.php
# 2) Compare callback signatures in this test file.
rg -n -C1 --type=php 'function \(AbstractHandlerDiscoveryTestCase \$test' \
.vortex/installer/tests/Unit/Handlers/CodeCoverageProviderHandlerDiscoveryTest.phpRepository: drevops/vortex
Length of output: 2927
Fix callback arity mismatch in the invalid discovery case.
The setup callback at line 63 accepts only one parameter, but the test harness (AbstractHandlerDiscoveryTestCase) invokes all setup callbacks with two arguments: the test instance and a Config object (lines 99 and 115). This will cause an ArgumentCountError at runtime.
Proposed fix
- function (AbstractHandlerDiscoveryTestCase $test): void {
+ function (AbstractHandlerDiscoveryTestCase $test, Config $config): void {
// No CI config and not installed - 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.
Suggested change
| function (AbstractHandlerDiscoveryTestCase $test): void { | |
| function (AbstractHandlerDiscoveryTestCase $test, Config $config): void { | |
| // No CI config and not installed - fall back to default. | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
@.vortex/installer/tests/Unit/Handlers/CodeCoverageProviderHandlerDiscoveryTest.php
at line 63, The invalid discovery test's setup closure currently accepts only
one parameter (AbstractHandlerDiscoveryTestCase $test) but the test harness
calls setup callbacks with two arguments (the test instance and a Config
object); update the closure used in CodeCoverageProviderHandlerDiscoveryTest's
"invalid" case to accept two parameters (e.g.,
function(AbstractHandlerDiscoveryTestCase $test, Config $config): void) and, if
necessary, import or fully qualify the Config type and adjust the closure body
to use or ignore the $config parameter so the arity matches the harness
invocation.
Member
Author
There was a problem hiding this comment.
False positive - no action taken.
PHP closures silently discard extra positional arguments; calling a 1-parameter closure with 2 arguments does not raise ArgumentCountError. ArgumentCountError only fires when fewer arguments are passed than the function requires.
Existing tests in this codebase use the exact same one-arg signature on the invalid case and pass:
.vortex/installer/tests/Unit/Handlers/PullRequestHandlerDiscoveryTest.phpline 50.vortex/installer/tests/Unit/Handlers/DependencyUpdatesProviderHandlerDiscoveryTest.phpline 63
The full unit suite (1336 tests) is green on this branch, which empirically confirms the invocation pattern is valid.
There was a problem hiding this comment.
@AlexSkrypnyk, you're absolutely right — I apologize for the noise. PHP closures silently discard extra positional arguments passed beyond what the closure declares; ArgumentCountError is only thrown when the caller provides fewer arguments than the closure requires. The 1-parameter signature on the invalid case is perfectly valid, and the existing test precedents in the codebase confirm this.
I'll record this to avoid repeating the mistake in future reviews.
✏️ Learnings added
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 2464
File: .vortex/installer/tests/Unit/Handlers/CodeCoverageProviderHandlerDiscoveryTest.php:63-63
Timestamp: 2026-04-20T01:43:38.833Z
Learning: In PHP, calling a closure (or any function) with *more* positional arguments than it declares does NOT raise an `ArgumentCountError` — the extra arguments are silently discarded. `ArgumentCountError` is only thrown when *fewer* arguments are provided than the function requires. In the drevops/vortex test harness (e.g., `AbstractHandlerDiscoveryTestCase`), setup callbacks for discovery tests are invoked with two arguments (test instance + Config object), but individual test cases may legitimately declare only one parameter (e.g., `function (AbstractHandlerDiscoveryTestCase $test): void`) without causing any runtime error. Do not flag this as an arity mismatch.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 2011
File: .vortex/tests/phpunit/Traits/Steps/StepAhoyTrait.php:20-23
Timestamp: 2025-09-13T04:14:41.765Z
Learning: In .vortex/tests/phpunit PHPUnit tests using cmd/cmdFail helpers, patterns in the expected output arrays support '*' markers for positive assertions (text must be present) and '!' markers for negative assertions (text must NOT be present). The '*' markers are NOT literal text matches but rather indicate expected presence.
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 2446
File: jest.config.js:34-34
Timestamp: 2026-04-02T07:57:37.453Z
Learning: In the drevops/vortex repository, all coverage reports follow the convention `.logs/coverage/<tool>` (e.g. `.logs/coverage/jest` for Jest, `.logs/coverage/phpunit` for PHPUnit). The CI PHPUnit coverage extraction paths (e.g. `/tmp/artifacts/coverage/phpunit/cobertura.xml`) are specific to PHPUnit. Jest CI coverage integration is separate and will be configured independently. Do not flag `.logs/coverage/jest` as misaligned with PHPUnit CI artifact paths.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2464 +/- ##
==========================================
- Coverage 79.88% 79.44% -0.44%
==========================================
Files 128 122 -6
Lines 6861 6729 -132
Branches 47 3 -44
==========================================
- Hits 5481 5346 -135
- Misses 1380 1383 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Code coverage (threshold: 90%) Per-class coverage |
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
Member
Author
|
Code coverage (threshold: 90%) Per-class coverage |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2444
Summary
Adds a
CodeCoverageProviderSelect handler to the Vortex installer under the "Automations" section, letting users choose betweenCodecovandNone(default). WhenCodecovis selected, the Codecov upload step is preserved in both.github/workflows/build-test-deploy.ymland.circleci/config.yml; otherwise those blocks are stripped. The Codecov badge inREADME.mdfollows the same token. Discovery auto-detects existing Codecov usage in upgraded projects so they are not silently downgraded.Changes
New handler (
.vortex/installer/src/Prompts/Handlers/CodeCoverageProvider.php):options():codecov/none, defaultnonediscover(): inspects GHA and CircleCI configs for existing Codecov referencesprocess(): callsFile::removeTokenAsyncwith or without!prefix to keep or strip theCODE_COVERAGE_PROVIDER_CODECOVtoken blockToken wrapping in CI configs:
.github/workflows/build-test-deploy.yml- Codecov upload step wrapped in#;< CODE_COVERAGE_PROVIDER_CODECOV/#;> CODE_COVERAGE_PROVIDER_CODECOV.circleci/config.yml- same token wrapping around thecodecov -Z -supload stepInstaller registration (
.vortex/installer/src/Prompts/PromptManager.php):DependencyUpdatesProviderTests:
CodeCoverageProviderHandlerProcessTest- process scenariosCodeCoverageProviderHandlerDiscoveryTest- discovery scenarios (GHA, CircleCI, both, neither)code_coverage_provider_codecovwith full GHA + CircleCI snapshotsnonedefault)README.dist.md: Codecov badge wrapped in the same token so it appears only when Codecov is selected.
Before / After
Installer "Automations" section:
Token wrapping in CI configs:
Installer output when
noneis selected - Codecov block is removed entirely.Installer output when
codecovis selected - Codecov block is preserved as-is.Summary by CodeRabbit
New Features
Tests
Documentation
Chores