fix(thold): correct counter delta, wrap modulus and percent denominator

.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:
@@ -95,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

tests/Unit/TholdCalculatePercentTest.php

@@ -0,0 +1,100 @@
+<?php
+/*
+ +-------------------------------------------------------------------------+
+ | Copyright (C) 2004-2026 The Cacti Group                                 |
+ |                                                                         |
+ | This program is free software; you can redistribute it and/or           |
+ | modify it under the terms of the GNU General Public License             |
+ | as published by the Free Software Foundation; either version 2          |
+ | of the License, or (at your option) any later version.                  |
+ +-------------------------------------------------------------------------+
+ | Cacti: The Complete RRDtool-based Graphing Solution                     |
+ +-------------------------------------------------------------------------+
+ | http://www.cacti.net/                                                   |
+ +-------------------------------------------------------------------------+
+*/
+
+/**
+ * thold_calculate_percent() expresses a reading as a percentage of a second
+ * data source on the same RRD.
+ *
+ * An empty string is the engine's "no usable value" sentinel; zero is a real
+ * percentage and is compared against the bounds as one.
+ */
+final class TholdCalculatePercentTest extends TestCase {
+	/**
+	 * @return void
+	 */
+	public static function setUpBeforeClass(): void {
+		self::loadPluginSource('thold_functions.php');
+	}
+
+	/**
+	 * @return array<string, mixed>
+	 */
+	private function threshold() {
+		return ['percent_ds' => 'total', 'local_data_id' => 4];
+	}
+
+	/**
+	 * @param float|int|string $denominator
+	 * @param float|int|string $reading
+	 *
+	 * @return mixed
+	 */
+	private function percent($denominator, $reading = 50) {
+		return thold_calculate_percent($this->threshold(), $reading, [4 => ['total' => $denominator]]);
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testReadingIsExpressedAsAPercentageOfTheReference(): void {
+		$this->assertEqualsWithDelta(25, $this->percent(200), 1.0e-9);
+	}
+
+	/**
+	 * A denominator below one used to truncate to zero, forcing the result to
+	 * zero and keeping any configured low threshold in permanent breach.
+	 *
+	 * @return void
+	 */
+	public function testFractionalDenominatorIsNotTruncated(): void {
+		$this->assertEqualsWithDelta(1000, $this->percent(0.5, 5), 1.0e-9);
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testNegativeDenominatorGivesANegativePercentage(): void {
+		$this->assertEqualsWithDelta(-25, $this->percent(-200), 1.0e-9);
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testZeroDenominatorGivesZeroRatherThanDividingByZero(): void {
+		$this->assertSame(0, $this->percent(0));
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testNonNumericDenominatorGivesZero(): void {
+		$this->assertSame(0, $this->percent('U'));
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testNonNumericReadingYieldsTheNoValueSentinel(): void {
+		$this->assertSame('', $this->percent(200, 'U'));
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testMissingReferenceDataSourceYieldsTheNoValueSentinel(): void {
+		$this->assertSame('', thold_calculate_percent($this->threshold(), 50, [4 => ['other' => 200]]));
+	}
+}

tests/Unit/TholdGetCurrentvalTest.php

@@ -0,0 +1,176 @@
+<?php
+/*
+ +-------------------------------------------------------------------------+
+ | Copyright (C) 2004-2026 The Cacti Group                                 |
+ |                                                                         |
+ | This program is free software; you can redistribute it and/or           |
+ | modify it under the terms of the GNU General Public License             |
+ | as published by the Free Software Foundation; either version 2          |
+ | of the License, or (at your option) any later version.                  |
+ +-------------------------------------------------------------------------+
+ | Cacti: The Complete RRDtool-based Graphing Solution                     |
+ +-------------------------------------------------------------------------+
+ | http://www.cacti.net/                                                   |
+ +-------------------------------------------------------------------------+
+*/
+
+/**
+ * thold_get_currentval() turns a raw sample into the rate a threshold is
+ * compared against, per the data source type.
+ */
+final class TholdGetCurrentvalTest extends TestCase {
+	const GAUGE    = 1;
+	const COUNTER  = 2;
+	const DERIVE   = 3;
+	const ABSOLUTE = 4;
+
+	/**
+	 * @return void
+	 */
+	public static function setUpBeforeClass(): void {
+		self::loadPluginSource('thold_functions.php');
+	}
+
+	/**
+	 * @param array<string, mixed> $overrides
+	 *
+	 * @return array<string, mixed>
+	 */
+	private function threshold(array $overrides = []) {
+		return $overrides + [
+			'local_data_id'        => 4,
+			'name'                 => 'traffic_in',
+			'data_source_type_id'  => self::COUNTER,
+			'rrd_step'             => 300,
+			'rrd_maximum'          => 0,
+			'lasttime'             => 0,
+			'oldvalue'             => 100,
+		];
+	}
+
+	/**
+	 * @param array<string, mixed> $thold
+	 * @param float|int|string     $reading
+	 *
+	 * @return mixed
+	 */
+	private function currentValue(array $thold, $reading) {
+		$reindexed      = [4 => ['traffic_in' => $reading]];
+		$time_reindexed = [4 => 1700000300];
+		$item           = [];
+		$currenttime    = 0;
+
+		return thold_get_currentval($thold, $reindexed, $time_reindexed, $item, $currenttime);
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testGaugeReturnsTheReadingUnchanged(): void {
+		$thold = $this->threshold(['data_source_type_id' => self::GAUGE]);
+
+		$this->assertSame(42, $this->currentValue($thold, 42));
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testAbsoluteDividesTheReadingByTheStep(): void {
+		$thold = $this->threshold(['data_source_type_id' => self::ABSOLUTE]);
+
+		$this->assertEqualsWithDelta(2, $this->currentValue($thold, 600), 1.0e-9);
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testCounterReturnsTheDeltaOverTheStep(): void {
+		$thold = $this->threshold(['oldvalue' => 100]);
+
+		$this->assertEqualsWithDelta(2, $this->currentValue($thold, 700), 1.0e-9);
+	}
+
+	/**
+	 * A counter that legitimately read zero last cycle is not the same as
+	 * having no previous reading. Treating it as absent reports a rate of zero
+	 * for the first interval after a device reboot.
+	 *
+	 * @return void
+	 */
+	public function testCounterTreatsAPreviousReadingOfZeroAsReal(): void {
+		$thold = $this->threshold(['oldvalue' => 0]);
+
+		$this->assertEqualsWithDelta(2, $this->currentValue($thold, 600), 1.0e-9);
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testCounterWithNoPreviousReadingYieldsZero(): void {
+		$thold = $this->threshold(['oldvalue' => '']);
+
+		$this->assertSame(0, $this->currentValue($thold, 600));
+	}
+
+	/**
+	 * A 32-bit counter that wraps has advanced by (2^32 - old) + new. Using
+	 * 2^32-1 as the modulus loses exactly one count per wrap.
+	 *
+	 * @return void
+	 */
+	public function testThirtyTwoBitWrapUsesTheCorrectModulus(): void {
+		$thold = $this->threshold(['oldvalue' => 4294967290, 'rrd_step' => 1]);
+
+		$this->assertEqualsWithDelta(11, $this->currentValue($thold, 5), 1.0e-9);
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testSixtyFourBitWrapUsesTheCorrectModulus(): void {
+		$thold = $this->threshold(['oldvalue' => '18446744073709551610', 'rrd_step' => 1]);
+
+		$this->assertEqualsWithDelta(11, $this->currentValue($thold, 5), 1.0e-9);
+	}
+
+	/**
+	 * RRD values can arrive in scientific notation. GMP accepts only integer
+	 * strings, so these values must use the non-fatal floating-point fallback.
+	 *
+	 * @return void
+	 */
+	public function testSixtyFourBitWrapAcceptsScientificNotation(): void {
+		$thold = $this->threshold(['oldvalue' => '1.8446744073709552E+19', 'rrd_step' => 1]);
+
+		$this->assertEqualsWithDelta(5, $this->currentValue($thold, 5), 1.0e-9);
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testDeriveDividesTheDeltaByTheStep(): void {
+		$thold = $this->threshold(['data_source_type_id' => self::DERIVE, 'oldvalue' => 100]);
+
+		$this->assertEqualsWithDelta(2, $this->currentValue($thold, 700), 1.0e-9);
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testNonNumericReadingYieldsTheNoValueSentinel(): void {
+		$this->assertSame('', $this->currentValue($this->threshold(), 'U'));
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testMissingDataSourceYieldsTheNoValueSentinel(): void {
+		$thold          = $this->threshold();
+		$reindexed      = [];
+		$time_reindexed = [4 => 1700000300];
+		$item           = [];
+		$currenttime    = 0;
+
+		$this->assertSame('', thold_get_currentval($thold, $reindexed, $time_reindexed, $item, $currenttime));
+	}
+}

tests/docker/Dockerfile

@@ -0,0 +1,29 @@
+# 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 gmp-dev \
+	&& docker-php-ext-install gmp \
+	&& 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 && composer test"]

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: