Slightly better OS module test output

Maybe faster Remove forced --debug in phpunit
murrant · Apr 25, 2023 · 80b7bd7 · 80b7bd7
1 parent 2271e29
commit 80b7bd7
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 25 deletions.
diff --git a/LibreNMS/Util/CiHelper.php b/LibreNMS/Util/CiHelper.php
@@ -178,7 +178,7 @@ public function getFlags()
      */
     public function checkUnit()
     {
-        $phpunit_cmd = [$this->checkPhpExec('phpunit'), '--colors=always', '--debug'];
+        $phpunit_cmd = [$this->checkPhpExec('phpunit'), '--colors=always'];
 
         if ($this->flags['fail-fast']) {
             array_push($phpunit_cmd, '--stop-on-error', '--stop-on-failure');

diff --git a/tests/OSModulesTest.php b/tests/OSModulesTest.php
@@ -27,13 +27,15 @@
 
 use DeviceCache;
 use Illuminate\Foundation\Testing\DatabaseTransactions;
+use Illuminate\Support\Arr;
 use LibreNMS\Config;
 use LibreNMS\Data\Source\Fping;
 use LibreNMS\Data\Source\FpingResponse;
 use LibreNMS\Exceptions\FileNotFoundException;
 use LibreNMS\Exceptions\InvalidModuleException;
 use LibreNMS\Util\Debug;
 use LibreNMS\Util\ModuleTestHelper;
+use PHPUnit\Util\Color;
 
 class OSModulesTest extends DBTestCase
 {
@@ -118,34 +120,13 @@ public function testOS($os, $variant, $modules)
         foreach ($modules as $module) {
             $expected = $expected_data[$module]['discovery'] ?? [];
             $actual = $results[$module]['discovery'] ?? [];
-            $this->assertEquals(
-                $expected,
-                $actual,
-                "OS $os: Discovered $module data does not match that found in $filename\n"
-                . print_r(array_diff($expected, $actual), true)
-                . $helper->getDiscoveryOutput($phpunit_debug ? null : $module)
-                . "\nOS $os: Discovered $module data does not match that found in $filename"
-            );
-
-            if ($module === 'route') {
-                // no route poller module
-                continue;
-            }
+            $this->checkTestData($expected, $actual, 'Discovered', $os, $module, $filename, $helper, $phpunit_debug);
 
-            if ($expected_data[$module]['poller'] == 'matches discovery') {
-                $expected = $expected_data[$module]['discovery'];
-            } else {
+            if ($expected_data[$module]['poller'] !== 'matches discovery') {
                 $expected = $expected_data[$module]['poller'] ?? [];
             }
             $actual = $results[$module]['poller'] ?? [];
-            $this->assertEquals(
-                $expected,
-                $actual,
-                "OS $os: Polled $module data does not match that found in $filename\n"
-                . print_r(array_diff($expected, $actual), true)
-                . $helper->getPollerOutput($phpunit_debug ? null : $module)
-                . "\nOS $os: Polled $module data does not match that found in $filename"
-            );
+            $this->checkTestData($expected, $actual, 'Polled', $os, $module, $filename, $helper, $phpunit_debug);
         }
 
         DeviceCache::flush(); // clear cached devices
@@ -184,4 +165,18 @@ private function stubClasses(): void
             return $mock;
         });
     }
+
+    private function checkTestData(array $expected, array $actual, string $type, string $os, mixed $module, string $filename, ModuleTestHelper $helper, bool $phpunit_debug): void
+    {
+        // try simple and fast comparison first, if that fails, do a costly/well formatted comparison
+        if ($expected != $actual) {
+            $message = Color::colorize('bg-red', "OS $os: $type $module data does not match that found in $filename");
+            $message .= PHP_EOL;
+            $message .= ($type == 'Discovered'
+                ? $helper->getDiscoveryOutput($phpunit_debug ? null : $module)
+                : $helper->getPollerOutput($phpunit_debug ? null : $module));
+
+            $this->assertSame(Arr::dot($expected), Arr::dot($actual), $message);
+        }
+    }
 }