test: expand Unit/Integration coverage for lifecycle, menus, and schema - #64
Open
TheWitness wants to merge 1 commit into
Open
TheWitness wants to merge 1 commit into
TheWitness wants to merge 1 commit into
Conversation
This plugin already had thorough Unit coverage for plugin_maint_check_host()/plugin_maint_check_schedule() (the actual maintenance-window logic) plus extensive Security coverage. The gap was setup.php's plugin lifecycle and setup functions, which were only checked for static existence. Closed that gap: - Unit: plugin_maint_version(), plugin_maint_uninstall(), plugin_maint_check_config()/plugin_maint_upgrade(), maint_config_arrays() (menu registration), maint_draw_navigation_text(), maint_device_action_array() (dropdown action registration), and maint_setup_database() (verifies both tables it provisions). - Integration: plugin_maint_install() (verifies every hook, the realm, and the tables it provisions, together, in one pass) - this plugin had no tests/Integration category before. No new unserialize() hardening was needed: every unserialize() call in this plugin already goes through sanitize_unserialize_selected_items(). Intentionally left uncovered: maint_device_edit_top_links() (print-only, depends on real request context), maint_device_action_prepare() (large print-heavy confirmation-dialog renderer), and maint_device_action_execute() (creates/updates real schedule rows with substantial branching) are left uncovered as comparatively low-value relative to the effort of safely mocking their dependencies, consistent with similarly-scoped functions skipped elsewhere in this coverage-expansion pass. tests/bootstrap-unit.php gained api_plugin_register_hook/ api_plugin_register_realm call-logging stubs and upgraded api_plugin_db_table_create to log its calls, reusing this repo's existing maint_test_queue()/maint_test_next() fixture convention where applicable. phpunit.xml now includes tests/Integration in the testsuite. Verified via a real WSL Cacti install (composer-managed Pest, not a plugin-local vendor tree): 50 passed (161 assertions), up from 41.
TheWitness
requested review from
bmfmancini and
xmacan
and
a lite review from Copilot
September 22, 2026 04:08
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved moderate findings affect integration-test validity and assertion completeness.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Expands automated coverage for the maintenance plugin’s lifecycle, navigation, actions, schema setup, and installation registration.
Changes:
- Adds unit tests for lifecycle, menus, navigation, actions, and database setup.
- Adds installation hook, realm, and table registration coverage.
- Extends test stubs and includes
tests/Integrationin PHPUnit.
| File | Summary |
|---|---|
tests/Unit/MaintLifecycleTest.php |
Adds lifecycle and version tests. |
tests/Unit/MaintDeviceActionsAndDatabaseTest.php |
Adds action and schema tests; moderate findings (1 vote each) require asserting labels/mappings and complete table definitions, including the composite primary key. |
tests/Unit/MaintConfigAndNavigationTest.php |
Adds menu and navigation tests; a moderate finding (1 vote) requires asserting complete breadcrumb values, not only keys. |
tests/Integration/MaintInstallHooksTest.php |
Adds installation registration coverage; a moderate finding (2 votes) notes that the unit bootstrap stubs APIs, so this is not Cacti-backed integration coverage. |
tests/bootstrap-unit.php |
Adds recording stubs for hook, realm, and schema calls. |
phpunit.xml |
Includes integration tests, but a moderate finding (1 vote) notes they still use the unit bootstrap. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| }); | ||
|
|
||
| it('registers every hook maint depends on, its realm, and provisions its tables', function () { | ||
| plugin_maint_install(); |
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
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.
This plugin already had thorough Unit coverage for
plugin_maint_check_host()/plugin_maint_check_schedule()(the actual maintenance-window logic) plus extensive Security coverage. The gap wassetup.php's plugin lifecycle and setup functions, which were only checked for static existence. Closed that gap, running the full suite against a real Cacti install in WSL (composer-managed Pest, not a plugin-local vendor tree) until everything passed.Added coverage
plugin_maint_version(),plugin_maint_uninstall(),plugin_maint_check_config()/plugin_maint_upgrade(),maint_config_arrays()(menu registration),maint_draw_navigation_text(),maint_device_action_array()(dropdown action registration), andmaint_setup_database()(verifies both tables it provisions).plugin_maint_install()(verifies every hook, the realm, and the tables it provisions, together, in one pass) — this plugin had notests/Integrationcategory before.Security
No new
unserialize()hardening was needed: everyunserialize()call in this plugin already goes throughsanitize_unserialize_selected_items().Intentionally not covered
maint_device_edit_top_links()(print-only, depends on real request context),maint_device_action_prepare()(large print-heavy confirmation-dialog renderer), andmaint_device_action_execute()(creates/updates real schedule rows with substantial branching) are left uncovered as comparatively low-value relative to the effort of safely mocking their dependencies, consistent with similarly-scoped functions skipped elsewhere in this coverage-expansion pass.Test infrastructure
tests/bootstrap-unit.phpgainedapi_plugin_register_hook/api_plugin_register_realmcall-logging stubs and upgradedapi_plugin_db_table_createto log its calls, reusing this repo's existingmaint_test_queue()/maint_test_next()fixture convention where applicable.phpunit.xmlnow includestests/Integrationin the testsuite.Result
Tests: 50 passed (161 assertions), up from 41.