Bail early when check/dump commands are missing - #339
Conversation
📝 WalkthroughWalkthroughDatabase check, optimize, repair, and export now validate resolved MySQL or MariaDB client binaries and invoke them directly. Acceptance tests update debug-output patterns to match direct executable paths. ChangesDatabase client command resolution
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@features/db-check.feature`:
- Around line 157-163: Add Behat scenarios covering missing database-check
binaries: in features/db-check.feature lines 157-163, disable both mysqlcheck
and mariadb-check and assert the expected error. Also update
features/db-export.feature lines 143-149 with the corresponding scenario
disabling both dump binaries and asserting its expected error, following the
existing feature-test patterns.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 611f562b-5a0a-4687-a99c-355ee1304c80
📒 Files selected for processing (3)
features/db-check.featurefeatures/db-export.featuresrc/DB_Command.php
| Then STDERR should match #Debug \(db\): Running shell command: /([^/]+/)+(mysqlcheck|mariadb-check) %s# | ||
|
|
||
| When I try `wp db check --debug` | ||
| Then STDERR should match #Debug \(db\): Running shell command: /usr/bin/env (mysqlcheck|mariadb-check) --no-defaults %s# | ||
| Then STDERR should match #Debug \(db\): Running shell command: /([^/]+/)+(mysqlcheck|mariadb-check) --no-defaults %s# | ||
|
|
||
| When I try `wp db check --no-defaults --debug` | ||
| Then STDERR should match #Debug \(db\): Running shell command: /usr/bin/env (mysqlcheck|mariadb-check) --no-defaults %s# | ||
| Then STDERR should match #Debug \(db\): Running shell command: /([^/]+/)+(mysqlcheck|mariadb-check) --no-defaults %s# |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Cover the new missing-binary error paths with Behat tests.
features/db-check.feature#L157-L163: add a scenario that disables both check binaries and asserts the expected error.features/db-export.feature#L143-L149: add a scenario that disables both dump binaries and asserts the expected error.
As per coding guidelines, all new features and bug fixes must include Behat acceptance tests following existing feature-test patterns.
📍 Affects 2 files
features/db-check.feature#L157-L163(this comment)features/db-export.feature#L143-L149
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@features/db-check.feature` around lines 157 - 163, Add Behat scenarios
covering missing database-check binaries: in features/db-check.feature lines
157-163, disable both mysqlcheck and mariadb-check and assert the expected
error. Also update features/db-export.feature lines 143-149 with the
corresponding scenario disabling both dump binaries and asserting its expected
error, following the existing feature-test patterns.
Source: Coding guidelines
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Bails early with an error in
db check,db optimize,db repair, anddb exportwhenget_sql_check_command()orget_sql_dump_command()returns an empty string because the binaries are missing.Follow-up to wp-cli/wp-cli#6363.
Summary by CodeRabbit