Repository navigation
[#3143] Added an opt-in provision check that fails on errors in the Drupal status report. - #3243
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes.
 |
| Layer / File(s) | Summary |
|---|---|
Configure and run the status-report check .env, .vortex/tooling/src/vortex-provision, .vortex/docs/content/development/provision.mdx, .vortex/docs/content/development/variables.mdx |
The script checks Drupal requirements at error severity when verification is enabled. It fails if the report cannot be read or returns errors. The exclusion setting accepts requirement keys. Verification is disabled by default, and the documentation describes both settings. |
Test verification outcomes .vortex/tooling/tests/unit/provision.bats |
Tests cover returned errors, an unreadable report, excluded requirements, and verification disabled by default. |
Priority:
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Bug fix · Severity of issue fixed: Medium
Sequence Diagram(s)
sequenceDiagram
participant Provision as "vortex-provision"
participant Drush
participant Drupal
Provision->>Drush: Request severity-2 requirements and titles
Drush->>Drupal: Read status-report requirements
Drupal-->>Drush: Return matching requirement titles
Drush-->>Provision: Return titles or command failure
Provision->>Provision: Fail on unreadable output or returned errors
Merge Risk: 
32235
-
The status-report check is opt-in and works as documented, with warnings not failing provisioning. The PR description still says the check is enabled by default, so the author should correct it before merge.
Pre-merge checks: 
-
Check name Status Explanation Resolution Linked Issues check Issue #3143requires coding changes that are met in part. Met: the provision script reads the status report after provisioning and fails on error severity (--severity=2), warnings pass, and failing …Add passing and failing status-report BATS scenarios to the per-script provision test file that #3143names. Cover warnings passing, error titles failing, unreadable report failure, exclusion handling, and the disabled setting. Confirm the …-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Out of Scope Changes check All reported changes connect to #3143. The provision script adds the check and its variables..envandprovision.mdxdefine and document the controls.variables.mdxis the regenerated variables …Docstring Coverage Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Title check The title clearly describes the main change: an opt-in provisioning check that fails when the Drupal status report contains errors. This matches the implementation summary and documented default-disab… Full details: Out of Scope Changes check
-
Explanation
All reported changes connect to
#3143. The provision script adds the check and its variables..envandprovision.mdxdefine and document the controls.variables.mdxis the regenerated variables table that the issue requires.provision.batsadds coverage for the new behavior. The installer fixtures under.vortex/installer/tests/Fixtures/handler_process/are regenerated for the new.enventries. One fixture file is excluded by the ignored-files rule, so its content was not reviewed. No unrelated change is established. The commit message difference about opt-in versus default-on is a documentation and default-value inconsistency. It is not an unrelated change, and it is covered under the linked issue assessment.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 2 files. (3 skipped: 3 unsupported.)
Full details: Title check
-
Explanation
The title clearly describes the main change: an opt-in provisioning check that fails when the Drupal status report contains errors. This matches the implementation summary and documented default-disabled behavior.
-
Full details: Linked Issues check
-
Explanation
Issue
#3143requires coding changes that are met in part. Met: the provision script reads the status report after provisioning and fails on error severity (--severity=2), warnings pass, and failing titles are printed.VORTEX_PROVISION_VERIFY_STATUS_REPORTandVORTEX_PROVISION_VERIFY_STATUS_REPORT_EXCLUDEare defined in.env, documented inprovision.mdx, and present in the regeneratedvariables.mdx.provision.batsadds passing, failing, unreadable, excluded, and disabled scenarios. Unmet: the issue requires BATS coverage in bothprovision.batsand the relevant per-script provision test file, plus unit and functional coverage of passing and failing paths. The change summary lists no per-script provision test file and no functional test change. Also, the PR description states the check is enabled by default (=1in.env). The.env, docs, and script summaries state the default is0(off). This conflict is unresolved. The issue expects provisioning to fail on errors and expects the check to be disabled per project or environment. A default-off check does not meet that expectation.Resolution
Add passing and failing status-report BATS scenarios to the per-script provision test file that
#3143names. Cover warnings passing, error titles failing, unreadable report failure, exclusion handling, and the disabled setting. Confirm the default value ofVORTEX_PROVISION_VERIFY_STATUS_REPORT. Make the.envvalue, the script default, the documentation, the variables table, and the PR description agree.
-
Finishing Touches: 💡 1
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
A rabbit checks the report at dawn,
And spots the errors to act upon.
Warnings pass without a fuss,
Excluded keys stay out of the bus.
The build reports what it has found,
Then hops along on safer ground.
Comment @coderabbitai help to get the list of available commands.
|
📖 Documentation preview for this pull request has been deployed to Netlify: https://6ae3b809e843f775aa98bbb3--vortex-docs.netlify.app This preview is rebuilt on every commit and is not the production documentation site. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
AlexSkrypnyk
changed the title
32235c4 to
50dd272
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1 similar comment
|
Code coverage (threshold: 90%) Per-class coverage |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3243 +/- ##
==========================================
- Coverage 92.28% 92.09% -0.20%
==========================================
Files 113 105 -8
Lines 5097 4931 -166
Branches 49 3 -46
==========================================
- Hits 4704 4541 -163
+ Misses 393 390 -3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Code coverage (threshold: 90%) Per-class coverage |
AlexSkrypnyk
added
the
Needs review
Closes #3143
Summary
vortex-provisioncan now read the Drupal status report after its last operation and fail when any requirement has the error severity. WithVORTEX_PROVISION_VERIFY_STATUS_REPORT=1, it runsdrush core:requirements --severity=2 --field=title --format=listonce maintenance mode is off and the optional configuration check has passed.Provisioning used to report success based on its own steps finishing, so a site with a pending schema update, a files directory that isn't writable or a missing PHP extension built green. The failure output now lists the title of each failing requirement, and a non-zero exit from Drush fails with
Unable to read the Drupal status report.instead of passing on empty output. Warnings never fail the run.The check is opt-in (
0in.envand in the script), like the otherVORTEX_PROVISION_VERIFY_*checks: a freshly provisioned template site reports errors for cron that has never run and for XML Sitemap, so enabling it by default fails every build.VORTEX_PROVISION_VERIFY_STATUS_REPORT_EXCLUDEtakes a comma-separated list of requirement keys passed to--ignore, so a project can accept known errors and keep the check on. Runs that skip post-provision operations (VORTEX_PROVISION_POST_OPERATIONS_SKIP=1, including the profile fallback) never reach the check.Before / After
Changes
Provision script
.vortex/tooling/src/vortex-provisionadds the 2 variables, aVerify status reportline in the settings summary and the verification step.Configuration and docs
.envdocumentsVORTEX_PROVISION_VERIFY_STATUS_REPORT=0and an emptyVORTEX_PROVISION_VERIFY_STATUS_REPORT_EXCLUDE..vortex/docs/content/development/provision.mdxdescribes both variables, and the regeneratedvariables.mdxtable carries them.Tests and fixtures
.vortex/tooling/tests/unit/provision.batsadds 4 scenarios: errors found, status report unreadable, errors excluded and check off by default..vortex/installer/tests/Fixtures/handler_process/are regenerated for the new.enventries.