OS Testing update (#7971)

* OS Testing update Option on pre-commit.php to run tests for just one OS. Reorganize tests a bit to support this. Improved bgp-peers capture and os module capture (initial) Better output when phpunit tests fail * fix a couple issues. Better output for another * Use the correct setup function
2024-10-07 16:52:45 +00:00 · 2017-12-28 16:12:08 -06:00
parent 12373d0f60
commit 20865b0ca4
7 changed files with 120 additions and 53 deletions
--- a/LibreNMS/Util/ModuleTestHelper.php
+++ b/LibreNMS/Util/ModuleTestHelper.php
@@ -40,6 +40,8 @@ class ModuleTestHelper
    private $json_dir;
    private $file_name;
    private $module_tables;
+    private $discovery_output;
+    private $poller_output;


    /**
@@ -377,7 +379,7 @@ class ModuleTestHelper

    public function generateTestData(Snmpsim $snmpsim, $no_save = false)
    {
-        global $device;
+        global $device, $debug, $vdebug;

        // Remove existing device in case it didn't get removed previously
        if ($existing_device = device_by_name($snmpsim->getIp())) {
@@ -402,16 +404,19 @@ class ModuleTestHelper
        // Run discovery
        if ($this->quiet) {
            ob_start();
+            $save_debug = $debug;
+            $save_vedbug = $vdebug;
+            $debug = true;
+            $vdebug = false;
        }

        discover_device($device, $this->getArgs());

        if ($this->quiet) {
-            $discovery_output = ob_get_contents();
+            $this->discovery_output = ob_get_contents();
            ob_end_clean();
-
-            d_echo($discovery_output);
-            d_echo(PHP_EOL);
+            $debug = $save_debug;
+            $vdebug = $save_vedbug;
        }

        $this->qPrint(PHP_EOL);
@@ -420,13 +425,20 @@ class ModuleTestHelper
        $data = array_merge_recursive($data, $this->dumpDb($device['device_id'], 'discovery'));

        // Run the poller
-        ob_start();
-        poll_device($device, $this->getArgs());
-        $poller_output = ob_get_contents();
-        ob_end_clean();
+        if ($this->quiet) {
+            ob_start();
+            $debug = true;
+            $vdebug = false;
+        }

-        d_echo($poller_output);
-        d_echo(PHP_EOL);
+        poll_device($device, $this->getArgs());
+
+        if ($this->quiet) {
+            $this->poller_output = ob_get_contents();
+            ob_end_clean();
+            $debug = $save_debug;
+            $vdebug = $save_vedbug;
+        }

        // Dump polled data
        $data = array_merge_recursive($data, $this->dumpDb($device['device_id'], 'poller'));
@@ -497,15 +509,22 @@ class ModuleTestHelper
                $rows = dbFetchRows("SELECT * FROM `$table` $join $where", $params);

                // remove unwanted fields
-                $keys = array_flip($info['excluded_fields']);
-                $formatted_data = array_map(function ($row) use ($keys) {
-                    return array_diff_key($row, $keys);
-                }, $rows);
+                if (isset($info['included_fields'])) {
+                    $keys = array_flip($info['included_fields']);
+                    $rows = array_map(function ($row) use ($keys) {
+                        return array_intersect_key($row, $keys);
+                    }, $rows);
+                } elseif (isset($info['excluded_fields'])) {
+                    $keys = array_flip($info['excluded_fields']);
+                    $rows = array_map(function ($row) use ($keys) {
+                        return array_diff_key($row, $keys);
+                    }, $rows);
+                }

                if (isset($key)) {
-                    $data[$module][$key][$table] = $formatted_data;
+                    $data[$module][$key][$table] = $rows;
                } else {
-                    $data[$module][$table] = $formatted_data;
+                    $data[$module][$table] = $rows;
                }
            }
        }
@@ -524,6 +543,16 @@ class ModuleTestHelper
        return array_intersect_key($this->module_tables, array_flip($this->getModules()));
    }

+    public function getLastDiscoveryOutput()
+    {
+        return $this->discovery_output;
+    }
+
+    public function getLastPollerOutput()
+    {
+        return $this->poller_output;
+    }
+
    /**
     * Get a list of all modules that support capturing data
     *
--- a/doc/Developing/os/Test-Units.md
+++ b/doc/Developing/os/Test-Units.md
@@ -29,7 +29,9 @@ one variant, please do not specify one.

 After you have saved your test data, you should run `./scripts/pre-commit.php -p -u` verify they pass.

-To run the full suite of tests enable db and snmpsim reliat tests: `./scripts/pre-commit.php --db --snmpsim -p -u`
+If you would like to test the data for a specific OS, use `./scripts/pre-commit.php -p --os osname`.
+
+To run the full suite of tests enable db and snmpsim reliant tests: `./scripts/pre-commit.php --db --snmpsim -p -u`

 ## Using snmpsim for testing

--- a/scripts/pre-commit.php
+++ b/scripts/pre-commit.php
@@ -7,11 +7,12 @@ chdir($install_dir);

 require $install_dir . '/vendor/autoload.php';

-$short_opts = 'lsufpch';
+$short_opts = 'lsufpcho:';
 $long_opts = array(
    'lint',
    'style',
    'unit',
+    'os:',
    'fail-fast',
    'passthru',
    'snmpsim',
@@ -27,6 +28,7 @@ Running $filename without options runs all checks.
  -l, --lint      Run php lint checks to test for valid syntax
  -s, --style     Run phpcs check to check for PSR-2 compliance
  -u, --unit      Run phpunit tests
+  -o, --os        Specific OS to run tests on. Implies --unit
  -f, --fail-fast Quit when any failure is encountered
  -p, --passthru  Display output from checks as it comes
      --db        Run unit tests that require a database
@@ -47,6 +49,13 @@ $completed_tests = array(
    'unit' => false,
 );

+if ($os = check_opt($options, 'os', 'o')) {
+    // enable unit tests, snmpsim, and db
+    $options['u'] = false;
+    $options['snmpsim'] = false;
+    $options['db'] = false;
+}
+
 $all = !check_opt($options, 'l', 'lint', 's', 'style', 'u', 'unit');
 if ($all) {
    // no test specified, run all tests in this order
@@ -69,7 +78,7 @@ foreach (array_keys($options) as $opt) {
    } elseif ($opt == 's' || $opt == 'style') {
        $ret = run_check('style', $passthru, $command_only);
    } elseif ($opt == 'u' || $opt == 'unit') {
-        $ret = run_check('unit', $passthru, $command_only, $fail_fast);
+        $ret = run_check('unit', $passthru, $command_only, $fail_fast, $os);
    }

    if ($fail_fast && $ret !== 0 && $ret !== 250) {
@@ -94,10 +103,10 @@ exit($return); //return the combined/single return value of tests
 * @param bool $passthru display the output as comes in
 * @param bool $command_only only display the intended command, no checks
 * @param bool $fail_fast Quit as soon as possible if error or failure
- * @param bool $snmpsim Use snmpsim
+ * @param string $os Only check a specific OS
 * @return int the return value from the check (0 = success)
 */
-function run_check($type, $passthru, $command_only, $fail_fast = false, $snmpsim = false)
+function run_check($type, $passthru, $command_only, $fail_fast = false, $os = null)
 {
    global $completed_tests;
    if (getenv('SKIP_' . strtoupper($type) . '_CHECK') || $completed_tests[$type]) {
@@ -108,7 +117,7 @@ function run_check($type, $passthru, $command_only, $fail_fast = false, $snmpsim
    $function = 'check_' . $type;
    if (function_exists($function)) {
        $completed_tests[$type] = true;
-        return $function($passthru, $command_only, $fail_fast, $snmpsim);
+        return $function($passthru, $command_only, $fail_fast, $os);
    }

    return 1;
@@ -209,9 +218,10 @@ function check_style($passthru = false, $command_only = false)
 * @param bool $passthru display the output as comes in
 * @param bool $command_only only display the intended command, no checks
 * @param bool $fail_fast Stop when any error or failure is encountered
+ * @param string $os only check a specific OS
 * @return int the return value from phpunit (0 = success)
 */
-function check_unit($passthru = false, $command_only = false, $fail_fast = false)
+function check_unit($passthru = false, $command_only = false, $fail_fast = false, $os = null)
 {
    $phpunit_bin = check_exec('phpunit');

@@ -221,6 +231,10 @@ function check_unit($passthru = false, $command_only = false, $fail_fast = false
        $phpunit_cmd .= ' --stop-on-error --stop-on-failure';
    }

+    if (!is_null($os)) {
+        $phpunit_cmd .= " --group os --filter \"@$os.*\"";
+    }
+
    if ($command_only) {
        echo $phpunit_cmd . PHP_EOL;
        return 250;
@@ -257,8 +271,17 @@ function check_opt($options)
    $args = func_get_args();
    array_shift($args);

-    $intersect = array_intersect(array_keys($options), $args);
-    return !empty($intersect);
+    foreach ($args as $option) {
+        if (isset($options[$option])) {
+            if ($options[$option] === false) {
+                // no data, return that option is enabled
+                return true;
+            }
+            return $options[$option];
+        }
+    }
+
+    return false;
 }

 /**
--- a/tests/OSDiscoveryTest.php
+++ b/tests/OSDiscoveryTest.php
@@ -25,45 +25,42 @@

 namespace LibreNMS\Tests;

+use LibreNMS\Config;
 use PHPUnit_Framework_ExpectationFailedException as PHPUnitException;

 class OSDiscoveryTest extends TestCase
 {
    private static $unchecked_files;

+    public static function setUpBeforeClass()
+    {
+        parent::setUpBeforeClass();
+
+        $glob = Config::get('install_dir') . "/tests/snmpsim/*.snmprec";
+
+        self::$unchecked_files = array_flip(array_map(function ($file) {
+            return basename($file, '.snmprec');
+        }, glob($glob)));
+    }
+
    /**
     * Populate a list of files to check and make sure it isn't empty
     */
    public function testHaveFilesToTest()
    {
-        global $config;
-
-        $glob = $config['install_dir'] . "/tests/snmpsim/*.snmprec";
-
-        self::$unchecked_files = array_flip(array_map(function ($file) {
-            return basename($file, '.snmprec');
-        }, glob($glob)));
-
        $this->assertNotEmpty(self::$unchecked_files);
    }

    /**
     * Test each OS provided by osProvider
     *
-     * @depends      testHaveFilesToTest
+     * @group os
     * @dataProvider osProvider
     * @param $os_name
     */
    public function testOS($os_name)
    {
-        global $config;
-
-        $this->assertNotEmpty(
-            self::$unchecked_files,
-            'Something wrong with the tests, $unchecked_files should be populated'
-        );
-
-        $glob = $config['install_dir'] . "/tests/snmpsim/$os_name*.snmprec";
+        $glob = Config::get('install_dir') . "/tests/snmpsim/$os_name*.snmprec";
        $files = array_map(function ($file) {
            return basename($file, '.snmprec');
        }, glob($glob));
@@ -88,7 +85,10 @@ class OSDiscoveryTest extends TestCase
     */
    public function testAllFilesTested()
    {
-        $this->assertEmpty(self::$unchecked_files);
+        $this->assertEmpty(
+            self::$unchecked_files,
+            "Not all snmprec files were checked: " . print_r(array_keys(self::$unchecked_files), true)
+        );
    }

    /**
@@ -140,10 +140,9 @@ class OSDiscoveryTest extends TestCase
     */
    public function osProvider()
    {
-        global $config;
-
        // make sure all OS are loaded
-        if (count($config['os']) < count(glob($config['install_dir'].'/includes/definitions/*.yaml'))) {
+        $config_os = array_keys(Config::get('os'));
+        if (count($config_os) < count(glob(Config::get('install_dir').'/includes/definitions/*.yaml'))) {
            load_all_os();
        }

@@ -152,11 +151,12 @@ class OSDiscoveryTest extends TestCase
            'generic',
            'ping',
        );
-        $all_os = array_diff(array_keys($config['os']), $excluded_os);
+        $filtered_os = array_diff($config_os, $excluded_os);

-        array_walk($all_os, function (&$os) {
-            $os = array($os);
-        });
+        $all_os = array();
+        foreach ($filtered_os as $os) {
+            $all_os[$os] = array($os);
+        }

        return $all_os;
    }
--- a/tests/OSModulesTest.php
+++ b/tests/OSModulesTest.php
@@ -32,6 +32,9 @@ use LibreNMS\Util\ModuleTestHelper;
 class OSModulesTest extends DBTestCase
 {
    /**
+     * Test all modules for a particular OS
+     *
+     * @group os
     * @dataProvider dumpedDataProvider
     * @param string $target_os
     * @param string $filename
@@ -55,13 +58,13 @@ class OSModulesTest extends DBTestCase
            $this->assertEquals(
                $data['discovery'],
                $results[$module]['discovery'],
-                "OS $target_os: Discovered $module data does not match that found in $filename"
+                "OS $target_os: Discovered $module data does not match that found in $filename\n" . $helper->getLastDiscoveryOutput()
            );

            $this->assertEquals(
                $data['poller'] == 'matches discovery' ? $data['discovery'] : $data['poller'],
                $results[$module]['poller'],
-                "OS $target_os: Polled $module data does not match that found in $filename"
+                "OS $target_os: Polled $module data does not match that found in $filename\n" . $helper->getLastPollerOutput()
            );
        }
    }
--- a/tests/data/routeros.json
+++ b/tests/data/routeros.json
@@ -90,6 +90,11 @@
    },
    "bgp-peers": {
        "discovery": {
+            "devices": [
+                {
+                    "bgpLocalAs": null
+                }
+            ],
            "bgpPeers": [],
            "bgpPeers_cbgp": []
        },
--- a/tests/module_tables.yaml
+++ b/tests/module_tables.yaml
@@ -10,6 +10,8 @@ arp-table:
    ipv4_mac:
        excluded_fields: [device_id, port_id]
 bgp-peers:
+    devices:
+        included_fields: [bgpLocalAs]
    bgpPeers:
        excluded_fields: [device_id, bgpPeer_id]
    bgpPeers_cbgp:
@@ -22,6 +24,9 @@ ports:
        excluded_fields: [device_id, port_id, poll_time, poll_period]
        joins:
            - { left: ports.port_id, right: ports_statistics.port_id }
+os:
+    devices:
+        included_fields: [sysName, sysObjectID, sysDescr, sysContact, version, hardware, features, location, os, type, serial, icon]
 processors:
    processors:
        excluded_fields: [device_id, processor_id]