From b14da91a98f4cfef5fe3041584c8e935a7e8a93a Mon Sep 17 00:00:00 2001 From: Franz Liedke Date: Tue, 19 May 2020 23:12:08 +0200 Subject: [PATCH 01/13] Run integration tests in a transaction --- framework/core/tests/integration/TestCase.php | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/framework/core/tests/integration/TestCase.php b/framework/core/tests/integration/TestCase.php index 16a65f07e..916b1c06f 100644 --- a/framework/core/tests/integration/TestCase.php +++ b/framework/core/tests/integration/TestCase.php @@ -23,6 +23,20 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase { use BuildsHttpRequests; + protected function setUp(): void + { + parent::setUp(); + + $this->database()->beginTransaction(); + } + + protected function tearDown(): void + { + parent::tearDown(); + + $this->database()->rollBack(); + } + /** * @var \Flarum\Foundation\InstalledApp */ @@ -94,13 +108,6 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase // We temporarily disable foreign key checks to simplify this process. $this->database()->getSchemaBuilder()->disableForeignKeyConstraints(); - // First, truncate all referenced tables so that they are empty. - foreach (array_keys($tableData) as $table) { - if ($table !== 'settings') { - $this->database()->table($table)->truncate(); - } - } - // Then, insert all rows required for this test case. foreach ($tableData as $table => $rows) { foreach ($rows as $row) { From a42ce7045d36327a360be2502a7903af5dc09ca6 Mon Sep 17 00:00:00 2001 From: Franz Liedke Date: Tue, 19 May 2020 23:37:41 +0200 Subject: [PATCH 02/13] Tests: DB tables no longer need to be truncated --- .../api/authentication/WithApiKeyTest.php | 4 ---- .../integration/api/discussions/CreateTest.php | 2 -- .../core/tests/integration/api/posts/CreateTest.php | 1 - .../core/tests/integration/api/users/ListTest.php | 1 - .../core/tests/integration/extenders/CsrfTest.php | 13 ------------- .../core/tests/integration/extenders/ModelTest.php | 1 - 6 files changed, 22 deletions(-) diff --git a/framework/core/tests/integration/api/authentication/WithApiKeyTest.php b/framework/core/tests/integration/api/authentication/WithApiKeyTest.php index 37e9bf3ca..2b5f82ba9 100644 --- a/framework/core/tests/integration/api/authentication/WithApiKeyTest.php +++ b/framework/core/tests/integration/api/authentication/WithApiKeyTest.php @@ -28,10 +28,6 @@ class WithApiKeyTest extends TestCase $this->adminUser(), $this->normalUser(), ], - 'group_permission' => [ - ['permission' => 'viewUserList', 'group_id' => 3] - ], - 'api_keys' => [], ]); } diff --git a/framework/core/tests/integration/api/discussions/CreateTest.php b/framework/core/tests/integration/api/discussions/CreateTest.php index 471bf1a2c..816b1590f 100644 --- a/framework/core/tests/integration/api/discussions/CreateTest.php +++ b/framework/core/tests/integration/api/discussions/CreateTest.php @@ -23,8 +23,6 @@ class CreateTest extends TestCase parent::setUp(); $this->prepareDatabase([ - 'discussions' => [], - 'posts' => [], 'users' => [ $this->adminUser(), $this->normalUser(), diff --git a/framework/core/tests/integration/api/posts/CreateTest.php b/framework/core/tests/integration/api/posts/CreateTest.php index 613810078..f9efe2d32 100644 --- a/framework/core/tests/integration/api/posts/CreateTest.php +++ b/framework/core/tests/integration/api/posts/CreateTest.php @@ -25,7 +25,6 @@ class CreateTest extends TestCase 'discussions' => [ ['id' => 1, 'title' => __CLASS__, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2], ], - 'posts' => [], 'users' => [ $this->normalUser(), ], diff --git a/framework/core/tests/integration/api/users/ListTest.php b/framework/core/tests/integration/api/users/ListTest.php index 3e33786b4..152ea18f6 100644 --- a/framework/core/tests/integration/api/users/ListTest.php +++ b/framework/core/tests/integration/api/users/ListTest.php @@ -29,7 +29,6 @@ class ListTest extends TestCase $this->adminGroup(), $this->guestGroup(), ], - 'group_permission' => [], 'group_user' => [ ['user_id' => 1, 'group_id' => 1], ], diff --git a/framework/core/tests/integration/extenders/CsrfTest.php b/framework/core/tests/integration/extenders/CsrfTest.php index 3219ab48e..db841e0a0 100644 --- a/framework/core/tests/integration/extenders/CsrfTest.php +++ b/framework/core/tests/integration/extenders/CsrfTest.php @@ -21,20 +21,11 @@ class CsrfTest extends TestCase 'email' => 'test@machine.local', ]; - protected function prepDb() - { - $this->prepareDatabase([ - 'users' => [], - ]); - } - /** * @test */ public function create_user_post_needs_csrf_token_by_default() { - $this->prepDb(); - $response = $this->send( $this->request('POST', '/api/users', [ 'json' => [ @@ -59,8 +50,6 @@ class CsrfTest extends TestCase ->exemptPath('/api/users') ); - $this->prepDb(); - $response = $this->send( $this->request('POST', '/api/users', [ 'json' => [ @@ -122,8 +111,6 @@ class CsrfTest extends TestCase ->exemptPath('/api/fake/*/up') ); - $this->prepDb(); - $response = $this->send( $this->request('POST', '/api/fake/route/i/made/up') ); diff --git a/framework/core/tests/integration/extenders/ModelTest.php b/framework/core/tests/integration/extenders/ModelTest.php index 6f7959e13..5034bf9e1 100644 --- a/framework/core/tests/integration/extenders/ModelTest.php +++ b/framework/core/tests/integration/extenders/ModelTest.php @@ -32,7 +32,6 @@ class ModelTest extends TestCase $this->adminUser(), $this->normalUser(), ], - 'discussions' => [] ]); } From 1b98526e89910c7c8799f81ec5c5814f6386c0b7 Mon Sep 17 00:00:00 2001 From: Franz Liedke Date: Wed, 20 May 2020 00:18:00 +0200 Subject: [PATCH 03/13] Tests: Rely on admin user, groups, permissions from test setup script --- .../integration/RetrievesAuthorizedUsers.php | 44 ------------------- .../api/authentication/WithApiKeyTest.php | 1 - .../csrf_protection/RequireCsrfTokenTest.php | 12 ----- .../api/discussions/CreateTest.php | 24 ---------- .../api/discussions/DeletionTest.php | 7 --- .../integration/api/discussions/ListTest.php | 7 --- .../integration/api/discussions/ShowTest.php | 11 ----- .../tests/integration/api/forum/ShowTest.php | 11 +---- .../integration/api/groups/CreateTest.php | 7 --- .../tests/integration/api/groups/ListTest.php | 16 +++---- .../integration/api/posts/CreateTest.php | 9 ---- .../integration/api/users/CreateTest.php | 10 ----- .../tests/integration/api/users/ListTest.php | 19 -------- .../tests/integration/api/users/ShowTest.php | 10 ----- .../integration/api/users/UpdateTest.php | 13 ------ .../extenders/ApiControllerTest.php | 5 --- .../extenders/ApiSerializerTest.php | 1 - .../tests/integration/extenders/EventTest.php | 9 ---- .../tests/integration/extenders/MailTest.php | 21 --------- .../tests/integration/extenders/ModelTest.php | 6 --- .../integration/extenders/ModelUrlTest.php | 1 - .../extenders/ModelVisibilityTest.php | 11 ----- .../integration/extenders/PolicyTest.php | 1 - .../integration/extenders/SettingsTest.php | 1 - .../integration/extenders/ThrottleApiTest.php | 9 ---- .../tests/integration/extenders/UserTest.php | 7 --- framework/core/tests/integration/setup.php | 4 +- 27 files changed, 8 insertions(+), 269 deletions(-) diff --git a/framework/core/tests/integration/RetrievesAuthorizedUsers.php b/framework/core/tests/integration/RetrievesAuthorizedUsers.php index df2f80e87..a4bd58a98 100644 --- a/framework/core/tests/integration/RetrievesAuthorizedUsers.php +++ b/framework/core/tests/integration/RetrievesAuthorizedUsers.php @@ -11,50 +11,6 @@ namespace Flarum\Tests\integration; trait RetrievesAuthorizedUsers { - protected function adminGroup(): array - { - return [ - 'id' => 1, - 'name_singular' => 'Admin', - 'name_plural' => 'Admins', - 'color' => '#B72A2A', - 'icon' => 'fas fa-wrench', - ]; - } - - protected function guestGroup(): array - { - return [ - 'id' => 2, - 'name_singular' => 'Guest', - 'name_plural' => 'Guests', - 'color' => null, - 'icon' => null, - ]; - } - - protected function memberGroup(): array - { - return [ - 'id' => 3, - 'name_singular' => 'Member', - 'name_plural' => 'Members', - 'color' => null, - 'icon' => null, - ]; - } - - protected function adminUser(): array - { - return [ - 'id' => 1, - 'username' => 'admin', - 'password' => '$2y$10$HMOAe.XaQjOimA778VmFue1OCt7tj5j0wk5vfoL/CMSJq2BQlfBV2', // BCrypt hash for "password" - 'email' => 'admin@machine.local', - 'is_email_confirmed' => 1, - ]; - } - protected function normalUser(): array { return [ diff --git a/framework/core/tests/integration/api/authentication/WithApiKeyTest.php b/framework/core/tests/integration/api/authentication/WithApiKeyTest.php index 2b5f82ba9..2518d4238 100644 --- a/framework/core/tests/integration/api/authentication/WithApiKeyTest.php +++ b/framework/core/tests/integration/api/authentication/WithApiKeyTest.php @@ -25,7 +25,6 @@ class WithApiKeyTest extends TestCase $this->prepareDatabase([ 'users' => [ - $this->adminUser(), $this->normalUser(), ], ]); diff --git a/framework/core/tests/integration/api/csrf_protection/RequireCsrfTokenTest.php b/framework/core/tests/integration/api/csrf_protection/RequireCsrfTokenTest.php index b6540eb66..965aeeef9 100644 --- a/framework/core/tests/integration/api/csrf_protection/RequireCsrfTokenTest.php +++ b/framework/core/tests/integration/api/csrf_protection/RequireCsrfTokenTest.php @@ -21,18 +21,6 @@ class RequireCsrfTokenTest extends TestCase parent::setUp(); $this->prepareDatabase([ - 'users' => [ - $this->adminUser(), - ], - 'groups' => [ - $this->adminGroup(), - ], - 'group_user' => [ - ['user_id' => 1, 'group_id' => 1], - ], - 'group_permission' => [ - ['permission' => 'viewUserList', 'group_id' => 3], - ], 'api_keys' => [ ['user_id' => 1, 'key' => 'superadmin'], ], diff --git a/framework/core/tests/integration/api/discussions/CreateTest.php b/framework/core/tests/integration/api/discussions/CreateTest.php index 816b1590f..e24969937 100644 --- a/framework/core/tests/integration/api/discussions/CreateTest.php +++ b/framework/core/tests/integration/api/discussions/CreateTest.php @@ -18,30 +18,6 @@ class CreateTest extends TestCase { use RetrievesAuthorizedUsers; - protected function setUp(): void - { - parent::setUp(); - - $this->prepareDatabase([ - 'users' => [ - $this->adminUser(), - $this->normalUser(), - ], - 'groups' => [ - $this->adminGroup(), - $this->memberGroup(), - ], - 'group_user' => [ - ['user_id' => 1, 'group_id' => 1], - ['user_id' => 2, 'group_id' => 3], - ], - 'group_permission' => [ - ['permission' => 'viewDiscussions', 'group_id' => 3], - ['permission' => 'startDiscussion', 'group_id' => 3], - ] - ]); - } - /** * @test */ diff --git a/framework/core/tests/integration/api/discussions/DeletionTest.php b/framework/core/tests/integration/api/discussions/DeletionTest.php index aa948d046..ea825fd9b 100644 --- a/framework/core/tests/integration/api/discussions/DeletionTest.php +++ b/framework/core/tests/integration/api/discussions/DeletionTest.php @@ -29,15 +29,8 @@ class DeletionTest extends TestCase ['id' => 1, 'discussion_id' => 1, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'type' => 'comment', 'content' => '

foo bar

'], ], 'users' => [ - $this->adminUser(), $this->normalUser(), ], - 'groups' => [ - $this->adminGroup(), - ], - 'group_user' => [ - ['user_id' => 1, 'group_id' => 1], - ], ]); } diff --git a/framework/core/tests/integration/api/discussions/ListTest.php b/framework/core/tests/integration/api/discussions/ListTest.php index 3e56c4150..b47bbe127 100644 --- a/framework/core/tests/integration/api/discussions/ListTest.php +++ b/framework/core/tests/integration/api/discussions/ListTest.php @@ -30,13 +30,6 @@ class ListTest extends TestCase ], 'users' => [ $this->normalUser(), - ], - 'groups' => [ - $this->memberGroup(), - $this->guestGroup(), - ], - 'group_permission' => [ - ['permission' => 'viewDiscussions', 'group_id' => 2], ] ]); } diff --git a/framework/core/tests/integration/api/discussions/ShowTest.php b/framework/core/tests/integration/api/discussions/ShowTest.php index b4300a34a..79d036c16 100644 --- a/framework/core/tests/integration/api/discussions/ShowTest.php +++ b/framework/core/tests/integration/api/discussions/ShowTest.php @@ -37,17 +37,6 @@ class ShowTest extends TestCase ], 'users' => [ $this->normalUser(), - ], - 'groups' => [ - $this->guestGroup(), - $this->memberGroup(), - ], - 'group_user' => [ - ['user_id' => 2, 'group_id' => 3], - ], - 'group_permission' => [ - ['permission' => 'viewDiscussions', 'group_id' => 2], - ['permission' => 'viewDiscussions', 'group_id' => 3], ] ]); } diff --git a/framework/core/tests/integration/api/forum/ShowTest.php b/framework/core/tests/integration/api/forum/ShowTest.php index 965473678..7ff520712 100644 --- a/framework/core/tests/integration/api/forum/ShowTest.php +++ b/framework/core/tests/integration/api/forum/ShowTest.php @@ -23,17 +23,8 @@ class ShowTest extends TestCase $this->prepareDatabase([ 'users' => [ - $this->adminUser(), $this->normalUser(), - ], - 'groups' => [ - $this->adminGroup(), - $this->memberGroup(), - ], - 'group_user' => [ - ['user_id' => 1, 'group_id' => 1], - ['user_id' => 2, 'group_id' => 3], - ], + ] ]); } diff --git a/framework/core/tests/integration/api/groups/CreateTest.php b/framework/core/tests/integration/api/groups/CreateTest.php index 360e7f3b3..ba4b43303 100644 --- a/framework/core/tests/integration/api/groups/CreateTest.php +++ b/framework/core/tests/integration/api/groups/CreateTest.php @@ -24,15 +24,8 @@ class CreateTest extends TestCase $this->prepareDatabase([ 'users' => [ - $this->adminUser(), $this->normalUser(), ], - 'groups' => [ - $this->adminGroup(), - ], - 'group_user' => [ - ['user_id' => 1, 'group_id' => 1], - ], ]); } diff --git a/framework/core/tests/integration/api/groups/ListTest.php b/framework/core/tests/integration/api/groups/ListTest.php index 13f476b1f..9cef5476f 100644 --- a/framework/core/tests/integration/api/groups/ListTest.php +++ b/framework/core/tests/integration/api/groups/ListTest.php @@ -22,16 +22,8 @@ class ListTest extends TestCase parent::setUp(); $this->prepareDatabase([ - 'users' => [ - $this->adminUser(), - $this->normalUser(), - ], 'groups' => [ - $this->adminGroup(), - $this->hiddenGroup() - ], - 'group_user' => [ - ['user_id' => 1, 'group_id' => 1], + $this->hiddenGroup(), ], ]); } @@ -48,7 +40,8 @@ class ListTest extends TestCase $this->assertEquals(200, $response->getStatusCode()); $data = json_decode($response->getBody()->getContents(), true); - $this->assertEquals(['1'], Arr::pluck($data['data'], 'id')); + // The four default groups created by the installer + $this->assertEquals(['1', '2', '3', '4'], Arr::pluck($data['data'], 'id')); } /** @@ -65,7 +58,8 @@ class ListTest extends TestCase $this->assertEquals(200, $response->getStatusCode()); $data = json_decode($response->getBody()->getContents(), true); - $this->assertEquals(['1', '10'], Arr::pluck($data['data'], 'id')); + // The four default groups created by the installer and our hidden group + $this->assertEquals(['1', '2', '3', '4', '10'], Arr::pluck($data['data'], 'id')); } protected function hiddenGroup(): array diff --git a/framework/core/tests/integration/api/posts/CreateTest.php b/framework/core/tests/integration/api/posts/CreateTest.php index f9efe2d32..cc023677c 100644 --- a/framework/core/tests/integration/api/posts/CreateTest.php +++ b/framework/core/tests/integration/api/posts/CreateTest.php @@ -27,15 +27,6 @@ class CreateTest extends TestCase ], 'users' => [ $this->normalUser(), - ], - 'groups' => [ - $this->memberGroup(), - ], - 'group_user' => [ - ['user_id' => 2, 'group_id' => 3], - ], - 'group_permission' => [ - ['permission' => 'viewDiscussions', 'group_id' => 3], ] ]); } diff --git a/framework/core/tests/integration/api/users/CreateTest.php b/framework/core/tests/integration/api/users/CreateTest.php index 65e821ab7..6e1d6d023 100644 --- a/framework/core/tests/integration/api/users/CreateTest.php +++ b/framework/core/tests/integration/api/users/CreateTest.php @@ -23,16 +23,6 @@ class CreateTest extends TestCase parent::setUp(); $this->prepareDatabase([ - 'users' => [ - $this->adminUser(), - $this->normalUser(), - ], - 'groups' => [ - $this->adminGroup() - ], - 'group_user' => [ - ['user_id' => 1, 'group_id' => 1], - ], 'settings' => [ ['key' => 'mail_driver', 'value' => 'log'], ], diff --git a/framework/core/tests/integration/api/users/ListTest.php b/framework/core/tests/integration/api/users/ListTest.php index 152ea18f6..ab9dac7a5 100644 --- a/framework/core/tests/integration/api/users/ListTest.php +++ b/framework/core/tests/integration/api/users/ListTest.php @@ -9,7 +9,6 @@ namespace Flarum\Tests\integration\api\users; -use Flarum\Group\Permission; use Flarum\Tests\integration\RetrievesAuthorizedUsers; use Flarum\Tests\integration\TestCase; @@ -17,24 +16,6 @@ class ListTest extends TestCase { use RetrievesAuthorizedUsers; - protected function setUp(): void - { - parent::setUp(); - - $this->prepareDatabase([ - 'users' => [ - $this->adminUser(), - ], - 'groups' => [ - $this->adminGroup(), - $this->guestGroup(), - ], - 'group_user' => [ - ['user_id' => 1, 'group_id' => 1], - ], - ]); - } - /** * @test */ diff --git a/framework/core/tests/integration/api/users/ShowTest.php b/framework/core/tests/integration/api/users/ShowTest.php index 3ee210203..3fb0e7bd2 100644 --- a/framework/core/tests/integration/api/users/ShowTest.php +++ b/framework/core/tests/integration/api/users/ShowTest.php @@ -22,18 +22,8 @@ class ShowTest extends TestCase $this->prepareDatabase([ 'users' => [ - $this->adminUser(), $this->normalUser(), ], - 'groups' => [ - $this->adminGroup() - ], - 'group_user' => [ - ['user_id' => 1, 'group_id' => 1], - ], - 'settings' => [ - ['key' => 'mail_driver', 'value' => 'log'], - ], ]); } diff --git a/framework/core/tests/integration/api/users/UpdateTest.php b/framework/core/tests/integration/api/users/UpdateTest.php index e143e6565..be6b397dd 100644 --- a/framework/core/tests/integration/api/users/UpdateTest.php +++ b/framework/core/tests/integration/api/users/UpdateTest.php @@ -22,20 +22,7 @@ class UpdateTest extends TestCase $this->prepareDatabase([ 'users' => [ - $this->adminUser(), $this->normalUser(), - ], - 'groups' => [ - $this->adminGroup(), - $this->memberGroup(), - ], - 'group_user' => [ - ['user_id' => 1, 'group_id' => 1], - ['user_id' => 2, 'group_id' => 3], - ], - 'group_permission' => [ - ['permission' => 'viewUserList', 'group_id' => 3], - ['permission' => 'viewDiscussions', 'group_id' => 3] ] ]); } diff --git a/framework/core/tests/integration/extenders/ApiControllerTest.php b/framework/core/tests/integration/extenders/ApiControllerTest.php index 5d8784cd7..7b3e21d8f 100644 --- a/framework/core/tests/integration/extenders/ApiControllerTest.php +++ b/framework/core/tests/integration/extenders/ApiControllerTest.php @@ -35,13 +35,8 @@ class ApiControllerTest extends TestCase { $this->prepareDatabase([ 'users' => [ - $this->adminUser(), $this->normalUser() ], - 'groups' => [ - $this->adminGroup(), - $this->memberGroup() - ], 'discussions' => [ ['id' => 1, 'title' => 'Custom Discussion Title', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'first_post_id' => 0, 'comment_count' => 1, 'is_private' => 0], ['id' => 2, 'title' => 'Custom Discussion Title', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 3, 'first_post_id' => 0, 'comment_count' => 1, 'is_private' => 0], diff --git a/framework/core/tests/integration/extenders/ApiSerializerTest.php b/framework/core/tests/integration/extenders/ApiSerializerTest.php index 2bee2bafd..8e3e32236 100644 --- a/framework/core/tests/integration/extenders/ApiSerializerTest.php +++ b/framework/core/tests/integration/extenders/ApiSerializerTest.php @@ -31,7 +31,6 @@ class ApiSerializerTest extends TestCase { $this->prepareDatabase([ 'users' => [ - $this->adminUser(), $this->normalUser() ], 'discussions' => [ diff --git a/framework/core/tests/integration/extenders/EventTest.php b/framework/core/tests/integration/extenders/EventTest.php index de32cd1ee..0aab2ce06 100644 --- a/framework/core/tests/integration/extenders/EventTest.php +++ b/framework/core/tests/integration/extenders/EventTest.php @@ -24,15 +24,6 @@ class EventTest extends TestCase protected function buildGroup() { - $this->prepareDatabase([ - 'groups' => [ - $this->adminGroup(), - ], - 'users' => [ - $this->adminUser(), - ], - ]); - $bus = $this->app()->getContainer()->make(Dispatcher::class); return $bus->dispatch( diff --git a/framework/core/tests/integration/extenders/MailTest.php b/framework/core/tests/integration/extenders/MailTest.php index 7a8a1dc52..a62773b3f 100644 --- a/framework/core/tests/integration/extenders/MailTest.php +++ b/framework/core/tests/integration/extenders/MailTest.php @@ -23,28 +23,11 @@ class MailTest extends TestCase { use RetrievesAuthorizedUsers; - protected function prepDb() - { - $this->prepareDatabase([ - 'users' => [ - $this->adminUser(), - ], - 'groups' => [ - $this->adminGroup(), - ], - 'group_user' => [ - ['user_id' => 1, 'group_id' => 1], - ], - ]); - } - /** * @test */ public function drivers_are_unchanged_by_default() { - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api/mail/settings', [ 'authenticatedAs' => 1, @@ -76,8 +59,6 @@ class MailTest extends TestCase ->driver('custom', CustomDriver::class) ); - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api/mail/settings', [ 'authenticatedAs' => 1, @@ -100,8 +81,6 @@ class MailTest extends TestCase ->driver('smtp', CustomDriver::class) ); - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api/mail/settings', [ 'authenticatedAs' => 1, diff --git a/framework/core/tests/integration/extenders/ModelTest.php b/framework/core/tests/integration/extenders/ModelTest.php index 5034bf9e1..9d01dc1fb 100644 --- a/framework/core/tests/integration/extenders/ModelTest.php +++ b/framework/core/tests/integration/extenders/ModelTest.php @@ -29,7 +29,6 @@ class ModelTest extends TestCase { $this->prepareDatabase([ 'users' => [ - $this->adminUser(), $this->normalUser(), ], ]); @@ -245,11 +244,6 @@ class ModelTest extends TestCase ); $this->prepDB(); - $this->prepareDatabase([ - 'groups' => [ - $this->adminGroup() - ] - ]); $group = Group::find(1); diff --git a/framework/core/tests/integration/extenders/ModelUrlTest.php b/framework/core/tests/integration/extenders/ModelUrlTest.php index e5eea08ad..44abd1ef6 100644 --- a/framework/core/tests/integration/extenders/ModelUrlTest.php +++ b/framework/core/tests/integration/extenders/ModelUrlTest.php @@ -26,7 +26,6 @@ class ModelUrlTest extends TestCase $userClass = User::class; $this->prepareDatabase([ 'users' => [ - $this->adminUser(), $this->normalUser(), ], 'settings' => [ diff --git a/framework/core/tests/integration/extenders/ModelVisibilityTest.php b/framework/core/tests/integration/extenders/ModelVisibilityTest.php index 0121e6e8a..69ff0820b 100644 --- a/framework/core/tests/integration/extenders/ModelVisibilityTest.php +++ b/framework/core/tests/integration/extenders/ModelVisibilityTest.php @@ -37,17 +37,6 @@ class ModelVisibilityTest extends TestCase ], 'users' => [ $this->normalUser(), - ], - 'groups' => [ - $this->guestGroup(), - $this->memberGroup(), - ], - 'group_user' => [ - ['user_id' => 2, 'group_id' => 3], - ], - 'group_permission' => [ - ['permission' => 'viewDiscussions', 'group_id' => 2], - ['permission' => 'viewDiscussions', 'group_id' => 3], ] ]); } diff --git a/framework/core/tests/integration/extenders/PolicyTest.php b/framework/core/tests/integration/extenders/PolicyTest.php index d45c2cdc5..b2f0420b8 100644 --- a/framework/core/tests/integration/extenders/PolicyTest.php +++ b/framework/core/tests/integration/extenders/PolicyTest.php @@ -32,7 +32,6 @@ class PolicyTest extends TestCase { $this->prepareDatabase([ 'users' => [ - $this->adminUser(), $this->normalUser(), ], 'discussions' => [ diff --git a/framework/core/tests/integration/extenders/SettingsTest.php b/framework/core/tests/integration/extenders/SettingsTest.php index 6a8f0b142..77c41d70e 100644 --- a/framework/core/tests/integration/extenders/SettingsTest.php +++ b/framework/core/tests/integration/extenders/SettingsTest.php @@ -21,7 +21,6 @@ class SettingsTest extends TestCase { $this->prepareDatabase([ 'users' => [ - $this->adminUser(), $this->normalUser() ], 'settings' => [ diff --git a/framework/core/tests/integration/extenders/ThrottleApiTest.php b/framework/core/tests/integration/extenders/ThrottleApiTest.php index 79400cf40..33a4853e7 100644 --- a/framework/core/tests/integration/extenders/ThrottleApiTest.php +++ b/framework/core/tests/integration/extenders/ThrottleApiTest.php @@ -22,15 +22,6 @@ class ThrottleApiTest extends TestCase $this->prepareDatabase([ 'users' => [ $this->normalUser(), - ], - 'groups' => [ - $this->memberGroup(), - ], - 'group_user' => [ - ['user_id' => 2, 'group_id' => 3], - ], - 'group_permission' => [ - ['permission' => 'viewDiscussions', 'group_id' => 3], ] ]); } diff --git a/framework/core/tests/integration/extenders/UserTest.php b/framework/core/tests/integration/extenders/UserTest.php index a4108509a..473805185 100644 --- a/framework/core/tests/integration/extenders/UserTest.php +++ b/framework/core/tests/integration/extenders/UserTest.php @@ -24,17 +24,10 @@ class UserTest extends TestCase { $this->prepareDatabase([ 'users' => [ - $this->adminUser(), $this->normalUser(), ], - 'group_permission' => [ - ['permission' => 'viewUserList', 'group_id' => 3] - ], 'settings' => [ ['key' => 'display_name_driver', 'value' => 'custom'], - ], - 'group_permission' => [ - ['permission' => 'viewUserList', 'group_id' => 3], ] ]); } diff --git a/framework/core/tests/integration/setup.php b/framework/core/tests/integration/setup.php index 2a18ca4a6..95739eaac 100644 --- a/framework/core/tests/integration/setup.php +++ b/framework/core/tests/integration/setup.php @@ -56,8 +56,8 @@ $pipeline = $installation ) ->adminUser(new AdminUser( 'admin', - 'secret', - 'admin@flarum.email' + 'password', + 'admin@machine.local' )) ->settings(['mail_driver' => 'log']) ->build(); From 9b2383fe20991a062fdbf355a2367dbed6db4e18 Mon Sep 17 00:00:00 2001 From: Franz Liedke Date: Thu, 21 May 2020 22:34:01 +0200 Subject: [PATCH 04/13] Tests: Stop using Eloquent models for seeding data --- .../tests/integration/BuildsHttpRequests.php | 20 ++++++++--- .../api/authentication/WithApiKeyTest.php | 33 ++++++------------- .../tests/integration/api/users/ListTest.php | 11 +++---- 3 files changed, 31 insertions(+), 33 deletions(-) diff --git a/framework/core/tests/integration/BuildsHttpRequests.php b/framework/core/tests/integration/BuildsHttpRequests.php index 09704de8b..410d67b56 100644 --- a/framework/core/tests/integration/BuildsHttpRequests.php +++ b/framework/core/tests/integration/BuildsHttpRequests.php @@ -9,8 +9,9 @@ namespace Flarum\Tests\integration; +use Carbon\Carbon; use Dflydev\FigCookies\SetCookie; -use Flarum\Http\AccessToken; +use Illuminate\Support\Str; use Laminas\Diactoros\CallbackStream; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; @@ -33,10 +34,21 @@ trait BuildsHttpRequests protected function requestAsUser(Request $req, int $userId): Request { - $token = AccessToken::generate($userId); - $token->save(); + $token = Str::random(40); - return $req->withAddedHeader('Authorization', "Token {$token->token}"); + /** + * We insert this directly instead of via `prepareDatabase` + * so that requests can be created/sent after the app is booted. + */ + $this->database()->table('access_tokens')->insert([ + 'token' => $token, + 'user_id' => $userId, + 'created_at' => Carbon::now()->toDateTimeString(), + 'last_activity_at' => Carbon::now()->toDateTimeString(), + 'lifetime_seconds' => 3600 + ]); + + return $req->withAddedHeader('Authorization', "Token {$token}"); } protected function requestWithCookiesFrom(Request $req, Response $previous): Request diff --git a/framework/core/tests/integration/api/authentication/WithApiKeyTest.php b/framework/core/tests/integration/api/authentication/WithApiKeyTest.php index 2518d4238..180a4d630 100644 --- a/framework/core/tests/integration/api/authentication/WithApiKeyTest.php +++ b/framework/core/tests/integration/api/authentication/WithApiKeyTest.php @@ -27,20 +27,13 @@ class WithApiKeyTest extends TestCase 'users' => [ $this->normalUser(), ], + 'api_keys' => [ + ['key' => 'mastertoken', 'user_id' => null, 'created_at' => Carbon::now()->toDateTimeString()], + ['key' => 'personaltoken', 'user_id' => 2, 'created_at' => Carbon::now()->toDateTimeString()], + ] ]); } - protected function key(int $user_id = null): ApiKey - { - return ApiKey::unguarded(function () use ($user_id) { - return ApiKey::query()->firstOrCreate([ - 'key' => Str::random(), - 'user_id' => $user_id, - 'created_at' => Carbon::now() - ]); - }); - } - /** * @test */ @@ -59,18 +52,16 @@ class WithApiKeyTest extends TestCase */ public function master_token_can_authenticate_as_anyone() { - $key = $this->key(); - $response = $this->send( $this->request('GET', '/api') - ->withAddedHeader('Authorization', "Token {$key->key}; userId=1") + ->withAddedHeader('Authorization', 'Token mastertoken; userId=1') ); $data = json_decode($response->getBody(), true); $this->assertTrue($data['data']['attributes']['canViewUserList']); $this->assertArrayHasKey('adminUrl', $data['data']['attributes']); - $key->refresh(); + $key = ApiKey::where('key', 'mastertoken')->first(); $this->assertNotNull($key->last_activity_at); } @@ -80,18 +71,16 @@ class WithApiKeyTest extends TestCase */ public function personal_api_token_cannot_authenticate_as_anyone() { - $key = $this->key(2); - $response = $this->send( $this->request('GET', '/api') - ->withAddedHeader('Authorization', "Token {$key->key}; userId=1") + ->withAddedHeader('Authorization', 'Token personaltoken; userId=1') ); $data = json_decode($response->getBody(), true); $this->assertTrue($data['data']['attributes']['canViewUserList']); $this->assertArrayNotHasKey('adminUrl', $data['data']['attributes']); - $key->refresh(); + $key = ApiKey::where('key', 'personaltoken')->first(); $this->assertNotNull($key->last_activity_at); } @@ -101,18 +90,16 @@ class WithApiKeyTest extends TestCase */ public function personal_api_token_authenticates_user() { - $key = $this->key(2); - $response = $this->send( $this->request('GET', '/api') - ->withAddedHeader('Authorization', "Token {$key->key}") + ->withAddedHeader('Authorization', 'Token personaltoken') ); $data = json_decode($response->getBody(), true); $this->assertTrue($data['data']['attributes']['canViewUserList']); $this->assertArrayNotHasKey('adminUrl', $data['data']['attributes']); - $key->refresh(); + $key = ApiKey::where('key', 'personaltoken')->first(); $this->assertNotNull($key->last_activity_at); } diff --git a/framework/core/tests/integration/api/users/ListTest.php b/framework/core/tests/integration/api/users/ListTest.php index ab9dac7a5..c2b05a941 100644 --- a/framework/core/tests/integration/api/users/ListTest.php +++ b/framework/core/tests/integration/api/users/ListTest.php @@ -33,12 +33,11 @@ class ListTest extends TestCase */ public function shows_index_for_guest_when_they_have_permission() { - Permission::unguarded(function () { - Permission::create([ - 'permission' => 'viewUserList', - 'group_id' => 2, - ]); - }); + $this->prepareDatabase([ + 'group_permission' => [ + ['permission' => 'viewUserList', 'group_id' => 2], + ], + ]); $response = $this->send( $this->request('GET', '/api/users') From 956050c6435f5ce1787a13c2d8d46662b71f1118 Mon Sep 17 00:00:00 2001 From: Franz Liedke Date: Thu, 21 May 2020 22:35:06 +0200 Subject: [PATCH 05/13] Tests: Always start transaction before seeding --- framework/core/tests/integration/TestCase.php | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/framework/core/tests/integration/TestCase.php b/framework/core/tests/integration/TestCase.php index 916b1c06f..5c8c5526c 100644 --- a/framework/core/tests/integration/TestCase.php +++ b/framework/core/tests/integration/TestCase.php @@ -23,13 +23,6 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase { use BuildsHttpRequests; - protected function setUp(): void - { - parent::setUp(); - - $this->database()->beginTransaction(); - } - protected function tearDown(): void { parent::tearDown(); @@ -61,6 +54,10 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase $site->extendWith($this->extenders); $this->app = $site->bootApp(); + + $this->database()->beginTransaction(); + + $this->populateDatabase(); } return $this->app; @@ -103,13 +100,23 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase return $this->database; } + protected $databaseContent = []; + protected function prepareDatabase(array $tableData) + { + $this->databaseContent = array_merge_recursive( + $this->databaseContent, + $tableData + ); + } + + protected function populateDatabase() { // We temporarily disable foreign key checks to simplify this process. $this->database()->getSchemaBuilder()->disableForeignKeyConstraints(); // Then, insert all rows required for this test case. - foreach ($tableData as $table => $rows) { + foreach ($this->databaseContent as $table => $rows) { foreach ($rows as $row) { if ($table === 'settings') { $this->database()->table($table)->updateOrInsert( From 450ca9932569c97715c9f596a244077eb5f01013 Mon Sep 17 00:00:00 2001 From: Franz Liedke Date: Thu, 21 May 2020 22:36:52 +0200 Subject: [PATCH 06/13] Boot app explicitly to run seeds in time --- .../core/tests/integration/extenders/ApiControllerTest.php | 4 ++++ .../core/tests/integration/extenders/ApiSerializerTest.php | 2 ++ framework/core/tests/integration/extenders/ModelTest.php | 6 +++++- .../tests/integration/extenders/ModelVisibilityTest.php | 2 ++ framework/core/tests/integration/extenders/PolicyTest.php | 2 ++ framework/core/tests/integration/extenders/UserTest.php | 2 ++ 6 files changed, 17 insertions(+), 1 deletion(-) diff --git a/framework/core/tests/integration/extenders/ApiControllerTest.php b/framework/core/tests/integration/extenders/ApiControllerTest.php index 7b3e21d8f..349622aff 100644 --- a/framework/core/tests/integration/extenders/ApiControllerTest.php +++ b/framework/core/tests/integration/extenders/ApiControllerTest.php @@ -43,6 +43,8 @@ class ApiControllerTest extends TestCase ['id' => 3, 'title' => 'Custom Discussion Title', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'first_post_id' => 0, 'comment_count' => 1, 'is_private' => 0], ], ]); + + $this->app(); } /** @@ -327,6 +329,8 @@ class ApiControllerTest extends TestCase ]) ); + echo $response->getBody(); + $payload = json_decode($response->getBody(), true); $this->assertArrayHasKey('customSerializer', $payload['data']['attributes']); diff --git a/framework/core/tests/integration/extenders/ApiSerializerTest.php b/framework/core/tests/integration/extenders/ApiSerializerTest.php index 8e3e32236..ad1e3274f 100644 --- a/framework/core/tests/integration/extenders/ApiSerializerTest.php +++ b/framework/core/tests/integration/extenders/ApiSerializerTest.php @@ -42,6 +42,8 @@ class ApiSerializerTest extends TestCase ['id' => 1, 'discussion_id' => 3, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'type' => 'discussionRenamed', 'content' => '

can i haz relationz?

'], ], ]); + + $this->app(); } /** diff --git a/framework/core/tests/integration/extenders/ModelTest.php b/framework/core/tests/integration/extenders/ModelTest.php index 9d01dc1fb..f4e219975 100644 --- a/framework/core/tests/integration/extenders/ModelTest.php +++ b/framework/core/tests/integration/extenders/ModelTest.php @@ -32,6 +32,8 @@ class ModelTest extends TestCase $this->normalUser(), ], ]); + + $this->app(); } protected function prepPostsHierarchy() @@ -47,6 +49,8 @@ class ModelTest extends TestCase ['id' => 1, 'discussion_id' => 1, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'type' => 'discussionRenamed', 'content' => '

can i haz relationz?

'], ], ]); + + $this->app(); } /** @@ -161,12 +165,12 @@ class ModelTest extends TestCase }) ); - $this->prepDB(); $this->prepareDatabase([ 'discussions' => [ ['id' => 1, 'title' => __CLASS__, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'first_post_id' => 1, 'comment_count' => 1] ] ]); + $this->prepDB(); $user = User::find(1); diff --git a/framework/core/tests/integration/extenders/ModelVisibilityTest.php b/framework/core/tests/integration/extenders/ModelVisibilityTest.php index 69ff0820b..d05c98fb7 100644 --- a/framework/core/tests/integration/extenders/ModelVisibilityTest.php +++ b/framework/core/tests/integration/extenders/ModelVisibilityTest.php @@ -39,6 +39,8 @@ class ModelVisibilityTest extends TestCase $this->normalUser(), ] ]); + + $this->app(); } /** diff --git a/framework/core/tests/integration/extenders/PolicyTest.php b/framework/core/tests/integration/extenders/PolicyTest.php index b2f0420b8..082af73e9 100644 --- a/framework/core/tests/integration/extenders/PolicyTest.php +++ b/framework/core/tests/integration/extenders/PolicyTest.php @@ -41,6 +41,8 @@ class PolicyTest extends TestCase ['id' => 1, 'discussion_id' => 1, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => '

a normal reply - too-obscure

'], ] ]); + + $this->app(); } /** diff --git a/framework/core/tests/integration/extenders/UserTest.php b/framework/core/tests/integration/extenders/UserTest.php index 473805185..02ce5f00e 100644 --- a/framework/core/tests/integration/extenders/UserTest.php +++ b/framework/core/tests/integration/extenders/UserTest.php @@ -30,6 +30,8 @@ class UserTest extends TestCase ['key' => 'display_name_driver', 'value' => 'custom'], ] ]); + + $this->app(); } protected function registerTestPreference() From 823e6cbd2e3984943e74c6db6c7179637344e565 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Wed, 6 Jan 2021 22:16:26 -0500 Subject: [PATCH 07/13] Add @inheritDoc to all setUp and tearDown methods --- framework/core/tests/integration/TestCase.php | 3 +++ .../tests/integration/api/authentication/WithApiKeyTest.php | 3 +++ .../tests/integration/api/authentication/WithTokenTest.php | 3 +++ .../integration/api/csrf_protection/RequireCsrfTokenTest.php | 3 +++ .../core/tests/integration/api/discussions/DeletionTest.php | 3 +++ framework/core/tests/integration/api/discussions/ListTest.php | 3 +++ framework/core/tests/integration/api/discussions/ShowTest.php | 3 +++ framework/core/tests/integration/api/forum/ShowTest.php | 3 +++ framework/core/tests/integration/api/groups/CreateTest.php | 3 +++ framework/core/tests/integration/api/groups/ListTest.php | 3 +++ .../core/tests/integration/api/notifications/ListTest.php | 3 +++ framework/core/tests/integration/api/posts/CreateTest.php | 3 +++ framework/core/tests/integration/api/users/CreateTest.php | 3 +++ framework/core/tests/integration/api/users/ShowTest.php | 3 +++ framework/core/tests/integration/api/users/UpdateTest.php | 3 +++ framework/core/tests/unit/Foundation/ContainerUtilTest.php | 3 +++ .../IlluminateValidationExceptionHandlerTest.php | 3 +++ .../ExceptionHandler/ValidationExceptionHandlerTest.php | 3 +++ .../tests/unit/Settings/DatabaseSettingsRepositoryTest.php | 3 +++ .../tests/unit/Settings/MemoryCacheSettingsRepositoryTest.php | 3 +++ framework/core/tests/unit/User/AbstractPolicyTest.php | 3 +++ framework/core/tests/unit/User/AvatarUploaderTest.php | 3 +++ 22 files changed, 66 insertions(+) diff --git a/framework/core/tests/integration/TestCase.php b/framework/core/tests/integration/TestCase.php index 5c8c5526c..1bf515a41 100644 --- a/framework/core/tests/integration/TestCase.php +++ b/framework/core/tests/integration/TestCase.php @@ -23,6 +23,9 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase { use BuildsHttpRequests; + /** + * @inheritDoc + */ protected function tearDown(): void { parent::tearDown(); diff --git a/framework/core/tests/integration/api/authentication/WithApiKeyTest.php b/framework/core/tests/integration/api/authentication/WithApiKeyTest.php index 180a4d630..a61303e14 100644 --- a/framework/core/tests/integration/api/authentication/WithApiKeyTest.php +++ b/framework/core/tests/integration/api/authentication/WithApiKeyTest.php @@ -19,6 +19,9 @@ class WithApiKeyTest extends TestCase { use RetrievesAuthorizedUsers; + /** + * @inheritDoc + */ protected function setUp(): void { parent::setUp(); diff --git a/framework/core/tests/integration/api/authentication/WithTokenTest.php b/framework/core/tests/integration/api/authentication/WithTokenTest.php index 4405076ea..79d7b0014 100644 --- a/framework/core/tests/integration/api/authentication/WithTokenTest.php +++ b/framework/core/tests/integration/api/authentication/WithTokenTest.php @@ -17,6 +17,9 @@ class WithTokenTest extends TestCase { use RetrievesAuthorizedUsers; + /** + * @inheritDoc + */ protected function setUp(): void { parent::setUp(); diff --git a/framework/core/tests/integration/api/csrf_protection/RequireCsrfTokenTest.php b/framework/core/tests/integration/api/csrf_protection/RequireCsrfTokenTest.php index 965aeeef9..152bcf4a6 100644 --- a/framework/core/tests/integration/api/csrf_protection/RequireCsrfTokenTest.php +++ b/framework/core/tests/integration/api/csrf_protection/RequireCsrfTokenTest.php @@ -16,6 +16,9 @@ class RequireCsrfTokenTest extends TestCase { use RetrievesAuthorizedUsers; + /** + * @inheritDoc + */ protected function setUp(): void { parent::setUp(); diff --git a/framework/core/tests/integration/api/discussions/DeletionTest.php b/framework/core/tests/integration/api/discussions/DeletionTest.php index ea825fd9b..f81922ab6 100644 --- a/framework/core/tests/integration/api/discussions/DeletionTest.php +++ b/framework/core/tests/integration/api/discussions/DeletionTest.php @@ -17,6 +17,9 @@ class DeletionTest extends TestCase { use RetrievesAuthorizedUsers; + /** + * @inheritDoc + */ protected function setUp(): void { parent::setUp(); diff --git a/framework/core/tests/integration/api/discussions/ListTest.php b/framework/core/tests/integration/api/discussions/ListTest.php index b47bbe127..d09ea5afa 100644 --- a/framework/core/tests/integration/api/discussions/ListTest.php +++ b/framework/core/tests/integration/api/discussions/ListTest.php @@ -17,6 +17,9 @@ class ListTest extends TestCase { use RetrievesAuthorizedUsers; + /** + * @inheritDoc + */ protected function setUp(): void { parent::setUp(); diff --git a/framework/core/tests/integration/api/discussions/ShowTest.php b/framework/core/tests/integration/api/discussions/ShowTest.php index 79d036c16..cf6f82ca5 100644 --- a/framework/core/tests/integration/api/discussions/ShowTest.php +++ b/framework/core/tests/integration/api/discussions/ShowTest.php @@ -20,6 +20,9 @@ class ShowTest extends TestCase { use RetrievesAuthorizedUsers; + /** + * @inheritDoc + */ protected function setUp(): void { parent::setUp(); diff --git a/framework/core/tests/integration/api/forum/ShowTest.php b/framework/core/tests/integration/api/forum/ShowTest.php index 7ff520712..7bb072f10 100644 --- a/framework/core/tests/integration/api/forum/ShowTest.php +++ b/framework/core/tests/integration/api/forum/ShowTest.php @@ -17,6 +17,9 @@ class ShowTest extends TestCase { use RetrievesAuthorizedUsers; + /** + * @inheritDoc + */ protected function setUp(): void { parent::setUp(); diff --git a/framework/core/tests/integration/api/groups/CreateTest.php b/framework/core/tests/integration/api/groups/CreateTest.php index ba4b43303..a92e33877 100644 --- a/framework/core/tests/integration/api/groups/CreateTest.php +++ b/framework/core/tests/integration/api/groups/CreateTest.php @@ -18,6 +18,9 @@ class CreateTest extends TestCase { use RetrievesAuthorizedUsers; + /** + * @inheritDoc + */ protected function setUp(): void { parent::setUp(); diff --git a/framework/core/tests/integration/api/groups/ListTest.php b/framework/core/tests/integration/api/groups/ListTest.php index 9cef5476f..0b1f7bf74 100644 --- a/framework/core/tests/integration/api/groups/ListTest.php +++ b/framework/core/tests/integration/api/groups/ListTest.php @@ -17,6 +17,9 @@ class ListTest extends TestCase { use RetrievesAuthorizedUsers; + /** + * @inheritDoc + */ protected function setUp(): void { parent::setUp(); diff --git a/framework/core/tests/integration/api/notifications/ListTest.php b/framework/core/tests/integration/api/notifications/ListTest.php index a33a756de..8a0a0a563 100644 --- a/framework/core/tests/integration/api/notifications/ListTest.php +++ b/framework/core/tests/integration/api/notifications/ListTest.php @@ -16,6 +16,9 @@ class ListTest extends TestCase { use RetrievesAuthorizedUsers; + /** + * @inheritDoc + */ protected function setUp(): void { parent::setUp(); diff --git a/framework/core/tests/integration/api/posts/CreateTest.php b/framework/core/tests/integration/api/posts/CreateTest.php index cc023677c..d1a57151d 100644 --- a/framework/core/tests/integration/api/posts/CreateTest.php +++ b/framework/core/tests/integration/api/posts/CreateTest.php @@ -17,6 +17,9 @@ class CreateTest extends TestCase { use RetrievesAuthorizedUsers; + /** + * @inheritDoc + */ protected function setUp(): void { parent::setUp(); diff --git a/framework/core/tests/integration/api/users/CreateTest.php b/framework/core/tests/integration/api/users/CreateTest.php index 6e1d6d023..c3eb38ace 100644 --- a/framework/core/tests/integration/api/users/CreateTest.php +++ b/framework/core/tests/integration/api/users/CreateTest.php @@ -18,6 +18,9 @@ class CreateTest extends TestCase { use RetrievesAuthorizedUsers; + /** + * @inheritDoc + */ protected function setUp(): void { parent::setUp(); diff --git a/framework/core/tests/integration/api/users/ShowTest.php b/framework/core/tests/integration/api/users/ShowTest.php index 3fb0e7bd2..fc7557ee8 100644 --- a/framework/core/tests/integration/api/users/ShowTest.php +++ b/framework/core/tests/integration/api/users/ShowTest.php @@ -16,6 +16,9 @@ class ShowTest extends TestCase { use RetrievesAuthorizedUsers; + /** + * @inheritDoc + */ protected function setUp(): void { parent::setUp(); diff --git a/framework/core/tests/integration/api/users/UpdateTest.php b/framework/core/tests/integration/api/users/UpdateTest.php index be6b397dd..280554b81 100644 --- a/framework/core/tests/integration/api/users/UpdateTest.php +++ b/framework/core/tests/integration/api/users/UpdateTest.php @@ -16,6 +16,9 @@ class UpdateTest extends TestCase { use RetrievesAuthorizedUsers; + /** + * @inheritDoc + */ protected function setUp(): void { parent::setUp(); diff --git a/framework/core/tests/unit/Foundation/ContainerUtilTest.php b/framework/core/tests/unit/Foundation/ContainerUtilTest.php index f7b8a2992..d90764f3b 100644 --- a/framework/core/tests/unit/Foundation/ContainerUtilTest.php +++ b/framework/core/tests/unit/Foundation/ContainerUtilTest.php @@ -17,6 +17,9 @@ class ContainerUtilTest extends TestCase { private $container; + /** + * @inheritDoc + */ protected function setUp() { parent::setUp(); diff --git a/framework/core/tests/unit/Foundation/ErrorHandling/ExceptionHandler/IlluminateValidationExceptionHandlerTest.php b/framework/core/tests/unit/Foundation/ErrorHandling/ExceptionHandler/IlluminateValidationExceptionHandlerTest.php index 0bf6d4441..b3c3273c2 100644 --- a/framework/core/tests/unit/Foundation/ErrorHandling/ExceptionHandler/IlluminateValidationExceptionHandlerTest.php +++ b/framework/core/tests/unit/Foundation/ErrorHandling/ExceptionHandler/IlluminateValidationExceptionHandlerTest.php @@ -20,6 +20,9 @@ class IlluminateValidationExceptionHandlerTest extends TestCase { private $handler; + /** + * @inheritDoc + */ protected function setUp(): void { $this->handler = new IlluminateValidationExceptionHandler; diff --git a/framework/core/tests/unit/Foundation/ErrorHandling/ExceptionHandler/ValidationExceptionHandlerTest.php b/framework/core/tests/unit/Foundation/ErrorHandling/ExceptionHandler/ValidationExceptionHandlerTest.php index edb4c0892..aa8df566f 100644 --- a/framework/core/tests/unit/Foundation/ErrorHandling/ExceptionHandler/ValidationExceptionHandlerTest.php +++ b/framework/core/tests/unit/Foundation/ErrorHandling/ExceptionHandler/ValidationExceptionHandlerTest.php @@ -17,6 +17,9 @@ class ValidationExceptionHandlerTest extends TestCase { private $handler; + /** + * @inheritDoc + */ protected function setUp(): void { $this->handler = new ValidationExceptionHandler; diff --git a/framework/core/tests/unit/Settings/DatabaseSettingsRepositoryTest.php b/framework/core/tests/unit/Settings/DatabaseSettingsRepositoryTest.php index 56de7ffd8..0f3b88d78 100644 --- a/framework/core/tests/unit/Settings/DatabaseSettingsRepositoryTest.php +++ b/framework/core/tests/unit/Settings/DatabaseSettingsRepositoryTest.php @@ -19,6 +19,9 @@ class DatabaseSettingsRepositoryTest extends TestCase private $connection; private $repository; + /** + * @inheritDoc + */ protected function setUp(): void { $this->connection = m::mock(ConnectionInterface::class); diff --git a/framework/core/tests/unit/Settings/MemoryCacheSettingsRepositoryTest.php b/framework/core/tests/unit/Settings/MemoryCacheSettingsRepositoryTest.php index 4011e4d6b..a7d6f1d09 100644 --- a/framework/core/tests/unit/Settings/MemoryCacheSettingsRepositoryTest.php +++ b/framework/core/tests/unit/Settings/MemoryCacheSettingsRepositoryTest.php @@ -19,6 +19,9 @@ class MemoryCacheSettingsRepositoryTest extends TestCase private $baseRepository; private $repository; + /** + * @inheritDoc + */ protected function setUp(): void { $this->baseRepository = m::mock(SettingsRepositoryInterface::class); diff --git a/framework/core/tests/unit/User/AbstractPolicyTest.php b/framework/core/tests/unit/User/AbstractPolicyTest.php index d2ba2efac..0a32eed11 100644 --- a/framework/core/tests/unit/User/AbstractPolicyTest.php +++ b/framework/core/tests/unit/User/AbstractPolicyTest.php @@ -21,6 +21,9 @@ class AbstractPolicyTest extends TestCase private $policy; private $dispatcher; + /** + * @inheritDoc + */ protected function setUp(): void { $this->policy = m::mock(CustomUserPolicy::class)->makePartial(); diff --git a/framework/core/tests/unit/User/AvatarUploaderTest.php b/framework/core/tests/unit/User/AvatarUploaderTest.php index 392bbe70d..a85e3ed9a 100644 --- a/framework/core/tests/unit/User/AvatarUploaderTest.php +++ b/framework/core/tests/unit/User/AvatarUploaderTest.php @@ -24,6 +24,9 @@ class AvatarUploaderTest extends TestCase private $filesystem; private $uploader; + /** + * @inheritDoc + */ protected function setUp(): void { $this->dispatcher = m::mock(Dispatcher::class); From b92695a9c2a4cf2fe05479c413da813e42bc0545 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Wed, 6 Jan 2021 22:34:32 -0500 Subject: [PATCH 08/13] Tests: remove prepDb workaround Previously, the `prepareDatabase` method would directly modify the database, booting the app in the process. This would prevent any extenders from being applied, since `->extend()` has no effect once the app is booted. Since the new implementation of `prepareDatabase` simply registers seed data to be applied during app boot, the workaround of sticking this seed data into `prepDb` is no longer necessary, and seed data common to all test cases in a class can be provided in `setUp`. When needed, app boot is explicitly triggered in individual test cases by calling `$this->app()`. --- .../extenders/ApiControllerTest.php | 65 ++----------------- .../extenders/ApiSerializerTest.php | 21 ++---- .../tests/integration/extenders/CsrfTest.php | 2 - .../tests/integration/extenders/ModelTest.php | 38 ++++++----- .../integration/extenders/ModelUrlTest.php | 11 ++-- .../extenders/ModelVisibilityTest.php | 21 +++--- .../integration/extenders/PolicyTest.php | 31 ++++----- .../integration/extenders/SettingsTest.php | 19 ++---- .../integration/extenders/ThrottleApiTest.php | 15 ++--- .../tests/integration/extenders/UserTest.php | 31 +++++---- 10 files changed, 96 insertions(+), 158 deletions(-) diff --git a/framework/core/tests/integration/extenders/ApiControllerTest.php b/framework/core/tests/integration/extenders/ApiControllerTest.php index 349622aff..2c6be92ff 100644 --- a/framework/core/tests/integration/extenders/ApiControllerTest.php +++ b/framework/core/tests/integration/extenders/ApiControllerTest.php @@ -31,8 +31,13 @@ class ApiControllerTest extends TestCase { use RetrievesAuthorizedUsers; - protected function prepDb() + /** + * @inheritDoc + */ + protected function setUp(): void { + parent::setUp(); + $this->prepareDatabase([ 'users' => [ $this->normalUser() @@ -43,8 +48,6 @@ class ApiControllerTest extends TestCase ['id' => 3, 'title' => 'Custom Discussion Title', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'first_post_id' => 0, 'comment_count' => 1, 'is_private' => 0], ], ]); - - $this->app(); } /** @@ -59,8 +62,6 @@ class ApiControllerTest extends TestCase }) ); - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api/discussions/1', [ 'authenticatedAs' => 1, @@ -82,8 +83,6 @@ class ApiControllerTest extends TestCase ->prepareDataForSerialization(CustomPrepareDataSerializationInvokableClass::class) ); - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api/discussions/1', [ 'authenticatedAs' => 1, @@ -110,8 +109,6 @@ class ApiControllerTest extends TestCase }) ); - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api', [ 'authenticatedAs' => 1, @@ -136,8 +133,6 @@ class ApiControllerTest extends TestCase ->prepareDataForSerialization(CustomInvokableClassArgsReference::class) ); - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api', [ 'authenticatedAs' => 1, @@ -163,8 +158,6 @@ class ApiControllerTest extends TestCase }) ); - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api/discussions/1', [ 'authenticatedAs' => 1, @@ -194,8 +187,6 @@ class ApiControllerTest extends TestCase }) ); - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api/discussions/1', [ 'authenticatedAs' => 1, @@ -221,8 +212,6 @@ class ApiControllerTest extends TestCase }) ); - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api/discussions/1', [ 'authenticatedAs' => 1, @@ -252,8 +241,6 @@ class ApiControllerTest extends TestCase }) ); - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api/discussions/1', [ 'authenticatedAs' => 1, @@ -270,8 +257,6 @@ class ApiControllerTest extends TestCase */ public function custom_serializer_doesnt_work_by_default() { - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api/discussions/1', [ 'authenticatedAs' => 1, @@ -293,8 +278,6 @@ class ApiControllerTest extends TestCase ->setSerializer(CustomDiscussionSerializer::class) ); - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api/discussions/1', [ 'authenticatedAs' => 1, @@ -315,8 +298,6 @@ class ApiControllerTest extends TestCase (new Extend\ApiController(ShowPostController::class)) ->setSerializer(CustomPostSerializer::class, CustomApiControllerInvokableClass::class) ); - - $this->prepDb(); $this->prepareDatabase([ 'posts' => [ ['id' => 1, 'discussion_id' => 1, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'type' => 'comment', 'content' => '

foo bar

'], @@ -329,8 +310,6 @@ class ApiControllerTest extends TestCase ]) ); - echo $response->getBody(); - $payload = json_decode($response->getBody(), true); $this->assertArrayHasKey('customSerializer', $payload['data']['attributes']); @@ -348,8 +327,6 @@ class ApiControllerTest extends TestCase }) ); - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api/users/2', [ 'authenticatedAs' => 1, @@ -366,8 +343,6 @@ class ApiControllerTest extends TestCase */ public function custom_relationship_not_included_by_default() { - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api/users/2', [ 'authenticatedAs' => 1, @@ -394,8 +369,6 @@ class ApiControllerTest extends TestCase ->addInclude('customApiControllerRelation') ); - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api/users/2', [ 'authenticatedAs' => 1, @@ -421,8 +394,6 @@ class ApiControllerTest extends TestCase ->addOptionalInclude('customApiControllerRelation2') ); - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api/users/2', [ 'authenticatedAs' => 1, @@ -441,8 +412,6 @@ class ApiControllerTest extends TestCase */ public function custom_relationship_included_by_default() { - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api/users/2', [ 'authenticatedAs' => 1, @@ -464,8 +433,6 @@ class ApiControllerTest extends TestCase ->removeInclude('groups') ); - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api/users/2', [ 'authenticatedAs' => 1, @@ -492,8 +459,6 @@ class ApiControllerTest extends TestCase ->removeOptionalInclude('customApiControllerRelation2') ); - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api/users/2', [ 'authenticatedAs' => 1, @@ -510,8 +475,6 @@ class ApiControllerTest extends TestCase */ public function custom_limit_doesnt_work_by_default() { - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api/discussions', [ 'authenticatedAs' => 1, @@ -533,8 +496,6 @@ class ApiControllerTest extends TestCase ->setLimit(1) ); - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api/discussions', [ 'authenticatedAs' => 1, @@ -556,8 +517,6 @@ class ApiControllerTest extends TestCase ->setMaxLimit(1) ); - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api/discussions', [ 'authenticatedAs' => 1, @@ -576,8 +535,6 @@ class ApiControllerTest extends TestCase */ public function custom_sort_field_doesnt_exist_by_default() { - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api/discussions', [ 'authenticatedAs' => 1, @@ -601,8 +558,6 @@ class ApiControllerTest extends TestCase }) ); - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api/discussions', [ 'authenticatedAs' => 1, @@ -624,8 +579,6 @@ class ApiControllerTest extends TestCase ->addSortField('userId') ); - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api/discussions', [ 'authenticatedAs' => 1, @@ -645,8 +598,6 @@ class ApiControllerTest extends TestCase */ public function custom_sort_field_exists_by_default() { - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api/discussions', [ 'authenticatedAs' => 1, @@ -668,8 +619,6 @@ class ApiControllerTest extends TestCase ->removeSortField('createdAt') ); - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api/discussions', [ 'authenticatedAs' => 1, @@ -692,8 +641,6 @@ class ApiControllerTest extends TestCase ->setSort(['userId' => 'desc']) ); - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api/discussions', [ 'authenticatedAs' => 1, diff --git a/framework/core/tests/integration/extenders/ApiSerializerTest.php b/framework/core/tests/integration/extenders/ApiSerializerTest.php index ad1e3274f..8548d453c 100644 --- a/framework/core/tests/integration/extenders/ApiSerializerTest.php +++ b/framework/core/tests/integration/extenders/ApiSerializerTest.php @@ -27,8 +27,13 @@ class ApiSerializerTest extends TestCase { use RetrievesAuthorizedUsers; - protected function prepDb() + /** + * @inheritDoc + */ + protected function setUp(): void { + parent::setUp(); + $this->prepareDatabase([ 'users' => [ $this->normalUser() @@ -42,8 +47,6 @@ class ApiSerializerTest extends TestCase ['id' => 1, 'discussion_id' => 3, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'type' => 'discussionRenamed', 'content' => '

can i haz relationz?

'], ], ]); - - $this->app(); } /** @@ -329,8 +332,6 @@ class ApiSerializerTest extends TestCase ->hasMany('customSerializerRelation', DiscussionSerializer::class) ); - $this->prepDb(); - $request = $this->request('GET', '/api/users/2', [ 'authenticatedAs' => 1, ]); @@ -356,8 +357,6 @@ class ApiSerializerTest extends TestCase ->hasOne('customSerializerRelation', DiscussionSerializer::class) ); - $this->prepDb(); - $request = $this->request('GET', '/api/users/2', [ 'authenticatedAs' => 1, ]); @@ -385,8 +384,6 @@ class ApiSerializerTest extends TestCase }) ); - $this->prepDb(); - $request = $this->request('GET', '/api/users/2', [ 'authenticatedAs' => 1, ]); @@ -412,8 +409,6 @@ class ApiSerializerTest extends TestCase ->relationship('customSerializerRelation', CustomRelationshipInvokableClass::class) ); - $this->prepDb(); - $request = $this->request('GET', '/api/users/2', [ 'authenticatedAs' => 1, ]); @@ -439,8 +434,6 @@ class ApiSerializerTest extends TestCase ->hasMany('anotherCustomRelation', DiscussionSerializer::class) ); - $this->prepDb(); - $request = $this->request('GET', '/api/users/2', [ 'authenticatedAs' => 1, ]); @@ -472,8 +465,6 @@ class ApiSerializerTest extends TestCase }) ); - $this->prepDb(); - $request = $this->request('GET', '/api/users/2', [ 'authenticatedAs' => 1, ]); diff --git a/framework/core/tests/integration/extenders/CsrfTest.php b/framework/core/tests/integration/extenders/CsrfTest.php index db841e0a0..0b6f8cec0 100644 --- a/framework/core/tests/integration/extenders/CsrfTest.php +++ b/framework/core/tests/integration/extenders/CsrfTest.php @@ -79,8 +79,6 @@ class CsrfTest extends TestCase ->exemptRoute('users.create') ); - $this->prepDb(); - $response = $this->send( $this->request('POST', '/api/users', [ 'json' => [ diff --git a/framework/core/tests/integration/extenders/ModelTest.php b/framework/core/tests/integration/extenders/ModelTest.php index f4e219975..f40c71d90 100644 --- a/framework/core/tests/integration/extenders/ModelTest.php +++ b/framework/core/tests/integration/extenders/ModelTest.php @@ -25,23 +25,23 @@ class ModelTest extends TestCase { use RetrievesAuthorizedUsers; - protected function prepDb() + /** + * @inheritDoc + */ + protected function setUp(): void { + parent::setUp(); + $this->prepareDatabase([ 'users' => [ $this->normalUser(), ], ]); - - $this->app(); } protected function prepPostsHierarchy() { $this->prepareDatabase([ - 'users' => [ - $this->normalUser(), - ], 'discussions' => [ ['id' => 1, 'title' => 'Discussion with post', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'first_post_id' => 1, 'comment_count' => 1, 'is_private' => 0], ], @@ -49,8 +49,6 @@ class ModelTest extends TestCase ['id' => 1, 'discussion_id' => 1, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'type' => 'discussionRenamed', 'content' => '

can i haz relationz?

'], ], ]); - - $this->app(); } /** @@ -58,7 +56,7 @@ class ModelTest extends TestCase */ public function custom_relationship_does_not_exist_by_default() { - $this->prepDB(); + $this->app(); $user = User::find(1); @@ -76,7 +74,7 @@ class ModelTest extends TestCase ->hasOne('customRelation', Discussion::class, 'user_id') ); - $this->prepDB(); + $this->app(); $user = User::find(1); @@ -93,7 +91,7 @@ class ModelTest extends TestCase ->hasMany('customRelation', Discussion::class, 'user_id') ); - $this->prepDB(); + $this->app(); $user = User::find(1); @@ -110,7 +108,7 @@ class ModelTest extends TestCase ->belongsTo('customRelation', Discussion::class, 'user_id') ); - $this->prepDB(); + $this->app(); $user = User::find(1); @@ -129,7 +127,7 @@ class ModelTest extends TestCase }) ); - $this->prepDB(); + $this->app(); $user = User::find(1); @@ -146,7 +144,7 @@ class ModelTest extends TestCase ->relationship('customRelation', CustomRelationClass::class) ); - $this->prepDB(); + $this->app(); $user = User::find(1); @@ -170,7 +168,8 @@ class ModelTest extends TestCase ['id' => 1, 'title' => __CLASS__, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'first_post_id' => 1, 'comment_count' => 1] ] ]); - $this->prepDB(); + + $this->app(); $user = User::find(1); @@ -190,6 +189,8 @@ class ModelTest extends TestCase $this->prepPostsHierarchy(); + $this->app(); + $post = CommentPost::find(1); $this->assertInstanceOf(Discussion::class, $post->ancestor); @@ -210,6 +211,8 @@ class ModelTest extends TestCase $this->prepPostsHierarchy(); + $this->app(); + $post = DiscussionRenamedPost::find(1); $this->assertInstanceOf(Discussion::class, $post->ancestor); @@ -229,6 +232,9 @@ class ModelTest extends TestCase ); $this->prepPostsHierarchy(); + + $this->app(); + $post = DiscussionRenamedPost::find(1); $this->assertInstanceOf(User::class, $post->ancestor); @@ -247,7 +253,7 @@ class ModelTest extends TestCase }) ); - $this->prepDB(); + $this->app(); $group = Group::find(1); diff --git a/framework/core/tests/integration/extenders/ModelUrlTest.php b/framework/core/tests/integration/extenders/ModelUrlTest.php index 44abd1ef6..ab3eef49b 100644 --- a/framework/core/tests/integration/extenders/ModelUrlTest.php +++ b/framework/core/tests/integration/extenders/ModelUrlTest.php @@ -21,8 +21,13 @@ class ModelUrlTest extends TestCase { use RetrievesAuthorizedUsers; - protected function prepDb() + /** + * @inheritDoc + */ + protected function setUp(): void { + parent::setUp(); + $userClass = User::class; $this->prepareDatabase([ 'users' => [ @@ -39,8 +44,6 @@ class ModelUrlTest extends TestCase */ public function uses_default_driver_by_default() { - $this->prepDb(); - $slugManager = $this->app()->getContainer()->make(SlugManager::class); $testUser = User::find(1); @@ -56,8 +59,6 @@ class ModelUrlTest extends TestCase { $this->extend((new Extend\ModelUrl(User::class))->addSlugDriver('testDriver', TestSlugDriver::class)); - $this->prepDb(); - $slugManager = $this->app()->getContainer()->make(SlugManager::class); $testUser = User::find(1); diff --git a/framework/core/tests/integration/extenders/ModelVisibilityTest.php b/framework/core/tests/integration/extenders/ModelVisibilityTest.php index d05c98fb7..25322167e 100644 --- a/framework/core/tests/integration/extenders/ModelVisibilityTest.php +++ b/framework/core/tests/integration/extenders/ModelVisibilityTest.php @@ -23,8 +23,13 @@ class ModelVisibilityTest extends TestCase { use RetrievesAuthorizedUsers; - protected function prepDb() + /** + * @inheritDoc + */ + protected function setUp(): void { + parent::setUp(); + $this->prepareDatabase([ 'discussions' => [ ['id' => 1, 'title' => 'Empty discussion', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'first_post_id' => null, 'comment_count' => 0, 'is_private' => 0], @@ -39,8 +44,6 @@ class ModelVisibilityTest extends TestCase $this->normalUser(), ] ]); - - $this->app(); } /** @@ -48,7 +51,7 @@ class ModelVisibilityTest extends TestCase */ public function user_can_see_posts_by_default() { - $this->prepDb(); + $this->app(); $actor = User::find(2); @@ -69,7 +72,7 @@ class ModelVisibilityTest extends TestCase }, 'view') ); - $this->prepDb(); + $this->app(); $actor = User::find(2); @@ -90,7 +93,7 @@ class ModelVisibilityTest extends TestCase }, 'view') ); - $this->prepDb(); + $this->app(); $actor = User::find(2); @@ -115,7 +118,7 @@ class ModelVisibilityTest extends TestCase }, 'view') ); - $this->prepDb(); + $this->app(); $actor = User::find(2); @@ -140,7 +143,7 @@ class ModelVisibilityTest extends TestCase }, 'viewPrivate') ); - $this->prepDb(); + $this->app(); $actor = User::find(2); @@ -169,7 +172,7 @@ class ModelVisibilityTest extends TestCase }) ); - $this->prepDb(); + $this->app(); $actor = User::find(2); diff --git a/framework/core/tests/integration/extenders/PolicyTest.php b/framework/core/tests/integration/extenders/PolicyTest.php index 082af73e9..978d0fff9 100644 --- a/framework/core/tests/integration/extenders/PolicyTest.php +++ b/framework/core/tests/integration/extenders/PolicyTest.php @@ -28,8 +28,13 @@ class PolicyTest extends TestCase // Request body to hide discussions sent in tests. protected $hideQuery = ['authenticatedAs' => 2, 'json' => ['data' => ['attributes' => ['isHidden' => true]]]]; - private function prepDb() + /** + * @inheritDoc + */ + protected function setUp(): void { + parent::setUp(); + $this->prepareDatabase([ 'users' => [ $this->normalUser(), @@ -41,8 +46,6 @@ class PolicyTest extends TestCase ['id' => 1, 'discussion_id' => 1, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => '

a normal reply - too-obscure

'], ] ]); - - $this->app(); } /** @@ -50,8 +53,6 @@ class PolicyTest extends TestCase */ public function unrelated_user_cant_hide_discussion_by_default() { - $this->prepDb(); - $response = $this->send( $this->request('PATCH', '/api/discussions/1', $this->hideQuery) ); @@ -69,8 +70,6 @@ class PolicyTest extends TestCase ->modelPolicy(Discussion::class, CustomPolicy::class) ); - $this->prepDb(); - $response = $this->send( $this->request('PATCH', '/api/discussions/1', $this->hideQuery) ); @@ -89,8 +88,6 @@ class PolicyTest extends TestCase ->modelPolicy(Discussion::class, CustomPolicy::class) ); - $this->prepDb(); - $response = $this->send( $this->request('PATCH', '/api/discussions/1', $this->hideQuery) ); @@ -110,8 +107,6 @@ class PolicyTest extends TestCase ->modelPolicy(Discussion::class, CustomPolicy::class) ); - $this->prepDb(); - $response = $this->send( $this->request('PATCH', '/api/discussions/1', $this->hideQuery) ); @@ -132,8 +127,6 @@ class PolicyTest extends TestCase ->modelPolicy(Discussion::class, ForceAllowHidePolicy::class) ); - $this->prepDb(); - $response = $this->send( $this->request('PATCH', '/api/discussions/1', $this->hideQuery) ); @@ -146,7 +139,7 @@ class PolicyTest extends TestCase */ public function regular_user_cant_start_discussions_by_default() { - $this->prepDb(); + $this->app(); $user = User::find(2); @@ -163,7 +156,7 @@ class PolicyTest extends TestCase ->globalPolicy(GlobalStartDiscussionPolicy::class) ); - $this->prepDb(); + $this->app(); $user = User::find(2); @@ -180,7 +173,7 @@ class PolicyTest extends TestCase ->globalPolicy(GlobalStartDiscussionPolicy::class) ); - $this->prepDb(); + $this->app(); $user = User::find(2); @@ -192,7 +185,7 @@ class PolicyTest extends TestCase */ public function unrelated_user_cant_hide_post_by_default() { - $this->prepDb(); + $this->app(); $user = User::find(2); @@ -207,7 +200,7 @@ class PolicyTest extends TestCase $this->extend( (new Extend\Policy)->modelPolicy(CommentPost::class, CommentPostChildClassPolicy::class) ); - $this->prepDb(); + $this->app(); $user = User::find(2); @@ -223,7 +216,7 @@ class PolicyTest extends TestCase (new Extend\Policy)->modelPolicy(Post::class, PostParentClassPolicy::class), (new Extend\Policy)->modelPolicy(CommentPost::class, CommentPostChildClassPolicy::class) ); - $this->prepDb(); + $this->app(); $user = User::find(2); diff --git a/framework/core/tests/integration/extenders/SettingsTest.php b/framework/core/tests/integration/extenders/SettingsTest.php index 77c41d70e..d279bfd27 100644 --- a/framework/core/tests/integration/extenders/SettingsTest.php +++ b/framework/core/tests/integration/extenders/SettingsTest.php @@ -17,8 +17,13 @@ class SettingsTest extends TestCase { use RetrievesAuthorizedUsers; - protected function prepDb() + /** + * @inheritDoc + */ + protected function setUp(): void { + parent::setUp(); + $this->prepareDatabase([ 'users' => [ $this->normalUser() @@ -35,8 +40,6 @@ class SettingsTest extends TestCase */ public function custom_setting_isnt_serialized_by_default() { - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api', [ 'authenticatedAs' => 1, @@ -58,8 +61,6 @@ class SettingsTest extends TestCase ->serializeToForum('customPrefix.customSetting', 'custom-prefix.custom_setting') ); - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api', [ 'authenticatedAs' => 1, @@ -84,8 +85,6 @@ class SettingsTest extends TestCase }) ); - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api', [ 'authenticatedAs' => 1, @@ -108,8 +107,6 @@ class SettingsTest extends TestCase ->serializeToForum('customPrefix.customSetting2', 'custom-prefix.custom_setting2', CustomInvokableClass::class) ); - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api', [ 'authenticatedAs' => 1, @@ -132,8 +129,6 @@ class SettingsTest extends TestCase ->serializeToForum('customPrefix.noCustomSetting', 'custom-prefix.no_custom_setting', null, 'customDefault') ); - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api', [ 'authenticatedAs' => 1, @@ -158,8 +153,6 @@ class SettingsTest extends TestCase }, 'customDefault') ); - $this->prepDb(); - $response = $this->send( $this->request('GET', '/api', [ 'authenticatedAs' => 1, diff --git a/framework/core/tests/integration/extenders/ThrottleApiTest.php b/framework/core/tests/integration/extenders/ThrottleApiTest.php index 33a4853e7..b2e88798b 100644 --- a/framework/core/tests/integration/extenders/ThrottleApiTest.php +++ b/framework/core/tests/integration/extenders/ThrottleApiTest.php @@ -17,8 +17,13 @@ class ThrottleApiTest extends TestCase { use RetrievesAuthorizedUsers; - protected function prepDb(): void + /** + * @inheritDoc + */ + protected function setUp(): void { + parent::setUp(); + $this->prepareDatabase([ 'users' => [ $this->normalUser(), @@ -31,8 +36,6 @@ class ThrottleApiTest extends TestCase */ public function list_discussions_not_restricted_by_default() { - $this->prepDb(); - $response = $this->send($this->request('GET', '/api/discussions', ['authenticatedAs' => 2])); $this->assertEquals(200, $response->getStatusCode()); @@ -49,8 +52,6 @@ class ThrottleApiTest extends TestCase } })); - $this->prepDb(); - $response = $this->send($this->request('GET', '/api/discussions', ['authenticatedAs' => 2])); $this->assertEquals(429, $response->getStatusCode()); @@ -74,10 +75,6 @@ class ThrottleApiTest extends TestCase }) ); - $this->prepDb(); - - $this->prepDb(); - $response = $this->send($this->request('GET', '/api/discussions', ['authenticatedAs' => 2])); $this->assertEquals(200, $response->getStatusCode()); diff --git a/framework/core/tests/integration/extenders/UserTest.php b/framework/core/tests/integration/extenders/UserTest.php index 02ce5f00e..497bc7f95 100644 --- a/framework/core/tests/integration/extenders/UserTest.php +++ b/framework/core/tests/integration/extenders/UserTest.php @@ -20,8 +20,13 @@ class UserTest extends TestCase { use RetrievesAuthorizedUsers; - protected function prepDb() + /** + * @inheritDoc + */ + protected function setUp(): void { + parent::setUp(); + $this->prepareDatabase([ 'users' => [ $this->normalUser(), @@ -30,8 +35,6 @@ class UserTest extends TestCase ['key' => 'display_name_driver', 'value' => 'custom'], ] ]); - - $this->app(); } protected function registerTestPreference() @@ -47,7 +50,7 @@ class UserTest extends TestCase */ public function username_display_name_driver_used_by_default() { - $this->prepDb(); + $this->app(); $user = User::find(1); @@ -64,7 +67,7 @@ class UserTest extends TestCase ->displayNameDriver('custom', CustomDisplayNameDriver::class) ); - $this->prepDb(); + $this->app(); $user = User::find(1); @@ -76,7 +79,8 @@ class UserTest extends TestCase */ public function user_has_permissions_for_expected_groups_if_no_processors_added() { - $this->prepDb(); + $this->app(); + $user = User::find(2); $this->assertContains('viewUserList', $user->getPermissions()); @@ -93,7 +97,8 @@ class UserTest extends TestCase }); })); - $this->prepDb(); + $this->app(); + $user = User::find(2); $this->assertNotContains('viewUserList', $user->getPermissions()); @@ -106,7 +111,8 @@ class UserTest extends TestCase { $this->extend((new Extend\User)->permissionGroups(CustomGroupProcessorClass::class)); - $this->prepDb(); + $this->app(); + $user = User::find(2); $this->assertNotContains('viewUserList', $user->getPermissions()); @@ -118,7 +124,8 @@ class UserTest extends TestCase public function can_add_user_preference() { $this->registerTestPreference(); - $this->prepDb(); + + $this->app(); /** @var User $user */ $user = User::find(2); @@ -131,7 +138,8 @@ class UserTest extends TestCase public function can_store_user_preference() { $this->registerTestPreference(); - $this->prepDb(); + + $this->app(); /** @var User $user */ $user = User::find(2); @@ -147,7 +155,8 @@ class UserTest extends TestCase public function storing_user_preference_modified_by_transformer() { $this->registerTestPreference(); - $this->prepDb(); + + $this->app(); /** @var User $user */ $user = User::find(2); From 17e2dc21ec6c0197b27e1a83be95eb25ca497360 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Thu, 7 Jan 2021 11:24:52 -0500 Subject: [PATCH 09/13] Tests: Comply with default permissions Before transactions, each test class would need to explicitly state starting state for permissions, which made the initial permission configuration somewhat arbitrary. Now, we might as well use the initial state of the default installation. One of the User show_test tests has been commented out until --- .../tests/integration/api/users/ShowTest.php | 108 ++++++++++-------- .../integration/extenders/PolicyTest.php | 12 +- 2 files changed, 68 insertions(+), 52 deletions(-) diff --git a/framework/core/tests/integration/api/users/ShowTest.php b/framework/core/tests/integration/api/users/ShowTest.php index fc7557ee8..c3bcefaee 100644 --- a/framework/core/tests/integration/api/users/ShowTest.php +++ b/framework/core/tests/integration/api/users/ShowTest.php @@ -30,6 +30,16 @@ class ShowTest extends TestCase ]); } + private function forbidGuestsFromSeeingForum() + { + $this->database()->table('group_permission')->where('permission', 'viewDiscussions')->where('group_id', 2)->delete(); + } + + private function forbidMembersFromSearchingUsers() + { + $this->database()->table('group_permission')->where('permission', 'viewUserList')->where('group_id', 3)->delete(); + } + /** * @test */ @@ -63,22 +73,52 @@ class ShowTest extends TestCase /** * @test */ - public function guest_cannot_see_user() + public function guest_can_see_user_by_default() { $response = $this->send( $this->request('GET', '/api/users/2') ); + $this->assertEquals(200, $response->getStatusCode()); + } + + /** + * @test + */ + public function guest_can_see_user_by_slug_by_default() + { + $response = $this->send( + $this->request('GET', '/api/users/normal')->withQueryParams([ + 'bySlug' => true + ]) + ); + + $this->assertEquals(200, $response->getStatusCode()); + } + + /** + * @test + */ + public function guest_cant_see_user_if_blocked() + { + $this->forbidGuestsFromSeeingForum(); + + $response = $this->send( + $this->request('GET', '/api/users/2') + ); + $this->assertEquals(404, $response->getStatusCode()); } /** * @test */ - public function guest_cannot_see_user_by_slug() + public function guest_cant_see_user_by_slug_if_blocked() { + $this->forbidGuestsFromSeeingForum(); + $response = $this->send( - $this->request('GET', '/api/users/2')->withQueryParams([ + $this->request('GET', '/api/users/normal')->withQueryParams([ 'bySlug' => true ]) ); @@ -119,7 +159,7 @@ class ShowTest extends TestCase /** * @test */ - public function user_cant_see_others_by_default() + public function user_can_see_others_by_default() { $response = $this->send( $this->request('GET', '/api/users/1', [ @@ -127,55 +167,31 @@ class ShowTest extends TestCase ]) ); - $this->assertEquals(404, $response->getStatusCode()); - } - - /** - * @test - */ - public function user_cant_see_others_by_default_via_slug() - { - $response = $this->send( - $this->request('GET', '/api/users/admin', [ - 'authenticatedAs' => 2, - ])->withQueryParams([ - 'bySlug' => true - ]) - ); - - $this->assertEquals(404, $response->getStatusCode()); - } - - /** - * @test - */ - public function user_can_see_others_if_allowed() - { - $this->prepareDatabase([ - 'group_permission' => [ - ['permission' => 'viewDiscussions', 'group_id' => 3], - ] - ]); - - $response = $this->send( - $this->request('GET', '/api/users/1', [ - 'authenticatedAs' => 2, - ]) - ); - $this->assertEquals(200, $response->getStatusCode()); } /** * @test */ - public function user_can_see_others_if_allowed_via_slug() + public function user_can_see_others_by_default_via_slug() { - $this->prepareDatabase([ - 'group_permission' => [ - ['permission' => 'viewDiscussions', 'group_id' => 3], - ] - ]); + $response = $this->send( + $this->request('GET', '/api/users/admin', [ + 'authenticatedAs' => 2, + ])->withQueryParams([ + 'bySlug' => true + ]) + ); + + $this->assertEquals(200, $response->getStatusCode()); + } + + /** + * @test + */ + public function user_can_still_see_others_via_slug_even_if_cant_search() + { + $this->forbidMembersFromSearchingUsers(); $response = $this->send( $this->request('GET', '/api/users/admin', [ diff --git a/framework/core/tests/integration/extenders/PolicyTest.php b/framework/core/tests/integration/extenders/PolicyTest.php index 978d0fff9..984885277 100644 --- a/framework/core/tests/integration/extenders/PolicyTest.php +++ b/framework/core/tests/integration/extenders/PolicyTest.php @@ -137,19 +137,19 @@ class PolicyTest extends TestCase /** * @test */ - public function regular_user_cant_start_discussions_by_default() + public function regular_user_can_start_discussions_by_default() { $this->app(); $user = User::find(2); - $this->assertEquals(false, $user->can('startDiscussion')); + $this->assertEquals(true, $user->can('startDiscussion')); } /** * @test */ - public function regular_user_can_start_discussions_if_granted_by_global_policy() + public function regular_user_cant_start_discussions_if_blocked_by_global_policy() { $this->extend( (new Extend\Policy) @@ -160,7 +160,7 @@ class PolicyTest extends TestCase $user = User::find(2); - $this->assertEquals(true, $user->can('startDiscussion')); + $this->assertEquals(false, $user->can('startDiscussion')); } /** @@ -177,7 +177,7 @@ class PolicyTest extends TestCase $user = User::find(2); - $this->assertEquals(false, $user->can('startDiscussion', Discussion::find(1))); + $this->assertEquals(true, $user->can('startDiscussion', Discussion::find(1))); } /** @@ -260,7 +260,7 @@ class GlobalStartDiscussionPolicy extends AbstractPolicy { protected function startDiscussion(User $user) { - return $this->allow(); + return $this->deny(); } } From eeac5ffce9988ff26f425e2053edd3744a99532e Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Thu, 7 Jan 2021 12:20:06 -0500 Subject: [PATCH 10/13] Tests: Add missing instantiation of data --- .../integration/api/discussions/CreateTest.php | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/framework/core/tests/integration/api/discussions/CreateTest.php b/framework/core/tests/integration/api/discussions/CreateTest.php index e24969937..c0cf9c210 100644 --- a/framework/core/tests/integration/api/discussions/CreateTest.php +++ b/framework/core/tests/integration/api/discussions/CreateTest.php @@ -18,6 +18,20 @@ class CreateTest extends TestCase { use RetrievesAuthorizedUsers; + /** + * @inheritDoc + */ + protected function setUp(): void + { + parent::setUp(); + + $this->prepareDatabase([ + 'users' => [ + $this->normalUser(), + ] + ]); + } + /** * @test */ From 7c0d98c63d2f7541c26ea9961a8f5d04af25c5bb Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Thu, 7 Jan 2021 14:10:53 -0500 Subject: [PATCH 11/13] Tests: purge settings cache Some tests need to change settings, but since MemoryCacheSettingsRepository caches settings in-memory, those changes aren't reflected. The new `purgeSettingsCache` removes it from the container, eliminating that cache. For UserTest, we also need to regenerate the display name driver, since that's set statically on boot, before we'll get a change to clear the settings cache. --- .../core/tests/integration/UsesSettings.php | 25 +++++++++++++++++++ .../integration/extenders/ModelUrlTest.php | 6 +++++ .../integration/extenders/SettingsTest.php | 14 +++++++++++ .../tests/integration/extenders/UserTest.php | 15 +++++++++++ 4 files changed, 60 insertions(+) create mode 100644 framework/core/tests/integration/UsesSettings.php diff --git a/framework/core/tests/integration/UsesSettings.php b/framework/core/tests/integration/UsesSettings.php new file mode 100644 index 000000000..d37aa0f7b --- /dev/null +++ b/framework/core/tests/integration/UsesSettings.php @@ -0,0 +1,25 @@ +app()->getContainer()->forgetInstance(SettingsRepositoryInterface::class); + } +} diff --git a/framework/core/tests/integration/extenders/ModelUrlTest.php b/framework/core/tests/integration/extenders/ModelUrlTest.php index ab3eef49b..a5b095ce8 100644 --- a/framework/core/tests/integration/extenders/ModelUrlTest.php +++ b/framework/core/tests/integration/extenders/ModelUrlTest.php @@ -15,11 +15,13 @@ use Flarum\Http\SlugDriverInterface; use Flarum\Http\SlugManager; use Flarum\Tests\integration\RetrievesAuthorizedUsers; use Flarum\Tests\integration\TestCase; +use Flarum\Tests\integration\UsesSettings; use Flarum\User\User; class ModelUrlTest extends TestCase { use RetrievesAuthorizedUsers; + use UsesSettings; /** * @inheritDoc @@ -44,6 +46,8 @@ class ModelUrlTest extends TestCase */ public function uses_default_driver_by_default() { + $this->purgeSettingsCache(); + $slugManager = $this->app()->getContainer()->make(SlugManager::class); $testUser = User::find(1); @@ -59,6 +63,8 @@ class ModelUrlTest extends TestCase { $this->extend((new Extend\ModelUrl(User::class))->addSlugDriver('testDriver', TestSlugDriver::class)); + $this->purgeSettingsCache(); + $slugManager = $this->app()->getContainer()->make(SlugManager::class); $testUser = User::find(1); diff --git a/framework/core/tests/integration/extenders/SettingsTest.php b/framework/core/tests/integration/extenders/SettingsTest.php index d279bfd27..f85d175e8 100644 --- a/framework/core/tests/integration/extenders/SettingsTest.php +++ b/framework/core/tests/integration/extenders/SettingsTest.php @@ -10,12 +10,14 @@ namespace Flarum\Tests\integration\extenders; use Flarum\Extend; +use Flarum\Tests\integration\UsesSettings; use Flarum\Tests\integration\RetrievesAuthorizedUsers; use Flarum\Tests\integration\TestCase; class SettingsTest extends TestCase { use RetrievesAuthorizedUsers; + use UsesSettings; /** * @inheritDoc @@ -40,6 +42,8 @@ class SettingsTest extends TestCase */ public function custom_setting_isnt_serialized_by_default() { + $this->purgeSettingsCache(); + $response = $this->send( $this->request('GET', '/api', [ 'authenticatedAs' => 1, @@ -61,6 +65,8 @@ class SettingsTest extends TestCase ->serializeToForum('customPrefix.customSetting', 'custom-prefix.custom_setting') ); + $this->purgeSettingsCache(); + $response = $this->send( $this->request('GET', '/api', [ 'authenticatedAs' => 1, @@ -85,6 +91,8 @@ class SettingsTest extends TestCase }) ); + $this->purgeSettingsCache(); + $response = $this->send( $this->request('GET', '/api', [ 'authenticatedAs' => 1, @@ -107,6 +115,8 @@ class SettingsTest extends TestCase ->serializeToForum('customPrefix.customSetting2', 'custom-prefix.custom_setting2', CustomInvokableClass::class) ); + $this->purgeSettingsCache(); + $response = $this->send( $this->request('GET', '/api', [ 'authenticatedAs' => 1, @@ -129,6 +139,8 @@ class SettingsTest extends TestCase ->serializeToForum('customPrefix.noCustomSetting', 'custom-prefix.no_custom_setting', null, 'customDefault') ); + $this->purgeSettingsCache(); + $response = $this->send( $this->request('GET', '/api', [ 'authenticatedAs' => 1, @@ -153,6 +165,8 @@ class SettingsTest extends TestCase }, 'customDefault') ); + $this->purgeSettingsCache(); + $response = $this->send( $this->request('GET', '/api', [ 'authenticatedAs' => 1, diff --git a/framework/core/tests/integration/extenders/UserTest.php b/framework/core/tests/integration/extenders/UserTest.php index 497bc7f95..f8b22d5bc 100644 --- a/framework/core/tests/integration/extenders/UserTest.php +++ b/framework/core/tests/integration/extenders/UserTest.php @@ -12,6 +12,7 @@ namespace Flarum\Tests\integration\extenders; use Flarum\Extend; use Flarum\Tests\integration\RetrievesAuthorizedUsers; use Flarum\Tests\integration\TestCase; +use Flarum\Tests\integration\UsesSettings; use Flarum\User\DisplayName\DriverInterface; use Flarum\User\User; use Illuminate\Support\Arr; @@ -19,6 +20,7 @@ use Illuminate\Support\Arr; class UserTest extends TestCase { use RetrievesAuthorizedUsers; + use UsesSettings; /** * @inheritDoc @@ -37,6 +39,17 @@ class UserTest extends TestCase ]); } + /** + * Purge the settings cache and reset the new display name driver. + */ + protected function recalculateDisplayNameDriver() + { + $this->purgeSettingsCache(); + $container = $this->app()->getContainer(); + $container->forgetInstance('flarum.user.display_name.driver'); + User::setDisplayNameDriver($container->make('flarum.user.display_name.driver')); + } + protected function registerTestPreference() { $this->extend( @@ -51,6 +64,7 @@ class UserTest extends TestCase public function username_display_name_driver_used_by_default() { $this->app(); + $this->recalculateDisplayNameDriver(); $user = User::find(1); @@ -68,6 +82,7 @@ class UserTest extends TestCase ); $this->app(); + $this->recalculateDisplayNameDriver(); $user = User::find(1); From 54e0d1b7daa89e5b339205c8f926cb20ac09d264 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Thu, 7 Jan 2021 17:23:56 -0500 Subject: [PATCH 12/13] Fix fulltext search tests Under InnoDB, database entries created in transactions are not processed by fulltext indexes until the transaction is committed. To work around this, cases that test fulltext search have been split off into a separate class that adds and removes seed discussions/posts outside of transactions during setUp/tearDown. --- .../integration/api/discussions/ListTest.php | 92 ------------- .../ListTestWithFulltextSearch.php | 130 ++++++++++++++++++ 2 files changed, 130 insertions(+), 92 deletions(-) create mode 100644 framework/core/tests/integration/api/discussions/ListTestWithFulltextSearch.php diff --git a/framework/core/tests/integration/api/discussions/ListTest.php b/framework/core/tests/integration/api/discussions/ListTest.php index d09ea5afa..48e1ee949 100644 --- a/framework/core/tests/integration/api/discussions/ListTest.php +++ b/framework/core/tests/integration/api/discussions/ListTest.php @@ -67,96 +67,4 @@ class ListTest extends TestCase $this->assertEquals(200, $response->getStatusCode()); } - - /** - * @test - */ - public function can_search_for_word_in_post() - { - $this->database()->table('discussions')->insert([ - ['id' => 2, 'title' => 'lightsail in title', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'comment_count' => 1], - ['id' => 3, 'title' => 'not in title', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'comment_count' => 1], - ]); - - $this->database()->table('posts')->insert([ - ['id' => 2, 'discussion_id' => 2, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'type' => 'comment', 'content' => '

not in text

'], - ['id' => 3, 'discussion_id' => 3, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'type' => 'comment', 'content' => '

lightsail in text

'], - ]); - - $response = $this->send( - $this->request('GET', '/api/discussions') - ->withQueryParams([ - 'filter' => ['q' => 'lightsail'], - 'include' => 'mostRelevantPost', - ]) - ); - - $data = json_decode($response->getBody()->getContents(), true); - $ids = array_map(function ($row) { - return $row['id']; - }, $data['data']); - - // Order-independent comparison - $this->assertEquals(['3'], $ids, 'IDs do not match', 0.0, 10, true); - } - - /** - * @test - */ - public function ignores_non_word_characters_when_searching() - { - $this->database()->table('discussions')->insert([ - ['id' => 2, 'title' => 'lightsail in title', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'comment_count' => 1], - ['id' => 3, 'title' => 'not in title', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'comment_count' => 1], - ]); - - $this->database()->table('posts')->insert([ - ['id' => 2, 'discussion_id' => 2, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'type' => 'comment', 'content' => '

not in text

'], - ['id' => 3, 'discussion_id' => 3, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'type' => 'comment', 'content' => '

lightsail in text

'], - ]); - - $response = $this->send( - $this->request('GET', '/api/discussions') - ->withQueryParams([ - 'filter' => ['q' => 'lightsail+'], - 'include' => 'mostRelevantPost', - ]) - ); - - $data = json_decode($response->getBody()->getContents(), true); - $ids = array_map(function ($row) { - return $row['id']; - }, $data['data']); - - // Order-independent comparison - $this->assertEquals(['3'], $ids, 'IDs do not match', 0.0, 10, true); - } - - /** - * @test - */ - public function search_for_special_characters_gives_empty_result() - { - $response = $this->send( - $this->request('GET', '/api/discussions') - ->withQueryParams([ - 'filter' => ['q' => '*'], - 'include' => 'mostRelevantPost', - ]) - ); - - $data = json_decode($response->getBody()->getContents(), true); - $this->assertEquals([], $data['data']); - - $response = $this->send( - $this->request('GET', '/api/discussions') - ->withQueryParams([ - 'filter' => ['q' => '@'], - 'include' => 'mostRelevantPost', - ]) - ); - - $data = json_decode($response->getBody()->getContents(), true); - $this->assertEquals([], $data['data']); - } } diff --git a/framework/core/tests/integration/api/discussions/ListTestWithFulltextSearch.php b/framework/core/tests/integration/api/discussions/ListTestWithFulltextSearch.php new file mode 100644 index 000000000..e2de57bb5 --- /dev/null +++ b/framework/core/tests/integration/api/discussions/ListTestWithFulltextSearch.php @@ -0,0 +1,130 @@ +database()->rollBack(); + + // We need to insert these outside of a transaction, because FULLTEXT indexing, + // which is needed for search, doesn't happen in transactions. + // We clean it up explcitly at the end. + $this->database()->table('discussions')->insert([ + ['id' => 1, 'title' => 'lightsail in title', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'comment_count' => 1], + ['id' => 2, 'title' => 'not in title', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'comment_count' => 1], + ]); + + $this->database()->table('posts')->insert([ + ['id' => 1, 'discussion_id' => 1, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => '

not in text

'], + ['id' => 2, 'discussion_id' => 2, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => '

lightsail in text

'], + ]); + + // We need to call these again, since we rolled back the transaction started by `::app()`. + $this->database()->beginTransaction(); + + $this->populateDatabase(); + } + + /** + * @inheritDoc + */ + protected function tearDown(): void + { + parent::tearDown(); + + $this->database()->table('discussions')->whereIn('id', [1,2])->delete(); + $this->database()->table('posts')->whereIn('id', [1, 2])->delete(); + } + + /** + * @test + */ + public function can_search_for_word_in_post() + { + $response = $this->send( + $this->request('GET', '/api/discussions') + ->withQueryParams([ + 'filter' => ['q' => 'lightsail'], + 'include' => 'mostRelevantPost', + ]) + ); + + $data = json_decode($response->getBody()->getContents(), true); + $ids = array_map(function ($row) { + return $row['id']; + }, $data['data']); + + // Order-independent comparison + $this->assertEquals(['3'], $ids, 'IDs do not match', 0.0, 10, true); + } + + /** + * @test + */ + public function ignores_non_word_characters_when_searching() + { + $response = $this->send( + $this->request('GET', '/api/discussions') + ->withQueryParams([ + 'filter' => ['q' => 'lightsail+'], + 'include' => 'mostRelevantPost', + ]) + ); + + $data = json_decode($response->getBody()->getContents(), true); + $ids = array_map(function ($row) { + return $row['id']; + }, $data['data']); + + // Order-independent comparison + $this->assertEquals(['3'], $ids, 'IDs do not match', 0.0, 10, true); + } + + /** + * @test + */ + public function search_for_special_characters_gives_empty_result() + { + $response = $this->send( + $this->request('GET', '/api/discussions') + ->withQueryParams([ + 'filter' => ['q' => '*'], + 'include' => 'mostRelevantPost', + ]) + ); + + $data = json_decode($response->getBody()->getContents(), true); + $this->assertEquals([], $data['data']); + + $response = $this->send( + $this->request('GET', '/api/discussions') + ->withQueryParams([ + 'filter' => ['q' => '@'], + 'include' => 'mostRelevantPost', + ]) + ); + + $data = json_decode($response->getBody()->getContents(), true); + $this->assertEquals([], $data['data']); + } +} From a5a8b075dde9191ada66007b864b5902c65bf1b8 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Thu, 7 Jan 2021 20:39:47 -0500 Subject: [PATCH 13/13] Apply fixes from StyleCI [ci skip] [skip ci] --- .../tests/integration/api/authentication/WithApiKeyTest.php | 1 - .../integration/api/discussions/ListTestWithFulltextSearch.php | 2 +- framework/core/tests/integration/extenders/SettingsTest.php | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/framework/core/tests/integration/api/authentication/WithApiKeyTest.php b/framework/core/tests/integration/api/authentication/WithApiKeyTest.php index a61303e14..fdbf14128 100644 --- a/framework/core/tests/integration/api/authentication/WithApiKeyTest.php +++ b/framework/core/tests/integration/api/authentication/WithApiKeyTest.php @@ -13,7 +13,6 @@ use Carbon\Carbon; use Flarum\Api\ApiKey; use Flarum\Tests\integration\RetrievesAuthorizedUsers; use Flarum\Tests\integration\TestCase; -use Illuminate\Support\Str; class WithApiKeyTest extends TestCase { diff --git a/framework/core/tests/integration/api/discussions/ListTestWithFulltextSearch.php b/framework/core/tests/integration/api/discussions/ListTestWithFulltextSearch.php index e2de57bb5..ce9edf8f9 100644 --- a/framework/core/tests/integration/api/discussions/ListTestWithFulltextSearch.php +++ b/framework/core/tests/integration/api/discussions/ListTestWithFulltextSearch.php @@ -52,7 +52,7 @@ class ListTest extends TestCase { parent::tearDown(); - $this->database()->table('discussions')->whereIn('id', [1,2])->delete(); + $this->database()->table('discussions')->whereIn('id', [1, 2])->delete(); $this->database()->table('posts')->whereIn('id', [1, 2])->delete(); } diff --git a/framework/core/tests/integration/extenders/SettingsTest.php b/framework/core/tests/integration/extenders/SettingsTest.php index f85d175e8..84433de25 100644 --- a/framework/core/tests/integration/extenders/SettingsTest.php +++ b/framework/core/tests/integration/extenders/SettingsTest.php @@ -10,9 +10,9 @@ namespace Flarum\Tests\integration\extenders; use Flarum\Extend; -use Flarum\Tests\integration\UsesSettings; use Flarum\Tests\integration\RetrievesAuthorizedUsers; use Flarum\Tests\integration\TestCase; +use Flarum\Tests\integration\UsesSettings; class SettingsTest extends TestCase {