refactor(thold): lift the evaluation settings out of the threshold check

[somethingwithproof]

Phase 2 of restructuring thold_check_threshold().

Stacked on #799 — the characterization tests there are the safety net this phase depends on, so it should merge after. Until then this PR's diff shows #799's commits too; once that lands, what remains is two commits against thold_functions.php plus one test file.

Eight write-only locals

The preamble assigned eight values that nothing ever read. Verified by listing every reference to each inside the function:

$global_alert_address  1 reference   (the assignment)
$deadnotify            1 reference   (the assignment)
$show_datasource       1 reference   (the assignment)
$baseu                 1 reference   (the assignment)
$suffix                2 references  (both assignments, no read)
$show_units            1 reference   (the assignment)
$units_suffix          1 reference   (the assignment)
$decimals              1 reference   (the assignment)

Two of them cost real work per evaluation:

• $baseu ran SELECT base_value FROM graph_templates_graph WHERE local_graph_id = ? and discarded the result. That is one round trip per threshold per poll cycle, feeding nothing.
• $show_datasource called thold_datasource_required() for a value nobody used.

$deadnotify is the one #751 reports.

Lifting the preamble

What remains moves to thold_evaluation_context(). It derives everything from the threshold row and the Cacti settings, decides nothing, and has no side effects, which is what lets it move without changing behaviour. thold_check_threshold() unpacks the twenty values its arms actually use.

thold_check_threshold() goes from 1432 lines to 1378.

The real gain is reachability. These rules were only exercisable by driving a whole poll; they now have direct tests:

• an unset thold_fail_trigger falls back to the global alert_trigger
• alerts reach the warning recipients only when the two lists differ and the operator enabled it
• a graph is attached unless notifications are text-only or the threshold has no graph
• each SNMP trap class follows its own setting

Verification

The 35 characterization tests pass unchanged — no assertion needed editing, which is the check that this phase was pure motion. 15 new tests cover the extracted function directly. 50 total, 100% of the changed lines covered.

composer lint and composer test both clean.

Next

Phase 3 extracts the emitter — the subject/syslog/email/command/trap/log/hook block repeated 18 times across the three arms — and is where the divergences behind #742 through #752 get collapsed onto one implementation. Having $context as one value to pass is what makes that tractable.

[Copilot]

Pull request overview

Refactors the thold_check_threshold() evaluator setup by extracting its configuration/derived-values preamble into a dedicated thold_evaluation_context() helper, and introduces a PHPUnit-based characterization/coverage harness to lock in existing behavior while enabling further safe refactors.

Changes:

• Extracts evaluation settings/derived values into thold_evaluation_context() and updates thold_check_threshold() to consume the returned context.
• Adds PHPUnit unit/characterization tests plus a Cacti stub framework and scenario/outcome helpers to exercise evaluator behavior without a live Cacti install.
• Adds CI (Docker-based) to run lint, tests with coverage, and enforce changed-lines coverage.

Reviewed changes

Copilot reviewed 17 out of 19 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
thold_functions.php	Adds thold_evaluation_context() and replaces inline preamble in thold_check_threshold() with context unpacking.
tests/Unit/ThresholdHiLowCharacterizationTest.php	Characterization tests for the hi/low arm behavior over a single poll.
tests/Unit/ThresholdBaselineCharacterizationTest.php	Characterization tests for the baseline arm behavior over a single poll.
tests/Unit/ThresholdTimeBasedCharacterizationTest.php	Characterization tests for the time-based arm behavior over a single poll.
tests/Unit/TholdEvaluationContextTest.php	Direct unit tests for thold_evaluation_context() behavior (triggers, recipients, graph attachment, trap settings).
tests/TestCase.php	PHPUnit base class providing shared reset/setup and plugin loading helpers.
tests/Helpers/CactiStubs.php	Programmable/recording stub layer for Cacti global functions used by the plugin.
tests/Helpers/ThresholdScenario.php	Builder/driver for running one threshold through one poll of thold_check_threshold().
tests/Helpers/ThresholdOutcome.php	Assertion helper that interprets recorded side effects (mail/log/traps/persisted columns).
tests/fixtures/optional-core-functions.php	Fixture for optional core functions to exercise function_exists() branches.
tests/fixtures/cacti-lib/variables.php	Minimal file fixture to satisfy runtime includes in plugin code during tests.
tests/bootstrap-unit.php	Test bootstrap providing Cacti function shims and global environment expected by the plugin.
tests/bin/patch-coverage.php	Computes coverage specifically for added/modified lines versus a base ref.
tests/docker/Dockerfile	Dockerized PHPUnit runner pinned to PHP 8.1 with pcov for coverage.
tests/docker/docker-compose.yml	Local developer entrypoint to run the same Docker-based test flow as CI.
phpunit.xml	PHPUnit configuration targeting unit tests and scoping coverage to thold_functions.php.
composer.json	Adds dev dependencies (PHPUnit, phplint) and scripts for lint/test/coverage/docker.
.gitignore	Ignores vendor, coverage output, and PHPUnit cache artifacts.
.github/workflows/php-unit-tests.yml	GitHub Actions workflow to run lint, tests with coverage, and changed-lines coverage gate in Docker.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.