test(installer): cover required-extension helpers (stacked on #62)#63
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #63 +/- ##
=========================================
Coverage 18.12% 18.13%
Complexity 7854 7854
=========================================
Files 666 666
Lines 43136 43208 +72
=========================================
+ Hits 7820 7837 +17
- Misses 35316 35371 +55 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds installer required-extension gating (stacked on #62) and introduces PHPUnit coverage for the two helper functions used to detect missing required extensions and render the “blocked install” alert.
Changes:
- Adds
InstallerRequiredExtensionsTestto coverxoInstallerExtensionAvailable()andxoInstallerBlockedHtml(). - Adds/uses a required-extensions config list to block progression when mandatory PHP extensions/symbols are missing, including server-side guards on DB pages.
- Adds installer language constants documenting the “missing required extensions” message (and records the change in
docs/lang_diff.txt).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/htdocs/install/InstallerRequiredExtensionsTest.php | New unit tests covering required-extension probing and blocked-alert HTML/escaping behavior. |
| htdocs/install/page_modcheck.php | Collects missing required extensions, shows a danger alert, and sets a $blockNext flag; aligns MySQLi row with the configured symbol list. |
| htdocs/install/page_dbsettings.php | Fails closed early (renders blocked alert) if MySQLi is not available before using any mysqli_* APIs. |
| htdocs/install/page_dbconnection.php | Same MySQLi fail-closed guard before attempting DB connection logic. |
| htdocs/install/language/english/install.php | Adds MISSING_REQUIRED_EXTENSIONS / _MSG language constants used by the installer UI. |
| htdocs/install/include/install_tpl.php | Disables the “Next” button when $blockNext is set. |
| htdocs/install/include/functions.php | Adds xoInstallerExtensionAvailable() and xoInstallerBlockedHtml() helper functions. |
| htdocs/install/include/config.php | Defines $configs['extensions_required'] with required extensions and required symbol lists (e.g., MySQLi partial-build detection). |
| docs/lang_diff.txt | Documents the new installer language defines. |
| // The extension is missing, so the function short-circuits on | ||
| // extension_loaded() before any symbol probe — and even when it | ||
| // does probe, class_exists(.., false) must not autoload. |
Copilot review on XOOPS#63: the comment claimed the call short-circuits on extension_loaded(), but json is loaded — the function reaches the symbol loop and the point is that class_exists(.., false) probes the missing class without invoking a registered autoloader. Comment-only change.
| // Keyed by extension name; value is the human-readable label shown on the | ||
| // requirements page. mysqli has no fallback driver, so a missing entry here | ||
| // would otherwise fatal later in the DB-connection step; the others are core | ||
| // extensions with no polyfill. mbstring is intentionally NOT listed: the | ||
| // installer autoloads symfony/polyfill-mbstring (via common.inc.php), so the | ||
| // mb_* functions remain available even without the extension — gating on |
| /** | ||
| * Whether a mandatory extension is usable. | ||
| * | ||
| * For extensions with no fallback driver (e.g. mysqli) a plain | ||
| * extension_loaded() is enough, but a partial/unusual build can report the |
Unit tests for the two pure helpers added in XOOPS#62: - xoInstallerExtensionAvailable(): loaded/unknown extension, optional symbol list (all present, one missing), and that a class-symbol probe does not trigger the autoloader (class_exists(.., false)). - xoInstallerBlockedHtml(): heading/label content and HTML escaping of the label. functions.php is pure declarations so it is required once directly; installer language constants are stub-defined in setUp().
Copilot review on XOOPS#63: the comment claimed the call short-circuits on extension_loaded(), but json is loaded — the function reaches the symbol loop and the point is that class_exists(.., false) probes the missing class without invoking a registered autoloader. Comment-only change.
XOOPS#62 is merged; this branch is now rebased onto master so the diff is test-only. Extend the suite to cover the helper XOOPS#62 added after this branch was originally cut: empty when all present, reports absent extensions, honours the per-extension symbol list, and preserves config order. Uses a real XoopsInstallWizard (required by the parameter type hint; class is guard-free and constructor-free).
mambax7
force-pushed
the
test/installer-required-extensions
branch
from
f7e9c6f to
0a474f0
Compare
|
Add Installer and Tests subsections under 2.7.0 Final for the work merged today: - #60 guard optional function calls that fatal on PHP 8.2 (mysqli_get_client_info(), Protector config cache) - #62 block install with a clear message when a mandatory PHP extension is missing, plus full required-extension guards on the DB steps - #63 unit coverage for the installer required-extension helpers
Summary
Adds unit coverage for the pure installer helpers introduced in #62, addressing the Codecov patch-coverage gap on that PR.
xoInstallerExtensionAvailable()— loaded vs. unknown extension; optional symbol list (all present / one missing); verifies a class-symbol probe does not trigger the autoloader (class_exists(.., false)).xoInstallerBlockedHtml()— heading/label content and HTML escaping.htdocs/install/include/functions.phpis pure function declarations (no top-level code), so it isrequire_once'd directly; installer language constants are stub-defined insetUp().This branch is cut from
fix/installer-required-extensions-gate(#62), not frommaster. Consequently:mastercurrently shows all of Fix/installer required extensions gate #62's production files as well. Those are inherited from Fix/installer required extensions gate #62, not authored here. The net-new delta of this PR is a single file:tests/unit/htdocs/install/InstallerRequiredExtensionsTest.php.masteruntil Fix/installer required extensions gate #62 merges). Because the branch carries Fix/installer required extensions gate #62's code, CI here is green (PHP 8.2–8.5, Sonar, Scrutinizer, CodeRabbit, codecov/patch all pass).config.php/functions.php) belong to Fix/installer required extensions gate #62 and are tracked/resolved there — do not re-fix them here.Merge order
master— the diff then collapses to the single test file, and stale Fix/installer required extensions gate #62-inherited review threads disappear.xoInstallerMissingRequired()test will be added during that rebase to cover the helper Fix/installer required extensions gate #62 added after this branch was cut.)codecov/projectis the only red check; it is the known non-blocking project-wide threshold and is expected to clear once the rebase removes #62's inherited production diff from this PR's coverage delta.Verification
PHPUnit is not installable in the dev environment (CI-only toolchain), so each assertion was independently validated with an equivalent plain-PHP harness against the #62 code (all checks pass); the test file lints clean. CI on this branch additionally runs the real PHPUnit suite green.