Improve Rrd datastore test (#16353)

* Improve Rrd datastore test
Don't start rrdtool process to test buildCommand

* lint fixes

LibreNMS/Data/Store/Rrd.php

@@ -44,28 +44,24 @@ class Rrd extends BaseDatastore
     private $sync_process;
     /** @var Proc */
     private $async_process;
+    /** @var string */
     private $rrd_dir;
+    /** @var string */
     private $version;
+    /** @var string */
     private $rrdcached;
+    /** @var string */
     private $rra;
+    /** @var int */
     private $step;
+    /** @var string */
+    private $rrdtool_executable;
 
     public function __construct()
     {
         parent::__construct();
-        $this->rrdcached = Config::get('rrdcached', false);
+        $this->loadConfig();
-
         $this->init();
-        $this->rrd_dir = Config::get('rrd_dir', Config::get('install_dir') . '/rrd');
-        $this->step = Config::get('rrd.step', 300);
-        $this->rra = Config::get(
-            'rrd_rra',
-            'RRA:AVERAGE:0.5:1:2016 RRA:AVERAGE:0.5:6:1440 RRA:AVERAGE:0.5:24:1440 RRA:AVERAGE:0.5:288:1440 ' .
-            ' RRA:MIN:0.5:1:2016 RRA:MIN:0.5:6:1440     RRA:MIN:0.5:24:1440     RRA:MIN:0.5:288:1440 ' .
-            ' RRA:MAX:0.5:1:2016 RRA:MAX:0.5:6:1440     RRA:MAX:0.5:24:1440     RRA:MAX:0.5:288:1440 ' .
-            ' RRA:LAST:0.5:1:2016 '
-        );
-        $this->version = Config::get('rrdtool_version', '1.4');
     }
 
     public function getName()
@@ -78,15 +74,31 @@ class Rrd extends BaseDatastore
         return Config::get('rrd.enable', true);
     }
 
+    protected function loadConfig(): void
+    {
+        $this->rrdcached = Config::get('rrdcached', false);
+        $this->rrd_dir = Config::get('rrd_dir', Config::get('install_dir') . '/rrd');
+        $this->step = Config::get('rrd.step', 300);
+        $this->rra = Config::get(
+            'rrd_rra',
+            'RRA:AVERAGE:0.5:1:2016 RRA:AVERAGE:0.5:6:1440 RRA:AVERAGE:0.5:24:1440 RRA:AVERAGE:0.5:288:1440 ' .
+            ' RRA:MIN:0.5:1:2016 RRA:MIN:0.5:6:1440     RRA:MIN:0.5:24:1440     RRA:MIN:0.5:288:1440 ' .
+            ' RRA:MAX:0.5:1:2016 RRA:MAX:0.5:6:1440     RRA:MAX:0.5:24:1440     RRA:MAX:0.5:288:1440 ' .
+            ' RRA:LAST:0.5:1:2016 '
+        );
+        $this->version = Config::get('rrdtool_version', '1.4');
+        $this->rrdtool_executable = Config::get('rrdtool', 'rrdtool');
+    }
+
     /**
      * Opens up a pipe to RRDTool using handles provided
      *
      * @param  bool  $dual_process  start an additional process that's output should be read after every command
      * @return bool the process(s) have been successfully started
      */
-    public function init($dual_process = true)
+    public function init($dual_process = true): bool
     {
-        $command = Config::get('rrdtool', 'rrdtool') . ' -';
+        $command = $this->rrdtool_executable . ' -';
 
         $descriptor_spec = [
             0 => ['pipe', 'r'], // stdin  is a pipe that the child will read from
@@ -94,7 +106,7 @@ class Rrd extends BaseDatastore
             2 => ['pipe', 'w'], // stderr is a pipe that the child will write to
         ];
 
-        $cwd = Config::get('rrd_dir');
+        $cwd = $this->rrd_dir;
 
         if (! $this->isSyncRunning()) {
             $this->sync_process = new Proc($command, $descriptor_spec, $cwd);
@@ -438,7 +450,7 @@ class Rrd extends BaseDatastore
      *
      * @throws FileExistsException if rrdtool <1.4.3 and the rrd file exists locally
      */
-    public function buildCommand($command, $filename, $options)
+    protected function buildCommand($command, $filename, $options): string
     {
         if ($command == 'create') {
             // <1.4.3 doesn't support -O, so make sure the file doesn't exist
@@ -579,7 +591,7 @@ class Rrd extends BaseDatastore
      */
     public function graph(string $options, array $env = null): string
     {
-        $process = new Process([Config::get('rrdtool', 'rrdtool'), '-'], $this->rrd_dir, $env);
+        $process = new Process([$this->rrdtool_executable, '-'], $this->rrd_dir, $env);
         $process->setTimeout(300);
         $process->setIdleTimeout(300);
 

LibreNMS/Proc.php

@@ -55,7 +55,7 @@ class Proc
      *
      * @param  string  $cmd  the command to execute
      * @param  array  $descriptorspec  the definition of pipes to initialize
-     * @param  null  $cwd  working directory to change to
+     * @param  string|null  $cwd  working directory to change to
      * @param  array|null  $env  array of environment variables to set
      * @param  bool  $blocking  set the output pipes to blocking (default: false)
      *

tests/RrdtoolTest.php

@@ -45,7 +45,6 @@ class RrdtoolTest extends TestCase
         $cmd = $this->buildCommandProxy('update', '/opt/librenms/rrd/f', 'o');
         $this->assertEquals('update /opt/librenms/rrd/f o', $cmd);
 
-        $this->app->forgetInstance(Rrd::class);
         Config::set('rrdtool_version', '1.6');
 
         $cmd = $this->buildCommandProxy('create', '/opt/librenms/rrd/f', 'o');
@@ -73,7 +72,6 @@ class RrdtoolTest extends TestCase
         $cmd = $this->buildCommandProxy('update', '/opt/librenms/rrd/f', 'o');
         $this->assertEquals('update f o --daemon server:42217', $cmd);
 
-        $this->app->forgetInstance(Rrd::class);
         Config::set('rrdtool_version', '1.6');
 
         $cmd = $this->buildCommandProxy('create', '/opt/librenms/rrd/f', 'o');
@@ -98,11 +96,11 @@ class RrdtoolTest extends TestCase
 
     private function buildCommandProxy($command, $filename, $options)
     {
-        // todo better tests
+        $mock = $this->mock(Rrd::class)->makePartial(); // avoid constructor
-        $reflection = new \ReflectionClass(Rrd::class);
+        // @phpstan-ignore method.protected
-        $method = $reflection->getMethod('buildCommand');
+        $mock->loadConfig(); // load config every time to clear cached settings
-        $method->setAccessible(true);
 
-        return $method->invokeArgs($this->app->make(Rrd::class), func_get_args());
+        // @phpstan-ignore method.protected
+        return $mock->buildCommand($command, $filename, $options);
     }
 }