From ee13299a776ee7b5185397b0bb552a10d89e4110 Mon Sep 17 00:00:00 2001
From: Tony Murray <murraytony@gmail.com>
Date: Sun, 24 May 2020 13:49:01 -0500
Subject: [PATCH] Testing cleanup (#11677)

All tests use transactions
One test isn't a db test...
---
 tests/AuthHTTPTest.php  |  4 +---
 tests/AuthSSOTest.php   | 44 ++++++++++++++---------------------------
 tests/BasicApiTest.php  |  3 +++
 tests/OSModulesTest.php |  3 +++
 4 files changed, 22 insertions(+), 32 deletions(-)

diff --git a/tests/AuthHTTPTest.php b/tests/AuthHTTPTest.php
index 3ffd2eeb28..74d1b76b27 100644
--- a/tests/AuthHTTPTest.php
+++ b/tests/AuthHTTPTest.php
@@ -27,10 +27,8 @@ namespace LibreNMS\Tests;
 
 use LibreNMS\Authentication\LegacyAuth;
 use LibreNMS\Config;
-use LibreNMS\Exceptions\AuthenticationException;
 
-// Note that as this test set depends on mres(), it is a DBTestCase even though the database is unused
-class AuthHTTPTest extends DBTestCase
+class AuthHTTPTest extends TestCase
 {
     private $original_auth_mech;
     private $server;
diff --git a/tests/AuthSSOTest.php b/tests/AuthSSOTest.php
index 6e5fff7b65..d8b03cbf34 100644
--- a/tests/AuthSSOTest.php
+++ b/tests/AuthSSOTest.php
@@ -25,12 +25,15 @@
 
 namespace LibreNMS\Tests;
 
+use Illuminate\Foundation\Testing\DatabaseTransactions;
+use Illuminate\Support\Str;
 use LibreNMS\Authentication\LegacyAuth;
 use LibreNMS\Config;
 
 class AuthSSOTest extends DBTestCase
 {
-    private $last_user = null;
+    use DatabaseTransactions;
+
     private $original_auth_mech = null;
     private $server;
 
@@ -75,7 +78,7 @@ class AuthSSOTest extends DBTestCase
         $_SERVER['REMOTE_USER'] = 'test';
 
         $_SERVER['mail'] = 'test@example.org';
-        $_SERVER['displayName'] = bin2hex(openssl_random_pseudo_bytes(16));
+        $_SERVER['displayName'] = Str::random();
     }
 
 
@@ -87,34 +90,18 @@ class AuthSSOTest extends DBTestCase
         Config::set('sso.mode', 'header');
 
         $_SERVER['REMOTE_ADDR'] = '::1';
-        $_SERVER['REMOTE_USER'] = bin2hex(openssl_random_pseudo_bytes(16));
+        $_SERVER['REMOTE_USER'] = Str::random();
 
         $_SERVER['HTTP_MAIL'] = 'test@example.org';
         $_SERVER['HTTP_DISPLAYNAME'] = 'Test User';
     }
 
-    public function makeBreakUser()
+    public function makeUser()
     {
-        $this->breakUser();
+        $user = Str::random();
+        $_SERVER['REMOTE_USER'] = $user;
 
-        $u = bin2hex(openssl_random_pseudo_bytes(16));
-        $this->last_user = $u;
-        $_SERVER['REMOTE_USER'] = $u;
-
-        return $u;
-    }
-
-    public function breakUser()
-    {
-        $a = LegacyAuth::reset();
-
-        if ($this->last_user !== null) {
-            $r = $a->deleteUser($a->getUserid($this->last_user));
-            $this->last_user = null;
-            return $r;
-        }
-
-        return true;
+        return $user;
     }
 
     // Excercise general auth flow
@@ -128,7 +115,7 @@ class AuthSSOTest extends DBTestCase
 
         // Create a random username and store it with the defaults
         $this->basicEnvironmentEnv();
-        $user = $this->makeBreakUser();
+        $user = $this->makeUser();
         $this->assertTrue($a->authenticate(['username' => $user]));
 
         // Retrieve it and validate
@@ -147,7 +134,7 @@ class AuthSSOTest extends DBTestCase
 
         // Create a random username and store it with the defaults
         $this->basicEnvironmentEnv();
-        $user = $this->makeBreakUser();
+        $user = $this->makeUser();
         $this->assertTrue($a->authenticate(['username' => $user]));
 
         // Retrieve it and validate
@@ -177,7 +164,7 @@ class AuthSSOTest extends DBTestCase
 
         // Create a random username and store it with the defaults
         $this->basicEnvironmentEnv();
-        $user = $this->makeBreakUser();
+        $user = $this->makeUser();
         $this->assertTrue($a->authenticate(['username' => $user]));
 
         // Change a few things and reauth
@@ -222,13 +209,13 @@ class AuthSSOTest extends DBTestCase
         unset($_SERVER['displayName']);
         unset($_SERVER['mail']);
 
-        $this->assertTrue($a->authenticate(['username' => $this->makeBreakUser()]));
+        $this->assertTrue($a->authenticate(['username' => $this->makeUser()]));
 
         $this->basicEnvironmentHeader();
         unset($_SERVER['HTTP_DISPLAYNAME']);
         unset($_SERVER['HTTP_MAIL']);
 
-        $this->assertTrue($a->authenticate(['username' => $this->makeBreakUser()]));
+        $this->assertTrue($a->authenticate(['username' => $this->makeUser()]));
     }
 
     // Document the modules current behaviour, so that changes trigger test failures
@@ -472,7 +459,6 @@ class AuthSSOTest extends DBTestCase
     {
         Config::set('auth_mechanism', $this->original_auth_mech);
         Config::forget('sso');
-        $this->breakUser();
         $_SERVER = $this->server;
         parent::tearDown();
     }
diff --git a/tests/BasicApiTest.php b/tests/BasicApiTest.php
index 3fc6425434..7e5a284ef0 100644
--- a/tests/BasicApiTest.php
+++ b/tests/BasicApiTest.php
@@ -28,9 +28,12 @@ namespace LibreNMS\Tests;
 use App\Models\ApiToken;
 use App\Models\Device;
 use App\Models\User;
+use Illuminate\Foundation\Testing\DatabaseTransactions;
 
 class BasicApiTest extends DBTestCase
 {
+    use DatabaseTransactions;
+
     public function testListDevices()
     {
         $user = factory(User::class)->state('admin')->create();
diff --git a/tests/OSModulesTest.php b/tests/OSModulesTest.php
index a3742488a9..00c300e3a3 100644
--- a/tests/OSModulesTest.php
+++ b/tests/OSModulesTest.php
@@ -26,6 +26,7 @@
 namespace LibreNMS\Tests;
 
 use DeviceCache;
+use Illuminate\Foundation\Testing\DatabaseTransactions;
 use LibreNMS\Config;
 use LibreNMS\Exceptions\FileNotFoundException;
 use LibreNMS\Exceptions\InvalidModuleException;
@@ -34,6 +35,8 @@ use LibreNMS\Util\ModuleTestHelper;
 
 class OSModulesTest extends DBTestCase
 {
+    use DatabaseTransactions;
+
     private $discoveryModules;
     private $pollerModules;