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>

8 files changed, 78 insertions, 52 deletions

diff --git a/plugins/CoreHome/tests/Integration/Column/UserIdTest.php b/plugins/CoreHome/tests/Integration/Column/UserIdTest.php
index 306a18fa33..848d933f5a 100644
--- a/plugins/CoreHome/tests/Integration/Column/UserIdTest.php
+++ b/plugins/CoreHome/tests/Integration/Column/UserIdTest.php
@@ -33,26 +33,21 @@ class UserIdTest extends IntegrationTestCase
 
     protected $date = '2014-04-04';
 
-    public function setUp(): void
+    protected static function beforeTableDataCached()
     {
-        parent::setUp();
-        $this->userId = new UserId();
-
-        $this->setSuperUser();
+        parent::beforeTableDataCached();
 
         Fixture::createSuperUser();
         Fixture::createWebsite('2014-01-01 00:00:00');
         Fixture::createWebsite('2014-01-01 00:00:00');
     }
 
-    public function tearDown(): void
+    public function setUp(): void
     {
-        // clean up your test here if needed
-        $tables = ArchiveTableCreator::getTablesArchivesInstalled();
-        if (!empty($tables)) {
-            Db::dropTables($tables);
-        }
-        parent::tearDown();
+        parent::setUp();
+        $this->userId = new UserId();
+
+        $this->setSuperUser();
     }
 
     public function test_isUsedInAtLeastOneSite_shouldReturnFalseByDefault_WhenNothingIsTracked()
