fix(thold): keep zero readings and stop falling back to the wrong data source

[somethingwithproof]

Fixes #787.

Two substitution helpers returned wrong values for inputs that occur in normal operation. Tests were written first and fail on develop.

A zero reading was blanked

if (empty($replace) || $replace === 0) {
    $replace = '';
}

empty('0') is true, so 0 and '0' both became an empty string. thold_str_replace() performs every tag substitution in notification bodies, subjects and trigger commands, so:

• an alert for a value that had dropped to zero rendered as Current value is with a blank — exactly the case an operator most needs to see;
• a trigger command invoked as --value <CURRENTVALUE> lost the argument entirely and shifted the ones after it.

Only null now counts as absent, which is what the guard was for.

An unknown data source returned the first one's value

$idx = array_search($data_template_rrd_id, $result['data_source_names'], true);

if (!isset($result['values']) || $idx === null || !cacti_sizeof($result['values'][$idx])) {
    return 0;
}

array_search() reports a miss as false, never null, so the guard never fired. $result['values'][false] is $result['values'][0] under PHP's bool-to-int key cast, so a lookup for a data source that does not exist quietly returned the first data source's value — which the caller then compares against the threshold bounds.

This is reachable today: thold_expression_specialtype_rpn() and the CDEF substitutions call get_current_value() with upper_limit, lower_limit, rrd_minimum, rrd_maximum and hdd_total, none of which are data source names.

Note the scope: this stops those call sites receiving another metric's value. It does not make them read the column they actually want — they still need to look up graph_templates_graph.upper_limit and friends. Worth a follow-up; I did not want to fold a behaviour change into a correctness fix.

While in the same function: thold_rrd_last() returns whatever rrdtool last printed, and a missing or unreadable RRD prints nothing. That empty string reached the timestamp arithmetic and threw TypeError: Unsupported operand types: string / int. The existing guard only handled -1.

Tests

20 tests, 100% of the changed lines covered.

The harness commit is the same one as #773 and #788, byte-identical, so whichever lands first the others merge cleanly. Once any of them merges this PR's diff is just the two thold_functions.php changes and its own tests.

[somethingwithproof]

The four Integration Test failures are the upstream break, not this PR. The workflow checks out Cacti/cacti unpinned and Install Cacti via CLI dies in core:

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

format_cacti_version_text() calls __() before the translation layer loads. Nothing here touches Cacti core, and the PHPUnit job passes. #776 pins the checkout to release/1.2.31 and is green.

[Copilot]

Pull request overview

This PR fixes two correctness issues in thold’s tag/data-source substitution paths that can surface in normal operation (zero readings being blanked, and unknown “data source names” returning the first DS value). It also introduces a PHPUnit-based harness plus CI coverage gating to lock the fixes in.

Changes:

• Fix thold_str_replace() so 0/'0' survive substitutions instead of becoming empty.
• Fix get_current_value() to (a) treat an array_search() miss correctly and (b) handle missing/unreadable RRD last output without a TypeError.
• Add PHPUnit test harness + Docker runner + GitHub Actions workflow with changed-lines coverage enforcement.

Reviewed changes

Copilot reviewed 13 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
thold_functions.php	Corrects substitution edge cases in get_current_value() and thold_str_replace() to prevent wrong/blank values.
tests/Unit/TholdStrReplaceTest.php	Unit coverage for zero/absent substitution behavior.
tests/Unit/GetCurrentValueTest.php	Unit coverage for unknown DS lookup and unreadable RRD behavior.
tests/TestCase.php	Shared PHPUnit base providing consistent stub reset and plugin source loading.
tests/Support/CactiStub.php	Framework-function recording/programmable stub to unit-test procedural plugin code.
tests/fixtures/optional-core-functions.php	Optional core-function shims for exercising function_exists() branches.
tests/fixtures/cacti-lib/variables.php	Minimal fixture file to satisfy runtime include_once() expectations.
tests/docker/Dockerfile	Reproducible PHP 8.1 + pcov test runner image.
tests/docker/docker-compose.yml	Local “mirror” invocation of the CI unit-test job.
tests/bootstrap.php	Test bootstrap providing stubbed Cacti globals/functions and plugin loader.
tests/bin/patch-coverage.php	Script enforcing statement coverage for changed PHP lines only.
phpunit.xml	PHPUnit config targeting thold_functions.php coverage and strictness flags.
composer.json	Adds PHPUnit dependency + scripts for local/CI test running.
.gitignore	Ignores vendor, coverage, and PHPUnit cache artifacts.
.github/workflows/php-unit-tests.yml	CI job to run lint + PHPUnit with coverage + enforce changed-line coverage.

---

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