From 867d3bd1714a4a9832877026d209dc7afe792c64 Mon Sep 17 00:00:00 2001 From: nanaya Date: Mon, 6 Nov 2023 19:11:30 +0900 Subject: [PATCH] Always create when running test instead --- app/Libraries/Groups.php | 50 +------------------ app/Models/Group.php | 17 +++++++ phpunit.dusk.xml | 3 ++ phpunit.xml | 3 ++ tests/Browser/SanityTest.php | 5 +- .../BeatmapsControllerSoloScoresTest.php | 1 - tests/Jobs/RemoveBeatmapsetSoloScoresTest.php | 1 - tests/Models/Solo/ScoreEsIndexTest.php | 1 - tests/SeederExtension.php | 38 ++++++++++++++ tests/TestCase.php | 18 +++---- 10 files changed, 72 insertions(+), 65 deletions(-) create mode 100644 tests/SeederExtension.php diff --git a/app/Libraries/Groups.php b/app/Libraries/Groups.php index 653fedffcb6..d3cbb0384f2 100644 --- a/app/Libraries/Groups.php +++ b/app/Libraries/Groups.php @@ -7,8 +7,6 @@ use App\Models\Group; use App\Traits\Memoizes; -use Ds\Set; -use Exception; use Illuminate\Database\Eloquent\Collection; use Illuminate\Database\Eloquent\ModelNotFoundException; @@ -61,56 +59,10 @@ public function byIdOrFail(int|string|null $id): Group /** * Get a group by its identifier (e.g. "admin"). - * - * If the requested group doesn't exist and it's one of the privilege - * related groups, a new is created. */ public function byIdentifier(string $id): ?Group { - $group = $this->allByIdentifier()->get($id); - - if ($group === null) { - static $privGroups = new Set([ - 'admin', - 'alumni', - 'announce', - 'bng', - 'bng_limited', - 'bot', - 'default', - 'dev', - 'gmt', - 'loved', - 'nat', - 'no_profile', - ]); - - if (!$privGroups->contains($id)) { - return null; - } - - try { - $group = Group::create([ - 'group_desc' => '', - 'group_name' => $id, - 'group_type' => 2, - 'identifier' => $id, - 'short_name' => $id, - ])->fresh(); - } catch (Exception $ex) { - if (!is_sql_unique_exception($ex)) { - throw $ex; - } - $group = Group::firstWhere(['identifier' => $id]); - } - - // TODO: This shouldn't have to be called here, since it's already - // called by `Group::afterCommit`, but `Group::afterCommit` isn't - // running in tests when creating/saving `Group`s. - $this->resetMemoized(); - } - - return $group; + return $this->allByIdentifier()->get($id); } protected function fetch(): Collection diff --git a/app/Models/Group.php b/app/Models/Group.php index 6d3c76cf11f..aa1c2025893 100644 --- a/app/Models/Group.php +++ b/app/Models/Group.php @@ -35,6 +35,23 @@ */ class Group extends Model implements AfterCommit { + // Identifier of groups which involved in permission checks + // and assumed to always exist in database. + const PRIV_GROUPS = [ + 'admin', + 'alumni', + 'announce', + 'bng', + 'bng_limited', + 'bot', + 'default', + 'dev', + 'gmt', + 'loved', + 'nat', + 'no_profile', + ]; + public $timestamps = false; protected $casts = [ diff --git a/phpunit.dusk.xml b/phpunit.dusk.xml index 60392c93245..0fe4666ae8f 100644 --- a/phpunit.dusk.xml +++ b/phpunit.dusk.xml @@ -8,6 +8,9 @@ convertWarningsToExceptions="true" processIsolation="false" stopOnFailure="false"> + + + ./tests/Browser diff --git a/phpunit.xml b/phpunit.xml index 18724d60148..4015d372f6c 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -4,6 +4,9 @@ bootstrap="vendor/autoload.php" colors="true" > + + + ./tests/ diff --git a/tests/Browser/SanityTest.php b/tests/Browser/SanityTest.php index d386f98887e..1cf3bf25431 100644 --- a/tests/Browser/SanityTest.php +++ b/tests/Browser/SanityTest.php @@ -100,7 +100,6 @@ private static function cleanup() Authorize::truncate(); TopicTrack::truncate(); Genre::truncate(); - Group::truncate(); Language::truncate(); LoginAttempt::truncate(); NewsPost::truncate(); @@ -111,8 +110,6 @@ private static function cleanup() UserNotification::truncate(); UserProfileCustomization::truncate(); UserStatistics\Osu::truncate(); - - app('groups')->resetMemoized(); } private static function createScaffolding() @@ -232,7 +229,7 @@ private static function createScaffolding() ]); // factory for /g/* - self::$scaffolding['group'] = Group::factory()->create(); + self::$scaffolding['group'] = Group::first(); // factory for comments self::$scaffolding['comment'] = Comment::factory()->create([ diff --git a/tests/Controllers/BeatmapsControllerSoloScoresTest.php b/tests/Controllers/BeatmapsControllerSoloScoresTest.php index 9c00f778509..4c746135de0 100644 --- a/tests/Controllers/BeatmapsControllerSoloScoresTest.php +++ b/tests/Controllers/BeatmapsControllerSoloScoresTest.php @@ -164,7 +164,6 @@ public static function tearDownAfterClass(): void Beatmapset::truncate(); Country::truncate(); Genre::truncate(); - Group::truncate(); Language::truncate(); SoloScore::truncate(); User::truncate(); diff --git a/tests/Jobs/RemoveBeatmapsetSoloScoresTest.php b/tests/Jobs/RemoveBeatmapsetSoloScoresTest.php index 342d9ad7bec..51aef7ccba7 100644 --- a/tests/Jobs/RemoveBeatmapsetSoloScoresTest.php +++ b/tests/Jobs/RemoveBeatmapsetSoloScoresTest.php @@ -70,7 +70,6 @@ public function testHandle() Beatmapset::truncate(); Country::truncate(); Genre::truncate(); - Group::truncate(); Language::truncate(); Score::truncate(); ScorePerformance::truncate(); diff --git a/tests/Models/Solo/ScoreEsIndexTest.php b/tests/Models/Solo/ScoreEsIndexTest.php index fcc0a79d133..7b71e711867 100644 --- a/tests/Models/Solo/ScoreEsIndexTest.php +++ b/tests/Models/Solo/ScoreEsIndexTest.php @@ -105,7 +105,6 @@ public static function tearDownAfterClass(): void Beatmapset::truncate(); Country::truncate(); Genre::truncate(); - Group::truncate(); Language::truncate(); Score::truncate(); User::truncate(); diff --git a/tests/SeederExtension.php b/tests/SeederExtension.php new file mode 100644 index 00000000000..765722223fb --- /dev/null +++ b/tests/SeederExtension.php @@ -0,0 +1,38 @@ +. Licensed under the GNU Affero General Public License v3.0. +// See the LICENCE file in the repository root for full licence text. + +declare(strict_types=1); + +namespace Tests; + +use App\Models\Group; +use PHPUnit\Runner\AfterLastTestHook; +use PHPUnit\Runner\BeforeFirstTestHook; + +class SeederExtension implements AfterLastTestHook, BeforeFirstTestHook +{ + public function executeAfterLastTest(): void + { + TestCase::withDbAccess(function () { + Group::truncate(); + }); + } + + public function executeBeforeFirstTest(): void + { + TestCase::withDbAccess(function () { + Group::truncate(); + foreach (Group::PRIV_GROUPS as $identifier) { + Group::create([ + 'group_desc' => '', + 'group_name' => $identifier, + 'group_type' => 2, + 'identifier' => $identifier, + 'short_name' => $identifier, + ]); + } + }); + } +} diff --git a/tests/TestCase.php b/tests/TestCase.php index 5d05eeff10d..fb3b10e1670 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -32,6 +32,15 @@ class TestCase extends BaseTestCase { use ArraySubsetAsserts, CreatesApplication, DatabaseTransactions; + public static function withDbAccess(callable $callback): void + { + $db = (new static())->createApplication()->make('db'); + + $callback(); + + static::resetAppDb($db); + } + protected static function reindexScores() { $search = new ScoreSearch(); @@ -54,15 +63,6 @@ protected static function resetAppDb(DatabaseManager $database): void } } - protected static function withDbAccess(callable $callback): void - { - $db = (new static())->createApplication()->make('db'); - - $callback(); - - static::resetAppDb($db); - } - protected $connectionsToTransact = [ 'mysql', 'mysql-chat',