diff --git a/plugins/CoreUpdater/tests/Integration/Commands/UpdateTest.php b/plugins/CoreUpdater/tests/Integration/Commands/UpdateTest.php
index a04003303b..e00697c1fb 100644
--- a/plugins/CoreUpdater/tests/Integration/Commands/UpdateTest.php
+++ b/plugins/CoreUpdater/tests/Integration/Commands/UpdateTest.php
@@ -102,8 +102,6 @@ class UpdateTest extends ConsoleCommandTestCase
 
     public function test_UpdateCommand_ReturnsCorrectExitCode_WhenErrorOccurs()
     {
-        $this->fail('This test currently lets PHPUnit fatal completely. Please investigate!');
-
         // create a blob table, then drop it manually so update 2.10.0-b10 will fail
         $tableName = ArchiveTableCreator::getBlobTable(Date::factory('2015-01-01'));
         Db::exec("DROP TABLE $tableName");
diff --git a/plugins/LogViewer b/plugins/LogViewer
-Subproject f7956375e0ef459504a0d0d1b762cde93d6d1c4
+Subproject 9c893d29c64f5ca60ed093cf553b897c83a755c
diff --git a/plugins/PrivacyManager/tests/Integration/DataPurgingTest.php b/plugins/PrivacyManager/tests/Integration/DataPurgingTest.php
index 0d39cb36d6..6c29f6de17 100644
--- a/plugins/PrivacyManager/tests/Integration/DataPurgingTest.php
+++ b/plugins/PrivacyManager/tests/Integration/DataPurgingTest.php
@@ -101,6 +101,8 @@ class DataPurgingTest extends IntegrationTestCase
     {
         parent::setUp();
 
+        $GLOBALS['DISABLE_GET_TABLES_INSTALLED_EVENTS_FOR_TEST'] = 1;
+
         LogDataPurger::$selectSegmentSize = 2;
         ReportsPurger::$selectSegmentSize = 2;
 
@@ -142,6 +144,8 @@ class DataPurgingTest extends IntegrationTestCase
 
     public function tearDown(): void
     {
+        $GLOBALS['DISABLE_GET_TABLES_INSTALLED_EVENTS_FOR_TEST'] = 0;
+
         parent::tearDown();
 
         $tempTableName = Common::prefixTable(RawLogDao::DELETE_UNUSED_ACTIONS_TEMP_TABLE_NAME);
diff --git a/plugins/TwoFactorAuth/tests/UI/TwoFactorAuth_spec.js b/plugins/TwoFactorAuth/tests/UI/TwoFactorAuth_spec.js
index 26e97fd4d1..60f57eb093 100644
--- a/plugins/TwoFactorAuth/tests/UI/TwoFactorAuth_spec.js
+++ b/plugins/TwoFactorAuth/tests/UI/TwoFactorAuth_spec.js
@@ -83,6 +83,7 @@ describe("TwoFactorAuth", function () {
     async function confirmPassword()
     {
         await page.waitFor('.confirmPasswordForm');
+        await page.waitFor(() => !!window.$);
         await page.evaluate(function(){
             $('.confirmPasswordForm #login_form_password').val('123abcDk3_l3');
             $('.confirmPasswordForm #login_form_submit').click();
@@ -93,18 +94,21 @@ describe("TwoFactorAuth", function () {
 
     it('a user with 2fa can open the widgetized view by token without needing to verify', async function () {
         await page.goto('?module=Widgetize&action=iframe&moduleToWidgetize=Actions&actionToWidgetize=getPageUrls&date=2018-03-04&token_auth=a4ca4238a0b923820dcc509a6f75849b&' + generalParams);
-        expect(await page.screenshotSelector('.widget')).to.matchImage('widgetized_no_verify');
+        const element = await page.$('.widget');
+        expect(await element.screenshot()).to.matchImage('widgetized_no_verify');
     });
 
     it('when logging in through logme and not providing auth code it should show auth code screen', async function () {
         await loginUser('with2FA', false);
-        expect(await page.screenshotSelector('.loginSection')).to.matchImage('logme_not_verified');
+        await page.waitFor(1000);
+        const section = await page.$('.loginSection');
+        expect(await section.screenshot()).to.matchImage('logme_not_verified');
     });
 
     it('when logging in and providing wrong code an error is shown', async function () {
         await page.type('.loginTwoFaForm #login_form_authcode', '555555');
         await page.evaluate(function(){
-            $('.loginTwoFaForm #login_form_submit').click();
+            document.querySelector('.loginTwoFaForm #login_form_submit').click();
         });
         await page.waitForNetworkIdle();
         const element = await page.$('.loginSection');
@@ -116,20 +120,23 @@ describe("TwoFactorAuth", function () {
         testEnvironment.save();
 
         await page.type('.loginTwoFaForm #login_form_authcode', '123456');
+        await page.waitFor(() => !!window.$);
         await page.evaluate(function(){
-            $('.loginTwoFaForm #login_form_submit').click();
+            document.querySelector('.loginTwoFaForm #login_form_submit').click();
         });
         await page.waitForNetworkIdle();
         await page.waitFor('.widget');
         await page.waitForNetworkIdle();
-        expect(await page.screenshotSelector('.pageWrap')).to.matchImage('logme_verified');
+
+        const element = await page.$('.pageWrap');
+        expect(await element.screenshot()).to.matchImage('logme_verified');
     });
 
     it('should show user settings when two-fa enabled', async function () {
         await loginUser('with2FA');
         await page.goto(userSettings);
-        await page.waitFor('.userSettings2FA', { visible: true });
-        await page.waitFor(500); // animation
+        await page.waitFor('.userSettings2FA', { visible: true, timeout: 0 });
+        await page.waitFor(750); // animation
         const elem = await page.$('.userSettings2FA');
         expect(await elem.screenshot()).to.matchImage('usersettings_twofa_enabled');
     });
@@ -137,19 +144,22 @@ describe("TwoFactorAuth", function () {
     it('should be possible to show recovery codes step1 authentication', async function () {
         await page.click('.showRecoveryCodesLink');
         await page.waitForNetworkIdle();
-        expect(await page.screenshotSelector('.loginSection')).to.matchImage('show_recovery_codes_step1');
+        const element = await page.$('.loginSection');
+        expect(await element.screenshot()).to.matchImage('show_recovery_codes_step1');
     });
 
     it('should be possible to show recovery codes step2 done', async function () {
         await confirmPassword();
         await page.waitForNetworkIdle();
-        expect(await page.screenshotSelector('#content')).to.matchImage('show_recovery_codes_step2');
+        const element = await page.$('#content');
+        expect(await element.screenshot()).to.matchImage('show_recovery_codes_step2');
     });
 
     it('should show user settings when two-fa enabled', async function () {
         requireTwoFa();
         await page.goto(userSettings);
-        expect(await page.screenshotSelector('.userSettings2FA')).to.matchImage('usersettings_twofa_enabled_required');
+        const element = await page.$('.userSettings2FA');
+        expect(await element.screenshot()).to.matchImage('usersettings_twofa_enabled_required');
     });
 
     it('should be possible to disable two factor', async function () {
@@ -165,7 +175,9 @@ describe("TwoFactorAuth", function () {
     it('should be possible to disable two factor step 2 confirmed', async function () {
         await selectModalButton('Yes');
         await page.waitFor(150);
-        expect(await page.screenshotSelector('.loginSection')).to.matchImage('usersettings_twofa_disable_step2');
+
+        const element = await page.$('.loginSection');
+        expect(await element.screenshot()).to.matchImage('usersettings_twofa_disable_step2');
     });
 
     it('should be possible to disable two factor step 3 verified', async function () {
@@ -180,7 +192,9 @@ describe("TwoFactorAuth", function () {
         await page.goto(userSettings);
         await page.click('.enable2FaLink');
         await confirmPassword();
-        expect(await page.screenshotSelector('#content')).to.matchImage('twofa_setup_step1');
+        await page.waitFor(1000);
+        const element = await page.$('#content');
+        expect(await element.screenshot()).to.matchImage('twofa_setup_step1');
     });
 
     it('should move to second step in setup - step 2', async function () {
@@ -193,13 +207,17 @@ describe("TwoFactorAuth", function () {
         await page.evaluate(function () {
             $('#qrcode').hide();
         });
-        expect(await page.screenshotSelector('#content')).to.matchImage('twofa_setup_step2');
+        const element = await page.$('#content');
+        expect(await element.screenshot()).to.matchImage('twofa_setup_step2');
     });
 
     it('should move to third step in setup - step 3', async function () {
         await page.click('.setupTwoFactorAuthentication .goToStep3');
         await page.waitForNetworkIdle();
-        expect(await page.screenshotSelector('#content')).to.matchImage('twofa_setup_step3');
+        await page.waitFor(1000);
+
+        const element = await page.$('#content');
+        expect(await element.screenshot()).to.matchImage('twofa_setup_step3');
     });
 
     it('should move to third step in setup - step 4 confirm', async function () {
@@ -214,7 +232,8 @@ describe("TwoFactorAuth", function () {
         await page.waitForNetworkIdle();
         await page.waitFor('#content', { visible: true });
         await page.waitForNetworkIdle();
-        expect(await page.screenshotSelector('#content')).to.matchImage('twofa_setup_step4');
+        const element = await page.$('#content');
+        expect(await element.screenshot()).to.matchImage('twofa_setup_step4');
     });
 
     it('should force user to setup 2fa when not set up yet but enforced', async function () {
diff --git a/plugins/TwoFactorAuth/tests/UI/expected-screenshots/TwoFactorAuth_logme_verified.png b/plugins/TwoFactorAuth/tests/UI/expected-screenshots/TwoFactorAuth_logme_verified.png
index 2bf171ea1c..e4f5bea535 100644
--- a/plugins/TwoFactorAuth/tests/UI/expected-screenshots/TwoFactorAuth_logme_verified.png
+++ b/plugins/TwoFactorAuth/tests/UI/expected-screenshots/TwoFactorAuth_logme_verified.png
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:750cea1f0946ac6372a1b5339c12f88aa36f6a58d0e56d3cd750fe0648d4d461
-size 181605
+oid sha256:3c5b3611278ddefdf25b172f6f908a7d1a3ee2b21e7c8e64f00515f41eb0148d
+size 181837
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);