test: add coverage for a few uncovered pure setup.php helpers - #372
Merged
Merged
Conversation
setup.php has a handful of small, pure functions that had no test coverage at all: plugin_syslog_version(), syslog_check_dependencies(), syslog_top_graph_refresh(), and syslog_draw_navigation_text(). Added tests/Unit/SetupSmallHelpersTest.php covering all four. Deliberately narrow in scope: this plugin's setup.php also contains much larger functions (plugin_syslog_install/uninstall(), syslog_connect(), syslog_check_upgrade(), syslog_install_advisor(), etc.) that do real database connection setup, read/write config files, and emit header()/exit flows - those are intentionally left untouched here rather than risk destabilizing this plugin's large, actively-developed existing test suite (147 tests across tests/Security and tests/Unit alone, plus regression/E2E/browser suites) with speculative new stubs/mocks for functions this involved. Verified via a real WSL Cacti install: 147 passed (687 assertions), up from 143 passed before this PR - full existing suite re-run to confirm no regressions from the bootstrap/test additions.
TheWitness
requested review from
bmfmancini and
xmacan
and
a lite review from Copilot
September 22, 2026 11:59
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The focused test addition is low risk; only a minor changelog documentation nit remains.
Review effort: Lite
Findings: None
What changed in this PR
Adds focused unit-test coverage for four small setup.php helpers.
Changes:
- Tests plugin metadata and dependency checks.
- Tests graph refresh passthrough and navigation preservation.
| File | Summary |
|---|---|
tests/Unit/SetupSmallHelpersTest.php |
Adds unit tests for four setup helpers; a changelog entry is recommended. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
Summary
setup.phphas a handful of small, pure functions that had no test coverage at all:plugin_syslog_version(),syslog_check_dependencies(),syslog_top_graph_refresh(), andsyslog_draw_navigation_text(). This PR adds coverage for those four.Scope note
This is deliberately narrow. This plugin's
setup.phpalso contains much larger functions (plugin_syslog_install()/uninstall(),syslog_connect(),syslog_check_upgrade(),syslog_install_advisor(), etc.) that do real database connection setup, read/write config files, and emitheader()/exitflows. Given this plugin's large, actively-developed existing test suite (147 tests acrosstests/Securityandtests/Unitalone, plus separateregression/E2E/browsersuites), I intentionally did not add speculative new stubs/mocks for those more involved functions in this PR - happy to follow up separately if useful.Result
Verified via a real WSL Cacti install (full existing suite re-run to confirm no regressions from the addition):
(up from 143 passed before this PR)