Fixing some test failures (#17852)

* fix controller test

* debug travis failure

* debug some more

* more debugging

* more debugging

* another debug

* another debug

* more debug + fix test?

* remove debug changes

* remove forced failure

* ui test change, check status in assert for more information in test output

* check image magick status AFTER comparison threshold check

* try to debug test performance on travis

* try to speed up some tests

* fix test change

* fix test

* update two screenshots

* update UI test again and fix random failure hopefully

* try to get rid of some more random failures

* some more debugging

* try fixing some more random failures

* stop using screenshotSelector in some tests since it (inexplicably) cant find jquery on the test page + mysqli test

* another ui test tweak

* trigger new build

* try fixing more random failures

* updates expected test file

Co-authored-by: sgiehl <stefan@matomo.org>

2 files changed, 31 insertions, 21 deletions

diff --git a/plugins/UsersManager/UserAccessFilter.php b/plugins/UsersManager/UserAccessFilter.php
index df41df381a..e331c20bb3 100644
--- a/plugins/UsersManager/UserAccessFilter.php
+++ b/plugins/UsersManager/UserAccessFilter.php
@@ -116,7 +116,9 @@ class UserAccessFilter
      */
     public function filterUser($user)
     {
-        if ($this->access->hasSuperUserAccess() || $this->isNonSuperUserAllowedToSeeThisLogin($user['login'])) {
+        if ($this->access->hasSuperUserAccess()
+            || (!empty($user['login']) && $this->isNonSuperUserAllowedToSeeThisLogin($user['login']))
+        ) {
             return $user;
         }
     }
diff --git a/plugins/UsersManager/tests/Integration/UserAccessFilterTest.php b/plugins/UsersManager/tests/Integration/UserAccessFilterTest.php
index 76f2cda923..85cab11ab3 100644
--- a/plugins/UsersManager/tests/Integration/UserAccessFilterTest.php
+++ b/plugins/UsersManager/tests/Integration/UserAccessFilterTest.php
@@ -46,7 +46,7 @@ class UserAccessFilterTest extends IntegrationTestCase
      */
     private $filter;
 
-    private $users = array(
+    private static $users = array(
         'login2' => array('view' => array(1,3,5),   'admin' => array(2,6)),
         'login3' => array('view' => array(),        'admin' => array()), // no access to any site
         'login4' => array('view' => array(6),       'admin' => array()), // only access to one with view
@@ -64,13 +64,19 @@ class UserAccessFilterTest extends IntegrationTestCase
         $this->model  = new Model();
         $this->access = new FakeAccess();
 
-        $this->createManyWebsites();
-        $this->createManyUsers();
         FakeAccess::clearAccess();
 
         $this->filter = new TestUserAccessFilter($this->model, $this->access);
     }
 
+    protected static function beforeTableDataCached()
+    {
+        parent::beforeTableDataCached();
+
+        self::createManyWebsites();
+        self::createManyUsers();
+    }
+
     public function test_filterUser_WithSuperUserAccess_ShouldAlwaysReturnTrue()
     {
         $this->configureAccessForLogin('login1');
@@ -258,7 +264,7 @@ class UserAccessFilterTest extends IntegrationTestCase
         $this->assertSame(array(), $this->buildLogins(array()));
     }
 
-    private function createManyWebsites()
+    private static function createManyWebsites()
     {
         for ($i = 0; $i < 10; $i++) {
             Fixture::createWebsite('2014-01-01 00:00:00');
@@ -281,24 +287,26 @@ class UserAccessFilterTest extends IntegrationTestCase
         return $logins;
     }
 
-    private function createManyUsers()
+    private static function createManyUsers()
     {
-        $this->model->addUser('login1', md5('pass'), 'email1@example.com', '2008-01-01 00:00:00');
-        $this->model->addUser('login2', md5('pass'), 'email2@example.com', '2008-01-01 00:00:00');
+        $model  = new Model();
+
+        $model->addUser('login1', md5('pass'), 'email1@example.com', '2008-01-01 00:00:00');
+        $model->addUser('login2', md5('pass'), 'email2@example.com', '2008-01-01 00:00:00');
         // login3 won't have access to any site
-        $this->model->addUser('login3', md5('pass'), 'email3@example.com', '2008-01-01 00:00:00');
-        $this->model->addUser('login4', md5('pass'), 'email4@example.com', '2008-01-01 00:00:00');
-        $this->model->addUser('login5', md5('pass'), 'email5@example.com', '2008-01-01 00:00:00');
-        $this->model->addUser('login6', md5('pass'), 'email6@example.com', '2008-01-01 00:00:00');
-        $this->model->addUser('login7', md5('pass'), 'email7@example.com', '2008-01-01 00:00:00');
-        $this->model->addUser('login8', md5('pass'), 'email8@example.com', '2008-01-01 00:00:00');
-        $this->model->addUser('anonymous', '', 'ano@example.com', '2008-01-01 00:00:00');
+        $model->addUser('login3', md5('pass'), 'email3@example.com', '2008-01-01 00:00:00');
+        $model->addUser('login4', md5('pass'), 'email4@example.com', '2008-01-01 00:00:00');
+        $model->addUser('login5', md5('pass'), 'email5@example.com', '2008-01-01 00:00:00');
+        $model->addUser('login6', md5('pass'), 'email6@example.com', '2008-01-01 00:00:00');
+        $model->addUser('login7', md5('pass'), 'email7@example.com', '2008-01-01 00:00:00');
+        $model->addUser('login8', md5('pass'), 'email8@example.com', '2008-01-01 00:00:00');
+        $model->addUser('anonymous', '', 'ano@example.com', '2008-01-01 00:00:00');
 
-        $this->model->setSuperUserAccess('login1', true); // we treat this one as our superuser
+        $model->setSuperUserAccess('login1', true); // we treat this one as our superuser
 
-        foreach ($this->users as $login => $permissions) {
+        foreach (self::$users as $login => $permissions) {
             foreach ($permissions as $access => $idSites) {
-                $this->model->addUserAccess($login, $access, $idSites);
+                $model->addUserAccess($login, $access, $idSites);
             }
         }
     }
@@ -311,9 +319,9 @@ class UserAccessFilterTest extends IntegrationTestCase
 
         if ($login === 'login1') {
             $hasSuperUser = true;
-        } elseif (isset($this->users[$login])) {
-            $idSitesAdmin = $this->users[$login]['admin'];
-            $idSitesView  = $this->users[$login]['view'];
+        } elseif (isset(self::$users[$login])) {
+            $idSitesAdmin = self::$users[$login]['admin'];
+            $idSitesView  = self::$users[$login]['view'];
         }
 
         FakeAccess::clearAccess($hasSuperUser, $idSitesAdmin, $idSitesView, $login);