test: pin the threshold evaluator's behaviour before restructuring it

[somethingwithproof]

Phase 1 of restructuring thold_check_threshold(). No production file is touched — this only adds tests.

Why this comes first

thold_check_threshold() is 1,431 lines, and after a 157-line preamble it is a switch with three arms that are near-copies of each other:

Arm	Lines	thold_mail	thold_snmptrap	db_execute_prepared
0 — hi/low	489	8	5	8
1 — baseline	251	4	2	5
2 — time based	530	6	4	11

They have drifted, and the drift is what #742 through #752 report. Collapsing them onto one implementation is the fix, but the function has six side effects per transition and no tests at all — so a structural change now would be a rewrite by hope.

These tests pin present behaviour, bugs included, so the later phases can move code and know nothing shifted.

How it works

The function returns nothing; everything it decides is a side effect through global Cacti functions. ThresholdScenario builds the fixture those need and runs one poll. ThresholdOutcome reads back what was emitted — mail, log rows, traps, and the columns written to thold_data — so a test asserts on behaviour rather than on SQL text and does not break when a query is reworded.

$outcome = ThresholdScenario::threshold(['thold_hi' => 90, 'lastread' => 95])
    ->alertRecipient('ops@example.org')
    ->poll();

$this->assertSame([ST_NOTIFYAL], $outcome->logStatuses());

persistedColumns() parses the SET clause and resolves each ? against the bound parameters, because these statements mix placeholders and literals in one clause.

What the tests found

Writing them turned up behaviour worth recording. Each is pinned with a comment saying it is recorded rather than endorsed:

• A time-based restoral sends no mail. It writes the ST_NOTIFYRS log row and clears the state, but never mails, so an operator watching a time-based threshold sees the alert and never the all-clear. The hi/low arm mails.
• An alert breach zeroes the warning counter. A threshold crossing the warning bound on its way up loses that progress.
• A hi/low breach below its trigger leaves no log row. ST_TRIGGERA exists for this case but only the time-based arm writes it.
• The alert state is persisted on the first breaching poll, before the trigger count is met, so the interface shows thresholds in alert that have not notified and may never do so.
• The To header carries trailing empty entries — ops@example.org,, — because the legacy contact list is joined with the global and device addresses whether or not those are set.

One harness correction worth calling out

Cacti's db_fetch_cell_prepared() returns false, not '', when a query matches no row. The stub originally returned '', and on PHP 8 that difference is a TypeError in the time-based re-alert arithmetic ($realerttime + $lastemailtime). I nearly filed that as a fatal before checking lib/database.php; it is not one, and the stub now matches core. Recording it here because it is the kind of thing a stub can invent.

Scope

25 tests over the hi/low and time-based arms. The baseline arm needs reference-value fixtures and is not covered yet; it will follow before Phase 2 begins.

The harness commit matches #773, #788, #790 and #791, extended with SQL-matched stub responses and gmp. Whichever lands first, the others merge cleanly.

Full plan for the remaining phases is written up separately; Phase 2 lifts the preamble, Phase 3 extracts the emitter, Phase 4 fixes the filed bugs one commit each, and Phase 5 — collapsing the arms — is optional and gated on maintainer appetite.

[somethingwithproof]

The four Integration Test failures are the upstream break, not this PR — which touches no production file at all. Install Cacti via CLI dies in core:

PHP Fatal error: Uncaught Error: Call to undefined function __()
  in cacti/lib/functions.php:7973

The PHPUnit job passes. #776 pins the Cacti checkout to release/1.2.31 and is green on the same workflow.

[Copilot]

Pull request overview

Adds a PHPUnit-based characterization test harness for thold_check_threshold() (hi/low and time-based arms) so the evaluator can be safely restructured later while preserving current side-effect behavior, and wires it into CI via a Dockerized test runner and patch-coverage gate.

Changes:

• Introduces a Cacti-function stub/recorder (CactiStub) plus ThresholdScenario/ThresholdOutcome helpers to exercise thold_check_threshold() by observing side effects (mail/log/traps/DB writes).
• Adds characterization tests covering key hi/low and time-based behaviors, including documented “recorded, not endorsed” quirks.
• Adds PHPUnit + Docker + GitHub Actions workflow, plus a patch-coverage script to enforce coverage of changed PHP lines.

Reviewed changes

Copilot reviewed 14 out of 16 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/Unit/ThresholdTimeBasedCharacterizationTest.php	Adds time-based arm characterization tests for one-poll behavior and drift vs hi/low.
tests/Unit/ThresholdHiLowCharacterizationTest.php	Adds hi/low arm characterization tests including notification/restoral/re-alert behaviors.
tests/TestCase.php	Base test case ensuring stub reset + plugin source/constants loading helpers.
tests/Support/ThresholdScenario.php	Scenario builder that stubs required DB/core calls and runs one poll.
tests/Support/ThresholdOutcome.php	Reads recorded stub calls to expose emitted outcomes in assertion-friendly form.
tests/Support/CactiStub.php	Recording + programmable shim backing the global Cacti function stubs.
tests/fixtures/optional-core-functions.php	Optional core functions for exercising function_exists() branches in isolated runs.
tests/fixtures/cacti-lib/variables.php	Placeholder for Cacti lib/variables.php so include_once paths resolve in tests.
tests/docker/Dockerfile	Docker image for running the unit test suite with coverage tooling.
tests/docker/docker-compose.yml	Local developer wrapper to run the same Dockerized test job as CI.
tests/bootstrap.php	Global-function shims and minimal Cacti environment bootstrap for unit tests.
tests/bin/patch-coverage.php	Computes coverage specifically for changed PHP lines by diffing vs base ref.
phpunit.xml	PHPUnit config + focused coverage scope for thold_functions.php.
composer.json	Adds PHPUnit dependency, dev autoloading for test support classes, and test scripts.
.gitignore	Ignores vendor/cache/coverage artifacts introduced by the test toolchain.
.github/workflows/php-unit-tests.yml	GitHub Actions workflow running Dockerized PHPUnit + patch-coverage enforcement.

Suppressed comments (1)

tests/Support/CactiStub.php:112

• CactiStub::record() phpdoc for $params should allow associative keys (it is used for both bound parameter lists and associative row data like sql_save).

	 * @param string            $fn     Cacti function name.
	 * @param string            $sql    SQL text, or '' for non-query calls.
	 * @param array<int, mixed> $params Bound parameters, if any.
	 *
	 * @return void

---

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

[somethingwithproof]

Phase 2 is up in #800, stacked on this one. It passes these 35 tests unchanged, which is the check that the move was behaviour-preserving.