fix(thold): make the unit suffix conversions inverses of each other

.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/TestCase.php

@@ -31,6 +31,7 @@ protected function setUp(): void {
 
 		CactiStubs::reset();
 		$GLOBALS['rpn_error'] = false;
+		$_SESSION = [];
 	}
 
 	/**

tests/Unit/TholdUnitSuffixTest.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/                                                   |
+ +-------------------------------------------------------------------------+
+*/
+
+/**
+ * The pair that moves a threshold bound between what an operator types and
+ * what is stored.
+ *
+ * They have to be inverses. A threshold is read out of the database, rendered
+ * into the form, and written back on every save, so any disagreement between
+ * them compounds each time the form is opened.
+ */
+final class TholdUnitSuffixTest extends TestCase {
+	/**
+	 * @return void
+	 */
+	public static function setUpBeforeClass(): void {
+		self::loadPluginSource('thold_functions.php');
+	}
+
+	/**
+	 * @return array<string, array{0: string, 1: float}>
+	 */
+	public static function suffixProvider() {
+		return [
+			'yocto' => ['5y', 5.0e-24],
+			'zepto' => ['5z', 5.0e-21],
+			'atto'  => ['5a', 5.0e-18],
+			'femto' => ['5f', 5.0e-15],
+			'pico'  => ['5p', 5.0e-12],
+			'nano'  => ['5n', 5.0e-9],
+			'micro' => ['5u', 5.0e-6],
+			'milli' => ['5m', 5.0e-3],
+			'kilo'  => ['5K', 5.0e3],
+			'mega'  => ['5M', 5.0e6],
+			'giga'  => ['5G', 5.0e9],
+			'tera'  => ['5T', 5.0e12],
+			'peta'  => ['5P', 5.0e15],
+			'exa'   => ['5E', 5.0e18],
+			'zetta' => ['5Z', 5.0e21],
+			'yotta' => ['5Y', 5.0e24],
+		];
+	}
+
+	/**
+	 * @dataProvider suffixProvider
+	 *
+	 * @param string $typed
+	 * @param float  $stored
+	 *
+	 * @return void
+	 */
+	public function testEachSuffixScalesByItsSiFactor($typed, $stored): void {
+		$this->assertEqualsWithDelta($stored, thold_display_to_raw($typed, 'thold_hi'), abs($stored) * 1.0e-9);
+	}
+
+	/**
+	 * @dataProvider suffixProvider
+	 *
+	 * @param string $typed
+	 * @param float  $stored
+	 *
+	 * @return void
+	 */
+	public function testEachStoredValueRendersWithItsSiSuffix($typed, $stored): void {
+		$this->assertSame($typed, thold_raw_to_display($stored));
+	}
+
+	/**
+	 * Opening a threshold and saving it again must not change it. This is the
+	 * failure that mattered: a value stored at 1e-12 rendered as 5f, which
+	 * parsed back as 1e-15, so every visit to the form divided it by a
+	 * thousand.
+	 *
+	 * @dataProvider suffixProvider
+	 *
+	 * @param string $typed
+	 * @param float  $stored
+	 *
+	 * @return void
+	 */
+	public function testAValueSurvivesBeingDisplayedAndReEntered($typed, $stored): void {
+		$round_tripped = thold_display_to_raw(thold_raw_to_display($stored), 'thold_hi');
+
+		$this->assertEqualsWithDelta($stored, $round_tripped, abs($stored) * 1.0e-9);
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testAPlainNumberIsLeftAlone(): void {
+		$this->assertSame('42', thold_display_to_raw('42', 'thold_hi'));
+		$this->assertSame('42', thold_raw_to_display(42));
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testZeroIsLeftAlone(): void {
+		$this->assertSame('0', thold_raw_to_display(0));
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testNegativeValuesKeepTheirSign(): void {
+		$this->assertSame('-5K', thold_raw_to_display(-5000));
+		$this->assertEqualsWithDelta(-5000, thold_display_to_raw('-5K', 'thold_hi'), 1.0e-6);
+	}
+
+	/**
+	 * @return array<string, array{0: string}>
+	 */
+	public static function rejectedInputProvider() {
+		return [
+			'unknown suffix' => ['5x'],
+			'letters only'   => ['abc'],
+			'empty'          => [''],
+		];
+	}
+
+	/**
+	 * @dataProvider rejectedInputProvider
+	 *
+	 * @param string $typed
+	 *
+	 * @return void
+	 */
+	public function testUnusableInputIsRejectedAndFlagged($typed): void {
+		$this->assertFalse(thold_display_to_raw($typed, 'thold_hi'));
+		$this->assertArrayHasKey('thold_hi', $_SESSION['sess_error_fields']);
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testNonNumericInputHasNoDisplayForm(): void {
+		$this->assertFalse(thold_raw_to_display('abc'));
+	}
+
+	/**
+	 * Beyond the largest and smallest suffix there is nothing left to index,
+	 * and the old code read past the end of the pattern and dropped the
+	 * magnitude entirely.
+	 *
+	 * @return void
+	 */
+	public function testMagnitudesBeyondTheLargestSuffixKeepTheirScale(): void {
+		$rendered = thold_raw_to_display(5.0e27);
+
+		$this->assertNotSame('5', $rendered);
+		$this->assertEqualsWithDelta(5.0e27, (float) thold_display_to_raw($rendered, 'thold_hi'), 5.0e18);
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testMagnitudesBelowTheSmallestSuffixKeepTheirScale(): void {
+		$rendered = thold_raw_to_display(5.0e-18);
+
+		$this->assertNotSame('5', $rendered);
+		$this->assertEqualsWithDelta(5.0e-18, (float) thold_display_to_raw($rendered, 'thold_hi'), 5.0e-27);
+	}
+}

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: