From 1009497f5f19f6b0c73e5d44bb1b3363a86ec88c Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Sun, 16 Aug 2026 19:33:56 -0700 Subject: [PATCH 1/7] test: add the PHP 8.1 unit-test harness Same harness as #773 and #788, so whichever lands first the others merge cleanly. Signed-off-by: Thomas Vincent --- tests/Support/CactiStub.php | 143 ++++++++++++ tests/bootstrap.php | 384 ++++++++++++++++++++++++++++++++ tests/docker/Dockerfile | 28 +++ tests/docker/docker-compose.yml | 15 ++ 4 files changed, 570 insertions(+) create mode 100644 tests/Support/CactiStub.php create mode 100644 tests/bootstrap.php create mode 100644 tests/docker/Dockerfile create mode 100644 tests/docker/docker-compose.yml diff --git a/tests/Support/CactiStub.php b/tests/Support/CactiStub.php new file mode 100644 index 00000000..17a5ab48 --- /dev/null +++ b/tests/Support/CactiStub.php @@ -0,0 +1,143 @@ +}> + */ + public static $calls = []; + + /** + * Queued return values, keyed by function name. Each call shifts one value + * off the front; an exhausted queue falls back to the type default. + * + * @var array> + */ + public static $returns = []; + + /** + * Values handed back by the get_*_request_var() family, keyed by var name. + * + * @var array + */ + public static $requestVars = []; + + /** + * Values handed back by read_config_option(), keyed by option name. + * + * @var array + */ + public static $configOptions = []; + + /** + * Messages passed to cacti_log(), in order. + * + * @var array + */ + public static $log = []; + + /** + * Clear all recorded and programmed state. + * + * @return void + */ + public static function reset() { + self::$calls = []; + self::$returns = []; + self::$requestVars = []; + self::$configOptions = []; + self::$log = []; + } + + /** + * Record one Cacti function call. + * + * @param string $fn Cacti function name. + * @param string $sql SQL text, or '' for non-query calls. + * @param array $params Bound parameters, if any. + * + * @return void + */ + public static function record($fn, $sql = '', array $params = []) { + self::$calls[] = ['fn' => $fn, 'sql' => $sql, 'params' => $params]; + } + + /** + * Queue one return value for the next call to $fn. + * + * @param string $fn Cacti function name. + * @param mixed $value Value to hand back. + * + * @return void + */ + public static function willReturn($fn, $value) { + self::$returns[$fn][] = $value; + } + + /** + * Take the next queued return value for $fn, or $default when none is left. + * + * @param string $fn Cacti function name. + * @param mixed $default Fallback when the queue is empty. + * + * @return mixed + */ + public static function nextReturn($fn, $default) { + if (!empty(self::$returns[$fn])) { + return array_shift(self::$returns[$fn]); + } + + return $default; + } + + /** + * All recorded calls to $fn. + * + * @param string $fn Cacti function name. + * + * @return array}> + */ + public static function callsTo($fn) { + return array_values(array_filter(self::$calls, function ($call) use ($fn) { + return $call['fn'] === $fn; + })); + } + + /** + * The recorded call log reduced to function names, in order. Useful for + * asserting transaction sequencing. + * + * @return array + */ + public static function callSequence() { + return array_column(self::$calls, 'fn'); + } +} diff --git a/tests/bootstrap.php b/tests/bootstrap.php new file mode 100644 index 00000000..e9d4d5ee --- /dev/null +++ b/tests/bootstrap.php @@ -0,0 +1,384 @@ + dirname(dirname(dirname(__DIR__))), + 'url_path' => '/cacti/', + 'cacti_version' => '1.2.31', + 'cacti_server_os' => 'unix', +]; + +// thold_expand_string() include_once()s library_path/variables.php at call time. +$GLOBALS['config']['library_path'] = __DIR__ . '/fixtures/cacti-lib'; + +// thold reads and writes this on every RPN evaluation. +$GLOBALS['rpn_error'] = false; + +if (!function_exists('db_execute')) { + function db_execute($sql, $log = true, $db_conn = false) { + CactiStub::record('db_execute', $sql); + + return CactiStub::nextReturn('db_execute', true); + } +} + +if (!function_exists('db_execute_prepared')) { + function db_execute_prepared($sql, $params = [], $log = true, $db_conn = false) { + CactiStub::record('db_execute_prepared', $sql, $params); + + return CactiStub::nextReturn('db_execute_prepared', true); + } +} + +if (!function_exists('db_fetch_assoc')) { + function db_fetch_assoc($sql, $log = true, $db_conn = false) { + CactiStub::record('db_fetch_assoc', $sql); + + return CactiStub::nextReturn('db_fetch_assoc', []); + } +} + +if (!function_exists('db_fetch_assoc_prepared')) { + function db_fetch_assoc_prepared($sql, $params = [], $log = true, $db_conn = false) { + CactiStub::record('db_fetch_assoc_prepared', $sql, $params); + + return CactiStub::nextReturn('db_fetch_assoc_prepared', []); + } +} + +if (!function_exists('db_fetch_row')) { + function db_fetch_row($sql, $log = true, $db_conn = false) { + CactiStub::record('db_fetch_row', $sql); + + return CactiStub::nextReturn('db_fetch_row', []); + } +} + +if (!function_exists('db_fetch_row_prepared')) { + function db_fetch_row_prepared($sql, $params = [], $log = true, $db_conn = false) { + CactiStub::record('db_fetch_row_prepared', $sql, $params); + + return CactiStub::nextReturn('db_fetch_row_prepared', []); + } +} + +if (!function_exists('db_fetch_cell')) { + function db_fetch_cell($sql, $col_name = '', $log = true, $db_conn = false) { + CactiStub::record('db_fetch_cell', $sql); + + return CactiStub::nextReturn('db_fetch_cell', ''); + } +} + +if (!function_exists('db_fetch_cell_prepared')) { + function db_fetch_cell_prepared($sql, $params = [], $col_name = '', $log = true, $db_conn = false) { + CactiStub::record('db_fetch_cell_prepared', $sql, $params); + + return CactiStub::nextReturn('db_fetch_cell_prepared', ''); + } +} + +if (!function_exists('db_qstr')) { + function db_qstr($string) { + return "'" . str_replace("'", "''", (string) $string) . "'"; + } +} + +if (!function_exists('db_begin_transaction')) { + function db_begin_transaction() { + CactiStub::record('db_begin_transaction'); + + return CactiStub::nextReturn('db_begin_transaction', true); + } +} + +if (!function_exists('db_commit_transaction')) { + function db_commit_transaction() { + CactiStub::record('db_commit_transaction'); + + return CactiStub::nextReturn('db_commit_transaction', true); + } +} + +if (!function_exists('db_rollback_transaction')) { + function db_rollback_transaction() { + CactiStub::record('db_rollback_transaction'); + + return CactiStub::nextReturn('db_rollback_transaction', true); + } +} + +if (!function_exists('html_escape')) { + function html_escape($string) { + return htmlspecialchars((string) $string, ENT_QUOTES, 'UTF-8'); + } +} + +/* + * Mirrors Cacti 1.2 lib/functions.php. KEEP IN SYNC: if core tightens its + * checks, tests here would otherwise keep passing while production diverges. + */ +if (!function_exists('sanitize_unserialize_selected_items')) { + function sanitize_unserialize_selected_items($items) { + if (empty($items)) { + return false; + } + + $data = unserialize($items, ['allowed_classes' => false]); // nosemgrep: php.lang.security.unserialize-use.unserialize-use -- test stub mirroring Cacti core; allowed_classes:false blocks object injection + + if (!is_array($data)) { + return false; + } + + foreach ($data as $value) { + if (!is_numeric($value)) { + return false; + } + } + + return $data; + } +} + +if (!function_exists('read_config_option')) { + function read_config_option($name, $force = false) { + return isset(CactiStub::$configOptions[$name]) ? CactiStub::$configOptions[$name] : ''; + } +} + +if (!function_exists('set_config_option')) { + function set_config_option($name, $value) { + CactiStub::$configOptions[$name] = $value; + } +} + +if (!function_exists('__')) { + function __($text) { + $args = array_slice(func_get_args(), 1); + + // Cacti's __() accepts sprintf arguments after the format string. + return $args === [] ? $text : vsprintf($text, $args); + } +} + +if (!function_exists('__esc')) { + function __esc($text) { + return htmlspecialchars(call_user_func_array('__', func_get_args()), ENT_QUOTES, 'UTF-8'); + } +} + +if (!function_exists('cacti_log')) { + function cacti_log($message, $output = false, $environ = 'CMDPHP', $level = 0) { + CactiStub::$log[] = $message; + } +} + +if (!function_exists('cacti_sizeof')) { + function cacti_sizeof($array) { + return (is_array($array) || $array instanceof Countable) ? count($array) : 0; + } +} + +if (!function_exists('cacti_count')) { + function cacti_count($array) { + return cacti_sizeof($array); + } +} + +if (!function_exists('get_request_var')) { + function get_request_var($name, $default = '') { + return isset(CactiStub::$requestVars[$name]) ? CactiStub::$requestVars[$name] : $default; + } +} + +if (!function_exists('get_nfilter_request_var')) { + function get_nfilter_request_var($name, $default = '') { + return get_request_var($name, $default); + } +} + +if (!function_exists('get_filter_request_var')) { + function get_filter_request_var($name, $filter = FILTER_VALIDATE_INT, $options = []) { + return get_request_var($name); + } +} + +if (!function_exists('isset_request_var')) { + function isset_request_var($name) { + return isset(CactiStub::$requestVars[$name]); + } +} + +if (!function_exists('cacti_escapeshellarg')) { + function cacti_escapeshellarg($string, $quote = true) { + return escapeshellarg((string) $string); + } +} + +if (!function_exists('api_plugin_hook_function')) { + function api_plugin_hook_function($name, $data = '') { + return $data; + } +} + +if (!function_exists('get_simple_graph_perms')) { + function get_simple_graph_perms($user_id) { + return CactiStub::nextReturn('get_simple_graph_perms', true); + } +} + +if (!function_exists('get_policies')) { + function get_policies($user_id) { + return CactiStub::nextReturn('get_policies', []); + } +} + +if (!function_exists('get_policy_where')) { + function get_policy_where($graph_auth_method, $policies, $sql_where) { + CactiStub::record('get_policy_where', $sql_where); + + return CactiStub::nextReturn('get_policy_where', $sql_where); + } +} + +if (!function_exists('expand_title')) { + function expand_title($host_id, $snmp_query_id, $snmp_index, $title) { + CactiStub::record('expand_title', $title); + + return CactiStub::nextReturn('expand_title', $title); + } +} + +if (!function_exists('get_graph_title')) { + function get_graph_title($local_graph_id) { + return CactiStub::nextReturn('get_graph_title', 'Traffic - eth0'); + } +} + +if (!function_exists('rrdtool_function_fetch')) { + function rrdtool_function_fetch($local_data_id, $start_time, $end_time, $resolution = 0, $show_unknown = false, $rrdtool_file = null) { + CactiStub::record('rrdtool_function_fetch', (string) $local_data_id); + + return CactiStub::nextReturn('rrdtool_function_fetch', []); + } +} + +if (!function_exists('get_data_source_path')) { + function get_data_source_path($local_data_id, $expand_paths = true) { + return '/var/lib/cacti/rra/test_' . (int) $local_data_id . '.rrd'; + } +} + +if (!function_exists('rrdtool_execute')) { + function rrdtool_execute($command, $log_to_stdout = false, $output_flag = 1, $rrdtool_pipe = false, $logopt = 'WEBLOG') { + CactiStub::record('rrdtool_execute', $command); + + return CactiStub::nextReturn('rrdtool_execute', ''); + } +} + +if (!function_exists('rrdtool_function_interface_speed')) { + function rrdtool_function_interface_speed($data_local) { + return CactiStub::nextReturn('rrdtool_function_interface_speed', 0); + } +} + +if (!function_exists('get_timeinstate')) { + function get_timeinstate($host) { + return CactiStub::nextReturn('get_timeinstate', '1 day'); + } +} + +if (!function_exists('get_daysfromtime')) { + function get_daysfromtime($timestamp) { + return CactiStub::nextReturn('get_daysfromtime', '1 day'); + } +} + +if (!function_exists('number_format_i18n')) { + function number_format_i18n($number, $decimals = 0, $baseu = 1000) { + return number_format((float) $number, $decimals < 0 ? 0 : (int) $decimals); + } +} + +if (!defined('FILTER_VALIDATE_IS_REGEX')) { + define('FILTER_VALIDATE_IS_REGEX', 99999); +} + +// Device states, from Cacti include/global_constants.php. +foreach (['HOST_UNKNOWN' => 0, 'HOST_DOWN' => 1, 'HOST_RECOVERING' => 2, 'HOST_UP' => 3, 'HOST_ERROR' => 4] as $name => $value) { + if (!defined($name)) { + define($name, $value); + } +} + +if (!defined('RRDTOOL_OUTPUT_STDOUT')) { + define('RRDTOOL_OUTPUT_STDOUT', 1); +} + +if (!defined('CACTI_DATE_TIME_FORMAT')) { + define('CACTI_DATE_TIME_FORMAT', 'Y-m-d H:i:s'); +} + +if (!defined('CACTI_PATH_BASE')) { + define('CACTI_PATH_BASE', $GLOBALS['config']['base_path']); +} + +/** + * Load a plugin source file at global scope. + * + * Several plugin files (includes/arrays.php in particular) define their data + * as file-scope variables that the rest of the plugin reads as globals, and + * they read $config while doing so. Requiring them from inside a method would + * make both halves of that method-local, so the require happens here and any + * variable the file introduced is published to $GLOBALS. + * + * @param string $path Absolute path to the file. + * + * @return void + */ +function thold_test_load($path) { + global $config; + + $__before = get_defined_vars(); + + require_once $path; + + foreach (get_defined_vars() as $__name => $__value) { + if (!array_key_exists($__name, $__before) && strncmp($__name, '__', 2) !== 0) { + $GLOBALS[$__name] = $__value; + } + } +} diff --git a/tests/docker/Dockerfile b/tests/docker/Dockerfile new file mode 100644 index 00000000..cb4279d1 --- /dev/null +++ b/tests/docker/Dockerfile @@ -0,0 +1,28 @@ +# Test runner for the Thold plugin. +# +# Pinned to PHP 8.1 because that is the oldest interpreter the CI matrix +# covers; what passes here passes on 8.2-8.4. pcov rather than Xdebug: line +# coverage is the only debug feature the suite needs and pcov is far cheaper. +FROM php:8.1-cli-alpine@sha256:7949370448b0b4d9787776dc5968e0fd8d48763292344b5fbf21539441228a98 + +# git is needed by the changed-line coverage gate, which diffs against the +# base branch. +RUN apk add --no-cache git \ + && apk add --no-cache --virtual .build-deps $PHPIZE_DEPS \ + && pecl install pcov \ + && docker-php-ext-enable pcov \ + && apk del .build-deps + +COPY --from=composer:2@sha256:4d71c3c2109c61d5415544264b59ad4087e4c5b7244481723664138fd36d5040 /usr/bin/composer /usr/bin/composer + +# The plugin lives where Cacti would put it, because thold_functions.php +# resolves its own includes through $config['base_path'] . '/plugins/thold'. +# No network or database is involved; the Cacti framework functions themselves +# are stubbed in tests/bootstrap.php. +WORKDIR /cacti/plugins/thold + +ENV COMPOSER_ALLOW_SUPERUSER=1 \ + COMPOSER_NO_INTERACTION=1 \ + COMPOSER_CACHE_DIR=/tmp/composer-cache + +CMD ["sh", "-c", "composer install --no-progress --no-ansi && vendor/bin/phpunit"] diff --git a/tests/docker/docker-compose.yml b/tests/docker/docker-compose.yml new file mode 100644 index 00000000..99d38b47 --- /dev/null +++ b/tests/docker/docker-compose.yml @@ -0,0 +1,15 @@ +# Local mirror of the unit-test CI job. `docker compose -f +# tests/docker/docker-compose.yml run --rm phpunit` runs exactly what CI runs. +services: + phpunit: + build: + context: . + dockerfile: Dockerfile + image: cacti-thold-test:php8.1 + working_dir: /cacti/plugins/thold + volumes: + - ../..:/cacti/plugins/thold + - composer-cache:/tmp/composer-cache + +volumes: + composer-cache: From babf8329a86bef30e30ba18da3160999976ce150 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Sun, 16 Aug 2026 19:34:11 -0700 Subject: [PATCH 2/7] fix(thold): keep a zero reading through tag substitution thold_str_replace() treated 0 and '0' as absent, so an alert for a value that had dropped to zero rendered as "Current value is " with a blank, and a trigger command invoked as --value lost the argument and shifted the ones after it. Refs #787 Signed-off-by: Thomas Vincent --- tests/Unit/TholdStrReplaceTest.php | 94 ++++++++++++++++++++++++++++++ thold_functions.php | 19 ++++-- 2 files changed, 107 insertions(+), 6 deletions(-) create mode 100644 tests/Unit/TholdStrReplaceTest.php diff --git a/tests/Unit/TholdStrReplaceTest.php b/tests/Unit/TholdStrReplaceTest.php new file mode 100644 index 00000000..69006c39 --- /dev/null +++ b/tests/Unit/TholdStrReplaceTest.php @@ -0,0 +1,94 @@ + + */ + public static function preservedValueProvider() { + return [ + 'integer zero' => [0, 'v=0'], + 'string zero' => ['0', 'v=0'], + 'float zero' => [0.0, 'v=0'], + 'negative' => [-5, 'v=-5'], + 'positive integer' => [5, 'v=5'], + 'float' => [2.5, 'v=2.5'], + 'string zero decimal' => ['0.0', 'v=0.0'], + ]; + } + + /** + * @dataProvider preservedValueProvider + * + * @param mixed $replace + * @param string $expected + * + * @return void + */ + public function testNumericValuesSurviveSubstitution($replace, $expected): void { + $this->assertSame($expected, thold_str_replace('', $replace, 'v=')); + } + + /** + * @return array + */ + public static function absentValueProvider() { + return [ + 'null' => [null], + 'false' => [false], + 'empty string' => [''], + ]; + } + + /** + * @dataProvider absentValueProvider + * + * @param mixed $replace + * + * @return void + */ + public function testAbsentValuesBecomeEmpty($replace): void { + $this->assertSame('v=', thold_str_replace('', $replace, 'v=')); + } + + /** + * @return void + */ + public function testEveryOccurrenceIsReplaced(): void { + $this->assertSame('0 and 0', thold_str_replace('', 0, ' and ')); + } + + /** + * @return void + */ + public function testSubjectWithoutTheTagIsUnchanged(): void { + $this->assertSame('no tags here', thold_str_replace('', 5, 'no tags here')); + } +} diff --git a/thold_functions.php b/thold_functions.php index 018cf99b..5e1838f6 100644 --- a/thold_functions.php +++ b/thold_functions.php @@ -8310,12 +8310,19 @@ function thold_get_cached_name(&$thold_data) { return $thold_data['name_cache']; } -function thold_str_replace($search, $replace, $subject) { - if (empty($replace) || $replace === 0) { - $replace = ''; - } - - return str_replace($search, $replace, $subject); +/** + * Substitute one tag, rendering an absent value as an empty string. + * + * Only null and false count as absent. Zero is a legitimate reading, and + * blanking it produced alert bodies reading "Current value is " for exactly + * the case an operator most needs to see. + * + * @param string $search Tag to replace. + * @param mixed $replace Value to substitute. + * @param string $subject Text containing the tag. + */ +function thold_str_replace(string $search, $replace, string $subject): string { + return str_replace($search, $replace ?? '', $subject); } function thold_template_import($xml_data) { From 06756ec7f6cf21ad6094014fdb33d86bbb62e8ff Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Sun, 16 Aug 2026 19:34:22 -0700 Subject: [PATCH 3/7] fix(thold): return zero when the requested data source is absent array_search() reports a miss as false, so a guard written against null let it through and $result['values'][false] read index 0. A lookup for a data source that does not exist returned the first one's value, which the caller then compared against the threshold bounds. Reached today from thold_expression_specialtype_rpn() and the CDEF substitutions, which pass column names such as upper_limit rather than data source names. Those call sites still need to read the real column; this only stops them silently receiving another metric. Refs #787 Signed-off-by: Thomas Vincent --- tests/Unit/GetCurrentValueTest.php | 148 +++++++++++++++++++++++++++++ thold_functions.php | 11 ++- 2 files changed, 155 insertions(+), 4 deletions(-) create mode 100644 tests/Unit/GetCurrentValueTest.php diff --git a/tests/Unit/GetCurrentValueTest.php b/tests/Unit/GetCurrentValueTest.php new file mode 100644 index 00000000..f82a2c44 --- /dev/null +++ b/tests/Unit/GetCurrentValueTest.php @@ -0,0 +1,148 @@ + 300]); + + // thold_rrd_last() returns whatever `rrdtool last` printed. + CactiStub::willReturn('rrdtool_execute', '1700000000'); + } + + /** + * @param array $fetch + * + * @return void + */ + private function rrdReturns(array $fetch) { + CactiStub::willReturn('rrdtool_function_fetch', $fetch); + } + + /** + * @return void + */ + public function testTheRequestedDataSourceValueIsReturned(): void { + $this->rrdReturns([ + 'data_source_names' => ['traffic_in', 'traffic_out'], + 'values' => [['1700000000' => 10.0], ['1700000000' => 20.0]], + ]); + + $this->assertSame(20.0, get_current_value(4, 'traffic_out')); + } + + /** + * array_search() returns false, not null, so a guard written against null + * let the miss through and PHP then read index 0 — the first data source. + * + * @return void + */ + public function testUnknownDataSourceReturnsZeroRatherThanTheFirstOne(): void { + $this->rrdReturns([ + 'data_source_names' => ['traffic_in', 'traffic_out'], + 'values' => [['1700000000' => 10.0], ['1700000000' => 20.0]], + ]); + + $this->assertSame(0, get_current_value(4, 'upper_limit')); + } + + /** + * @return void + */ + public function testFirstDataSourceIsStillReachableByName(): void { + $this->rrdReturns([ + 'data_source_names' => ['traffic_in', 'traffic_out'], + 'values' => [['1700000000' => 10.0], ['1700000000' => 20.0]], + ]); + + $this->assertSame(10.0, get_current_value(4, 'traffic_in')); + } + + /** + * @return void + */ + public function testMissingDataSourceNamesReturnsZero(): void { + $this->rrdReturns([]); + + $this->assertSame(0, get_current_value(4, 'traffic_in')); + } + + /** + * @return void + */ + public function testMissingValuesReturnsZero(): void { + $this->rrdReturns(['data_source_names' => ['traffic_in']]); + + $this->assertSame(0, get_current_value(4, 'traffic_in')); + } + + /** + * @return void + */ + public function testEmptyValueSeriesReturnsZero(): void { + $this->rrdReturns([ + 'data_source_names' => ['traffic_in'], + 'values' => [[]], + ]); + + $this->assertSame(0, get_current_value(4, 'traffic_in')); + } + + /** + * A missing or unreadable RRD makes `rrdtool last` print nothing, which + * used to reach the timestamp arithmetic as an empty string and fatal. + * + * @return void + */ + public function testUnreadableRrdReturnsZeroRatherThanThrowing(): void { + CactiStub::reset(); + CactiStub::willReturn('db_fetch_row_prepared', ['rrd_step' => 300]); + CactiStub::willReturn('rrdtool_execute', ''); + $this->rrdReturns([]); + + $this->assertSame(0, get_current_value(4, 'traffic_in')); + } + + /** + * @return void + */ + public function testValueIsRoundedToFourDecimals(): void { + $this->rrdReturns([ + 'data_source_names' => ['traffic_in'], + 'values' => [['1700000000' => 1.23456789]], + ]); + + $this->assertSame(1.2346, get_current_value(4, 'traffic_in')); + } +} diff --git a/thold_functions.php b/thold_functions.php index 5e1838f6..d8228c50 100644 --- a/thold_functions.php +++ b/thold_functions.php @@ -4861,8 +4861,9 @@ function get_current_value($local_data_id, $data_template_rrd_id, $cdef = 0) { $last_time_entry = thold_rrd_last($local_data_id); - // This should fix and 'did you really mean month 899 errors', this is because your RRD has not polled yet - if ($last_time_entry == -1) { + // This should fix and 'did you really mean month 899 errors', this is because your RRD has not polled yet. + // A missing or unreadable RRD makes rrdtool print nothing, which is not a timestamp either. + if (!is_numeric($last_time_entry) || $last_time_entry == -1) { $last_time_entry = time(); } @@ -4884,11 +4885,13 @@ function get_current_value($local_data_id, $data_template_rrd_id, $cdef = 0) { return 0; } + // array_search() reports a miss as false. Testing for null let the miss + // through, and $result['values'][false] then read index 0, so a lookup for + // a data source that does not exist returned the first one's value. $idx = array_search($data_template_rrd_id, $result['data_source_names'], true); // Return Blank if the value was not found (Cache Cleared?) - - if (!isset($result['values']) || $idx === null || !cacti_sizeof($result['values'][$idx])) { + if ($idx === false || !isset($result['values'][$idx]) || !cacti_sizeof($result['values'][$idx])) { return 0; } From 767819151138a0b7cf0952fec4ce255badae190a Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Sun, 16 Aug 2026 20:18:33 -0700 Subject: [PATCH 4/7] test: follow Cacti's test layout and composer scripts Adopts the conventions from Cacti core: tests/bootstrap-unit.php, tests/Helpers for the stubs, a phpunit.xml carrying error_reporting -1 and CACTI_TEST_BOOTSTRAP, and composer lint / test / test:coverage scripts so CI runs the same commands a developer does. The dev toolchain is Cacti's, pinned to the same platform php 8.1.0. Cacti core runs Pest and this suite does not, because pest ^2 does not resolve on PHP 8.1 -- the platform Cacti's own composer.json pins. Releases up to v2.36.0 conflict with phpunit 10.5.62 and later, every earlier 10.x release is blocked by advisory PKSA-z3gr-8qht-p93v, and v2.36.1, which does resolve, requires PHP 8.2. The stack installs on 8.2 and above; 8.1 is the floor this plugin's CI matrix targets. The tests are written in the plain PHPUnit class style that Cacti's tests/Pest.php explicitly supports, so they run unchanged under Pest wherever it is installable. Signed-off-by: Thomas Vincent --- tests/Support/CactiStub.php | 143 ----------- tests/Unit/GetCurrentValueTest.php | 12 +- tests/bootstrap.php | 384 ----------------------------- tests/docker/Dockerfile | 5 +- 4 files changed, 9 insertions(+), 535 deletions(-) delete mode 100644 tests/Support/CactiStub.php delete mode 100644 tests/bootstrap.php diff --git a/tests/Support/CactiStub.php b/tests/Support/CactiStub.php deleted file mode 100644 index 17a5ab48..00000000 --- a/tests/Support/CactiStub.php +++ /dev/null @@ -1,143 +0,0 @@ -}> - */ - public static $calls = []; - - /** - * Queued return values, keyed by function name. Each call shifts one value - * off the front; an exhausted queue falls back to the type default. - * - * @var array> - */ - public static $returns = []; - - /** - * Values handed back by the get_*_request_var() family, keyed by var name. - * - * @var array - */ - public static $requestVars = []; - - /** - * Values handed back by read_config_option(), keyed by option name. - * - * @var array - */ - public static $configOptions = []; - - /** - * Messages passed to cacti_log(), in order. - * - * @var array - */ - public static $log = []; - - /** - * Clear all recorded and programmed state. - * - * @return void - */ - public static function reset() { - self::$calls = []; - self::$returns = []; - self::$requestVars = []; - self::$configOptions = []; - self::$log = []; - } - - /** - * Record one Cacti function call. - * - * @param string $fn Cacti function name. - * @param string $sql SQL text, or '' for non-query calls. - * @param array $params Bound parameters, if any. - * - * @return void - */ - public static function record($fn, $sql = '', array $params = []) { - self::$calls[] = ['fn' => $fn, 'sql' => $sql, 'params' => $params]; - } - - /** - * Queue one return value for the next call to $fn. - * - * @param string $fn Cacti function name. - * @param mixed $value Value to hand back. - * - * @return void - */ - public static function willReturn($fn, $value) { - self::$returns[$fn][] = $value; - } - - /** - * Take the next queued return value for $fn, or $default when none is left. - * - * @param string $fn Cacti function name. - * @param mixed $default Fallback when the queue is empty. - * - * @return mixed - */ - public static function nextReturn($fn, $default) { - if (!empty(self::$returns[$fn])) { - return array_shift(self::$returns[$fn]); - } - - return $default; - } - - /** - * All recorded calls to $fn. - * - * @param string $fn Cacti function name. - * - * @return array}> - */ - public static function callsTo($fn) { - return array_values(array_filter(self::$calls, function ($call) use ($fn) { - return $call['fn'] === $fn; - })); - } - - /** - * The recorded call log reduced to function names, in order. Useful for - * asserting transaction sequencing. - * - * @return array - */ - public static function callSequence() { - return array_column(self::$calls, 'fn'); - } -} diff --git a/tests/Unit/GetCurrentValueTest.php b/tests/Unit/GetCurrentValueTest.php index f82a2c44..f467b9bc 100644 --- a/tests/Unit/GetCurrentValueTest.php +++ b/tests/Unit/GetCurrentValueTest.php @@ -35,10 +35,10 @@ public static function setUpBeforeClass(): void { protected function setUp(): void { parent::setUp(); - CactiStub::willReturn('db_fetch_row_prepared', ['rrd_step' => 300]); + CactiStubs::willReturn('db_fetch_row_prepared', ['rrd_step' => 300]); // thold_rrd_last() returns whatever `rrdtool last` printed. - CactiStub::willReturn('rrdtool_execute', '1700000000'); + CactiStubs::willReturn('rrdtool_execute', '1700000000'); } /** @@ -47,7 +47,7 @@ protected function setUp(): void { * @return void */ private function rrdReturns(array $fetch) { - CactiStub::willReturn('rrdtool_function_fetch', $fetch); + CactiStubs::willReturn('rrdtool_function_fetch', $fetch); } /** @@ -126,9 +126,9 @@ public function testEmptyValueSeriesReturnsZero(): void { * @return void */ public function testUnreadableRrdReturnsZeroRatherThanThrowing(): void { - CactiStub::reset(); - CactiStub::willReturn('db_fetch_row_prepared', ['rrd_step' => 300]); - CactiStub::willReturn('rrdtool_execute', ''); + CactiStubs::reset(); + CactiStubs::willReturn('db_fetch_row_prepared', ['rrd_step' => 300]); + CactiStubs::willReturn('rrdtool_execute', ''); $this->rrdReturns([]); $this->assertSame(0, get_current_value(4, 'traffic_in')); diff --git a/tests/bootstrap.php b/tests/bootstrap.php deleted file mode 100644 index e9d4d5ee..00000000 --- a/tests/bootstrap.php +++ /dev/null @@ -1,384 +0,0 @@ - dirname(dirname(dirname(__DIR__))), - 'url_path' => '/cacti/', - 'cacti_version' => '1.2.31', - 'cacti_server_os' => 'unix', -]; - -// thold_expand_string() include_once()s library_path/variables.php at call time. -$GLOBALS['config']['library_path'] = __DIR__ . '/fixtures/cacti-lib'; - -// thold reads and writes this on every RPN evaluation. -$GLOBALS['rpn_error'] = false; - -if (!function_exists('db_execute')) { - function db_execute($sql, $log = true, $db_conn = false) { - CactiStub::record('db_execute', $sql); - - return CactiStub::nextReturn('db_execute', true); - } -} - -if (!function_exists('db_execute_prepared')) { - function db_execute_prepared($sql, $params = [], $log = true, $db_conn = false) { - CactiStub::record('db_execute_prepared', $sql, $params); - - return CactiStub::nextReturn('db_execute_prepared', true); - } -} - -if (!function_exists('db_fetch_assoc')) { - function db_fetch_assoc($sql, $log = true, $db_conn = false) { - CactiStub::record('db_fetch_assoc', $sql); - - return CactiStub::nextReturn('db_fetch_assoc', []); - } -} - -if (!function_exists('db_fetch_assoc_prepared')) { - function db_fetch_assoc_prepared($sql, $params = [], $log = true, $db_conn = false) { - CactiStub::record('db_fetch_assoc_prepared', $sql, $params); - - return CactiStub::nextReturn('db_fetch_assoc_prepared', []); - } -} - -if (!function_exists('db_fetch_row')) { - function db_fetch_row($sql, $log = true, $db_conn = false) { - CactiStub::record('db_fetch_row', $sql); - - return CactiStub::nextReturn('db_fetch_row', []); - } -} - -if (!function_exists('db_fetch_row_prepared')) { - function db_fetch_row_prepared($sql, $params = [], $log = true, $db_conn = false) { - CactiStub::record('db_fetch_row_prepared', $sql, $params); - - return CactiStub::nextReturn('db_fetch_row_prepared', []); - } -} - -if (!function_exists('db_fetch_cell')) { - function db_fetch_cell($sql, $col_name = '', $log = true, $db_conn = false) { - CactiStub::record('db_fetch_cell', $sql); - - return CactiStub::nextReturn('db_fetch_cell', ''); - } -} - -if (!function_exists('db_fetch_cell_prepared')) { - function db_fetch_cell_prepared($sql, $params = [], $col_name = '', $log = true, $db_conn = false) { - CactiStub::record('db_fetch_cell_prepared', $sql, $params); - - return CactiStub::nextReturn('db_fetch_cell_prepared', ''); - } -} - -if (!function_exists('db_qstr')) { - function db_qstr($string) { - return "'" . str_replace("'", "''", (string) $string) . "'"; - } -} - -if (!function_exists('db_begin_transaction')) { - function db_begin_transaction() { - CactiStub::record('db_begin_transaction'); - - return CactiStub::nextReturn('db_begin_transaction', true); - } -} - -if (!function_exists('db_commit_transaction')) { - function db_commit_transaction() { - CactiStub::record('db_commit_transaction'); - - return CactiStub::nextReturn('db_commit_transaction', true); - } -} - -if (!function_exists('db_rollback_transaction')) { - function db_rollback_transaction() { - CactiStub::record('db_rollback_transaction'); - - return CactiStub::nextReturn('db_rollback_transaction', true); - } -} - -if (!function_exists('html_escape')) { - function html_escape($string) { - return htmlspecialchars((string) $string, ENT_QUOTES, 'UTF-8'); - } -} - -/* - * Mirrors Cacti 1.2 lib/functions.php. KEEP IN SYNC: if core tightens its - * checks, tests here would otherwise keep passing while production diverges. - */ -if (!function_exists('sanitize_unserialize_selected_items')) { - function sanitize_unserialize_selected_items($items) { - if (empty($items)) { - return false; - } - - $data = unserialize($items, ['allowed_classes' => false]); // nosemgrep: php.lang.security.unserialize-use.unserialize-use -- test stub mirroring Cacti core; allowed_classes:false blocks object injection - - if (!is_array($data)) { - return false; - } - - foreach ($data as $value) { - if (!is_numeric($value)) { - return false; - } - } - - return $data; - } -} - -if (!function_exists('read_config_option')) { - function read_config_option($name, $force = false) { - return isset(CactiStub::$configOptions[$name]) ? CactiStub::$configOptions[$name] : ''; - } -} - -if (!function_exists('set_config_option')) { - function set_config_option($name, $value) { - CactiStub::$configOptions[$name] = $value; - } -} - -if (!function_exists('__')) { - function __($text) { - $args = array_slice(func_get_args(), 1); - - // Cacti's __() accepts sprintf arguments after the format string. - return $args === [] ? $text : vsprintf($text, $args); - } -} - -if (!function_exists('__esc')) { - function __esc($text) { - return htmlspecialchars(call_user_func_array('__', func_get_args()), ENT_QUOTES, 'UTF-8'); - } -} - -if (!function_exists('cacti_log')) { - function cacti_log($message, $output = false, $environ = 'CMDPHP', $level = 0) { - CactiStub::$log[] = $message; - } -} - -if (!function_exists('cacti_sizeof')) { - function cacti_sizeof($array) { - return (is_array($array) || $array instanceof Countable) ? count($array) : 0; - } -} - -if (!function_exists('cacti_count')) { - function cacti_count($array) { - return cacti_sizeof($array); - } -} - -if (!function_exists('get_request_var')) { - function get_request_var($name, $default = '') { - return isset(CactiStub::$requestVars[$name]) ? CactiStub::$requestVars[$name] : $default; - } -} - -if (!function_exists('get_nfilter_request_var')) { - function get_nfilter_request_var($name, $default = '') { - return get_request_var($name, $default); - } -} - -if (!function_exists('get_filter_request_var')) { - function get_filter_request_var($name, $filter = FILTER_VALIDATE_INT, $options = []) { - return get_request_var($name); - } -} - -if (!function_exists('isset_request_var')) { - function isset_request_var($name) { - return isset(CactiStub::$requestVars[$name]); - } -} - -if (!function_exists('cacti_escapeshellarg')) { - function cacti_escapeshellarg($string, $quote = true) { - return escapeshellarg((string) $string); - } -} - -if (!function_exists('api_plugin_hook_function')) { - function api_plugin_hook_function($name, $data = '') { - return $data; - } -} - -if (!function_exists('get_simple_graph_perms')) { - function get_simple_graph_perms($user_id) { - return CactiStub::nextReturn('get_simple_graph_perms', true); - } -} - -if (!function_exists('get_policies')) { - function get_policies($user_id) { - return CactiStub::nextReturn('get_policies', []); - } -} - -if (!function_exists('get_policy_where')) { - function get_policy_where($graph_auth_method, $policies, $sql_where) { - CactiStub::record('get_policy_where', $sql_where); - - return CactiStub::nextReturn('get_policy_where', $sql_where); - } -} - -if (!function_exists('expand_title')) { - function expand_title($host_id, $snmp_query_id, $snmp_index, $title) { - CactiStub::record('expand_title', $title); - - return CactiStub::nextReturn('expand_title', $title); - } -} - -if (!function_exists('get_graph_title')) { - function get_graph_title($local_graph_id) { - return CactiStub::nextReturn('get_graph_title', 'Traffic - eth0'); - } -} - -if (!function_exists('rrdtool_function_fetch')) { - function rrdtool_function_fetch($local_data_id, $start_time, $end_time, $resolution = 0, $show_unknown = false, $rrdtool_file = null) { - CactiStub::record('rrdtool_function_fetch', (string) $local_data_id); - - return CactiStub::nextReturn('rrdtool_function_fetch', []); - } -} - -if (!function_exists('get_data_source_path')) { - function get_data_source_path($local_data_id, $expand_paths = true) { - return '/var/lib/cacti/rra/test_' . (int) $local_data_id . '.rrd'; - } -} - -if (!function_exists('rrdtool_execute')) { - function rrdtool_execute($command, $log_to_stdout = false, $output_flag = 1, $rrdtool_pipe = false, $logopt = 'WEBLOG') { - CactiStub::record('rrdtool_execute', $command); - - return CactiStub::nextReturn('rrdtool_execute', ''); - } -} - -if (!function_exists('rrdtool_function_interface_speed')) { - function rrdtool_function_interface_speed($data_local) { - return CactiStub::nextReturn('rrdtool_function_interface_speed', 0); - } -} - -if (!function_exists('get_timeinstate')) { - function get_timeinstate($host) { - return CactiStub::nextReturn('get_timeinstate', '1 day'); - } -} - -if (!function_exists('get_daysfromtime')) { - function get_daysfromtime($timestamp) { - return CactiStub::nextReturn('get_daysfromtime', '1 day'); - } -} - -if (!function_exists('number_format_i18n')) { - function number_format_i18n($number, $decimals = 0, $baseu = 1000) { - return number_format((float) $number, $decimals < 0 ? 0 : (int) $decimals); - } -} - -if (!defined('FILTER_VALIDATE_IS_REGEX')) { - define('FILTER_VALIDATE_IS_REGEX', 99999); -} - -// Device states, from Cacti include/global_constants.php. -foreach (['HOST_UNKNOWN' => 0, 'HOST_DOWN' => 1, 'HOST_RECOVERING' => 2, 'HOST_UP' => 3, 'HOST_ERROR' => 4] as $name => $value) { - if (!defined($name)) { - define($name, $value); - } -} - -if (!defined('RRDTOOL_OUTPUT_STDOUT')) { - define('RRDTOOL_OUTPUT_STDOUT', 1); -} - -if (!defined('CACTI_DATE_TIME_FORMAT')) { - define('CACTI_DATE_TIME_FORMAT', 'Y-m-d H:i:s'); -} - -if (!defined('CACTI_PATH_BASE')) { - define('CACTI_PATH_BASE', $GLOBALS['config']['base_path']); -} - -/** - * Load a plugin source file at global scope. - * - * Several plugin files (includes/arrays.php in particular) define their data - * as file-scope variables that the rest of the plugin reads as globals, and - * they read $config while doing so. Requiring them from inside a method would - * make both halves of that method-local, so the require happens here and any - * variable the file introduced is published to $GLOBALS. - * - * @param string $path Absolute path to the file. - * - * @return void - */ -function thold_test_load($path) { - global $config; - - $__before = get_defined_vars(); - - require_once $path; - - foreach (get_defined_vars() as $__name => $__value) { - if (!array_key_exists($__name, $__before) && strncmp($__name, '__', 2) !== 0) { - $GLOBALS[$__name] = $__value; - } - } -} diff --git a/tests/docker/Dockerfile b/tests/docker/Dockerfile index cb4279d1..518f7321 100644 --- a/tests/docker/Dockerfile +++ b/tests/docker/Dockerfile @@ -7,7 +7,8 @@ FROM php:8.1-cli-alpine@sha256:7949370448b0b4d9787776dc5968e0fd8d48763292344b5fb # git is needed by the changed-line coverage gate, which diffs against the # base branch. -RUN apk add --no-cache git \ +RUN apk add --no-cache git gmp-dev \ + && docker-php-ext-install gmp \ && apk add --no-cache --virtual .build-deps $PHPIZE_DEPS \ && pecl install pcov \ && docker-php-ext-enable pcov \ @@ -25,4 +26,4 @@ ENV COMPOSER_ALLOW_SUPERUSER=1 \ COMPOSER_NO_INTERACTION=1 \ COMPOSER_CACHE_DIR=/tmp/composer-cache -CMD ["sh", "-c", "composer install --no-progress --no-ansi && vendor/bin/phpunit"] +CMD ["sh", "-c", "composer install --no-progress --no-ansi && composer test"] From 01c6882c6aa72a3468e2c93a4cb47d84909b9627 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Mon, 17 Aug 2026 14:47:13 -0700 Subject: [PATCH 5/7] ci: keep plugin PR integration checks on pinned Cacti --- .github/workflows/plugin-ci-workflow.yml | 9 --------- 1 file changed, 9 deletions(-) diff --git a/.github/workflows/plugin-ci-workflow.yml b/.github/workflows/plugin-ci-workflow.yml index 5e4f3db6..79b16b7b 100644 --- a/.github/workflows/plugin-ci-workflow.yml +++ b/.github/workflows/plugin-ci-workflow.yml @@ -35,21 +35,12 @@ jobs: integration-test: runs-on: ${{ matrix.os }} - # A failure against the pinned release is a real failure. The develop entry - # is advisory: it is how a core regression becomes visible here, but it must - # not turn the plugin's own pull requests red. - continue-on-error: ${{ matrix.cacti != 'release/1.2.31' }} - strategy: fail-fast: false matrix: php: ['8.1', '8.2', '8.3', '8.4'] os: [ubuntu-latest] cacti: ['release/1.2.31'] - include: - - php: '8.4' - os: ubuntu-latest - cacti: 'develop' services: mariadb: From 1e632a7e458072b2623840a0f0bdd1ba5bf36c64 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Mon, 17 Aug 2026 14:51:10 -0700 Subject: [PATCH 6/7] test: match the optional Cacti database stub signature --- tests/bootstrap-unit.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bootstrap-unit.php b/tests/bootstrap-unit.php index ce2b2615..41a83a31 100644 --- a/tests/bootstrap-unit.php +++ b/tests/bootstrap-unit.php @@ -157,7 +157,7 @@ function db_fetch_cell_prepared($sql, $params = [], $col_name = '', $log = true, } if (!function_exists('db_qstr')) { - function db_qstr($string) { + function db_qstr($string, $db_conn = false) { return "'" . str_replace("'", "''", (string) $string) . "'"; } } From 62db59e6db3dedc3182e9e86598628bbbf208ef5 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Mon, 17 Aug 2026 14:59:50 -0700 Subject: [PATCH 7/7] ci: bound package index refreshes --- .github/workflows/plugin-ci-workflow.yml | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/.github/workflows/plugin-ci-workflow.yml b/.github/workflows/plugin-ci-workflow.yml index 79b16b7b..e17554e8 100644 --- a/.github/workflows/plugin-ci-workflow.yml +++ b/.github/workflows/plugin-ci-workflow.yml @@ -86,7 +86,24 @@ jobs: echo "PHP_BINARY=$(command -v php)" >> "$GITHUB_ENV" - name: Run apt-get update - run: sudo apt-get update + run: | + for attempt in 1 2 3; do + if sudo timeout 3m apt-get \ + -o Dpkg::Lock::Timeout=60 \ + -o Acquire::Retries=3 \ + -o Acquire::http::Timeout=30 \ + -o Acquire::https::Timeout=30 \ + update; then + exit 0 + fi + + if [ "$attempt" -lt 3 ]; then + sleep 10 + fi + done + + echo 'apt-get update failed after three bounded attempts.' >&2 + exit 1 - name: Install System Dependencies run: sudo apt-get install -y apache2 snmp snmpd rrdtool fping