From b99246234ba642481263fb23aa4f43755c05565d Mon Sep 17 00:00:00 2001 From: Pig Fang Date: Sat, 7 Sep 2019 10:18:24 +0800 Subject: [PATCH] Tweak tests and containers --- app/Http/Controllers/AdminController.php | 2 +- app/Http/Controllers/SetupController.php | 52 ++++++------- app/Providers/AppServiceProvider.php | 4 +- app/Services/Option.php | 5 -- app/Services/Webpack.php | 8 +- app/helpers.php | 2 +- tests/AdminControllerTest.php | 1 + tests/AuthControllerTest.php | 14 ++++ tests/ServicesTest/WebpackTest.php | 19 +++-- tests/SetupControllerTest.php | 98 ++++++++++++------------ 10 files changed, 110 insertions(+), 95 deletions(-) diff --git a/app/Http/Controllers/AdminController.php b/app/Http/Controllers/AdminController.php index 0546736d..ae437924 100644 --- a/app/Http/Controllers/AdminController.php +++ b/app/Http/Controllers/AdminController.php @@ -376,7 +376,7 @@ class AdminController extends Controller 'version' => config('app.version'), 'env' => config('app.env'), 'debug' => config('app.debug') ? trans('general.yes') : trans('general.no'), - 'commit' => Str::limit(app('webpack')->commit, 10, ''), + 'commit' => Str::limit(resolve(\App\Services\Webpack::class)->commit, 10, ''), 'laravel' => app()->version(), ], 'server' => [ diff --git a/app/Http/Controllers/SetupController.php b/app/Http/Controllers/SetupController.php index 34a6b7fb..9ca67916 100644 --- a/app/Http/Controllers/SetupController.php +++ b/app/Http/Controllers/SetupController.php @@ -2,18 +2,16 @@ namespace App\Http\Controllers; -use DB; -use Log; -use File; -use Option; -use Artisan; use App\Models\User; use Illuminate\Support\Str; use Illuminate\Http\Request; use Composer\Semver\Comparator; +use Illuminate\Database\Connection; use Illuminate\Filesystem\Filesystem; use App\Exceptions\PrettyPageException; +use Illuminate\Database\DatabaseManager; use Symfony\Component\Finder\SplFileInfo; +use Illuminate\Contracts\Console\Kernel as Artisan; class SetupController extends Controller { @@ -26,17 +24,19 @@ class SetupController extends Controller return view('setup.wizard.welcome'); } - public function database(Request $request) - { + public function database( + Request $request, + Filesystem $filesystem, + Connection $connection, + DatabaseManager $manager + ) { if ($request->isMethod('get')) { try { - DB::getPdo(); + $connection->getPdo(); return redirect('setup/info'); - // @codeCoverageIgnoreStart } catch (\Exception $e) { return view('setup.wizard.database'); - // @codeCoverageIgnoreEnd } } @@ -51,7 +51,7 @@ class SetupController extends Controller ]); try { - DB::connection('temp')->getPdo(); + $manager->connection('temp')->getPdo(); } catch (\Exception $e) { $msg = iconv('gbk', 'utf-8', $e->getMessage()); $type = humanize_db_type($request->input('type')); @@ -62,7 +62,7 @@ class SetupController extends Controller ); } - $content = File::get(base_path('.env')); + $content = $filesystem->get(base_path('.env')); $content = preg_replace( '/DB_CONNECTION.+/', 'DB_CONNECTION = '.$request->input('type'), @@ -98,12 +98,12 @@ class SetupController extends Controller 'DB_PREFIX = '.$request->input('prefix'), $content ); - File::put(base_path('.env'), $content); + $filesystem->put(base_path('.env'), $content); return redirect('setup/info'); } - public function finish(Request $request, Filesystem $filesystem) + public function finish(Request $request, Filesystem $filesystem, Artisan $artisan) { $data = $this->validate($request, [ 'email' => 'required|email', @@ -113,31 +113,29 @@ class SetupController extends Controller ]); if ($request->has('generate_random')) { - Artisan::call('key:generate'); - Artisan::call('salt:random'); + $artisan->call('key:generate'); + $artisan->call('salt:random'); } - Artisan::call('jwt:secret', ['--no-interaction' => true]); - Artisan::call('passport:keys', ['--no-interaction' => true]); + $artisan->call('jwt:secret', ['--no-interaction' => true]); + $artisan->call('passport:keys', ['--no-interaction' => true]); // Create tables - Artisan::call('migrate', [ + $artisan->call('migrate', [ '--force' => true, '--path' => [ 'database/migrations', 'vendor/laravel/passport/database/migrations', ], ]); - Log::info('[SetupWizard] Tables migrated.'); - - Option::set('site_name', $request->input('site_name')); $siteUrl = url('/'); - if (Str::endsWith($siteUrl, '/index.php')) { $siteUrl = substr($siteUrl, 0, -10); // @codeCoverageIgnore } - - Option::set('site_url', $siteUrl); + option([ + 'site_name' => $request->input('site_name'), + 'site_url' => $siteUrl, + ]); // Register super admin $user = new User; @@ -162,7 +160,7 @@ class SetupController extends Controller ]); } - public function update(Filesystem $filesystem) + public function update(Filesystem $filesystem, Artisan $artisan) { collect($filesystem->files(database_path('update_scripts'))) ->filter(function (SplFileInfo $file) { @@ -175,7 +173,7 @@ class SetupController extends Controller }); option(['version' => config('app.version')]); - Artisan::call('view:clear'); + $artisan->call('view:clear'); $filesystem->put(storage_path('install.lock'), ''); return view('setup.updates.success'); diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index a07130af..1d59b901 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -24,7 +24,7 @@ class AppServiceProvider extends ServiceProvider $this->app->singleton(\App\Services\Option::class); $this->app->alias(\App\Services\Option::class, 'options'); $this->app->singleton('parsedown', \Parsedown::class); - $this->app->singleton('webpack', \App\Services\Webpack::class); + $this->app->singleton(\App\Services\Webpack::class); $this->app->singleton(\App\Services\Filter::class); } @@ -45,7 +45,7 @@ class AppServiceProvider extends ServiceProvider Event::listen(Events\RenderingHeader::class, function ($event) { $blessing = [ 'version' => config('app.version'), - 'commit' => app('webpack')->commit, + 'commit' => resolve(\App\Services\Webpack::class)->commit, 'locale' => config('app.locale'), 'fallback_locale' => config('app.fallback_locale'), 'base_url' => url('/'), diff --git a/app/Services/Option.php b/app/Services/Option.php index d7af6b93..cbe4931b 100644 --- a/app/Services/Option.php +++ b/app/Services/Option.php @@ -76,11 +76,6 @@ class Option } } - public function has($key) - { - return $this->items->has($key); - } - public function all(): array { return $this->items->all(); diff --git a/app/Services/Webpack.php b/app/Services/Webpack.php index df33d695..ab1e79d2 100644 --- a/app/Services/Webpack.php +++ b/app/Services/Webpack.php @@ -4,18 +4,18 @@ declare(strict_types=1); namespace App\Services; -use File; use Illuminate\Support\Arr; +use Illuminate\Filesystem\Filesystem; class Webpack { protected $manifest = []; - public function __construct() + public function __construct(Filesystem $filesystem) { $path = public_path('app/manifest.json'); - if (File::exists($path)) { - $this->manifest = json_decode(File::get($path), true); + if ($filesystem->exists($path)) { + $this->manifest = json_decode($filesystem->get($path), true); } } diff --git a/app/helpers.php b/app/helpers.php index b509a215..b21e1009 100644 --- a/app/helpers.php +++ b/app/helpers.php @@ -16,7 +16,7 @@ if (! function_exists('webpack_assets')) { return "http://$host:8080/$relativeUri"; // @codeCoverageIgnoreEnd } else { - $path = app('webpack')->$relativeUri; + $path = resolve(\App\Services\Webpack::class)->$relativeUri; $cdn = option('cdn_address'); return $cdn ? "$cdn/app/$path" : url("/app/$path"); diff --git a/tests/AdminControllerTest.php b/tests/AdminControllerTest.php index c3c74d69..43a69e00 100644 --- a/tests/AdminControllerTest.php +++ b/tests/AdminControllerTest.php @@ -26,6 +26,7 @@ class AdminControllerTest extends TestCase public function testChartData() { factory(User::class)->create(); + factory(User::class)->create(['register_at' => '2019-01-01 00:00:00']); factory(Texture::class)->create(); $this->getJson('/admin/chart') ->assertJson(['labels' => [ diff --git a/tests/AuthControllerTest.php b/tests/AuthControllerTest.php index 7d5cb267..b6e42382 100644 --- a/tests/AuthControllerTest.php +++ b/tests/AuthControllerTest.php @@ -509,6 +509,20 @@ class AuthControllerTest extends TestCase $this->assertTrue($user->verifyPassword('12345678')); } + public function testCaptcha() + { + $this->mock(\Gregwar\Captcha\CaptchaBuilder::class, function ($mock) { + $mock->shouldReceive('build')->with(100, 34)->once(); + $mock->shouldReceive('getPhrase')->once()->andReturn('くみこ'); + $mock->shouldReceive('output')->once()->andReturn(''); + }); + $this->get('/auth/captcha') + ->assertSuccessful() + ->assertHeader('Content-Type', 'image/jpeg') + ->assertHeader('Cache-Control', 'no-store, private') + ->assertSessionHas('captcha', 'くみこ'); + } + public function testFillEmail() { $user = factory(User::class)->create(['email' => '']); diff --git a/tests/ServicesTest/WebpackTest.php b/tests/ServicesTest/WebpackTest.php index bcbfaff5..f2cf6bb3 100644 --- a/tests/ServicesTest/WebpackTest.php +++ b/tests/ServicesTest/WebpackTest.php @@ -2,15 +2,24 @@ namespace Tests; -use File; +use Illuminate\Filesystem\Filesystem; class WebpackTest extends TestCase { public function testManifest() { - File::shouldReceive('exists')->andReturn(true); - File::shouldReceive('get')->andReturn(json_encode(['a' => 'b'])); - $key = 'a'; - $this->assertEquals('b', app('webpack')->$key); + $this->mock(Filesystem::class, function ($mock) { + $mock->shouldReceive('exists') + ->with(public_path('app/manifest.json')) + ->once() + ->andReturn(true); + + $mock->shouldReceive('get') + ->with(public_path('app/manifest.json')) + ->once() + ->andReturn(json_encode(['a' => 'b'])); + }); + $this->assertEquals('b', resolve(\App\Services\Webpack::class)->{'a'}); + $this->assertEquals('', resolve(\App\Services\Webpack::class)->{'nope'}); } } diff --git a/tests/SetupControllerTest.php b/tests/SetupControllerTest.php index 62a6e90b..e2d83820 100644 --- a/tests/SetupControllerTest.php +++ b/tests/SetupControllerTest.php @@ -3,16 +3,21 @@ namespace Tests; use Illuminate\Support\Str; -use Illuminate\Support\Facades\File; use Illuminate\Filesystem\Filesystem; -use Illuminate\Support\Facades\Artisan; use Symfony\Component\Finder\SplFileInfo; +use Illuminate\Contracts\Console\Kernel as Artisan; use Illuminate\Foundation\Testing\DatabaseTransactions; class SetupControllerTest extends TestCase { use DatabaseTransactions; + public function setUp(): void + { + parent::setUp(); + $this->spy(\App\Services\Webpack::class); + } + public function testWelcome() { $this->mock(Filesystem::class, function ($mock) { @@ -42,6 +47,15 @@ class SetupControllerTest extends TestCase ->with(storage_path('install.lock'), '') ->atLeast(1) ->andReturn(true); + + $mock->shouldReceive('get') + ->with(base_path('.env')) + ->once() + ->andReturn('DB_CONNECTION = abc'); + $mock->shouldReceive('put') + ->with(base_path('.env'), 'DB_CONNECTION = '.env('DB_CONNECTION')) + ->once() + ->andReturn(true); }); $fake = [ @@ -53,31 +67,21 @@ class SetupControllerTest extends TestCase 'password' => env('DB_PASSWORD'), 'prefix' => '', ]; - File::shouldReceive('get')->with(base_path('.env'))->andReturn(''); - File::shouldReceive('put')->with(base_path('.env'), ''); $this->post('/setup/database', $fake)->assertRedirect('/setup/info'); - $this->get('/setup/database')->assertRedirect('/setup/info'); - } - public function testReportDatabaseConnectionError() - { - $this->mock(Filesystem::class, function ($mock) { - $mock->shouldReceive('exists') - ->with(storage_path('install.lock')) - ->atLeast(1) - ->andReturn(false); - $mock->shouldReceive('put') - ->with(storage_path('install.lock'), '') - ->atLeast(1) - ->andReturn(true); + $this->mock(\Illuminate\Database\DatabaseManager::class, function ($mock) { + $mock->shouldReceive('connection')->andThrow(new \Exception())->once(); }); + $this->post('/setup/database', ['type' => 'sqlite']) + ->assertSee( + trans('setup.database.connection-error', ['type' => 'SQLite', 'msg' => '']) + ); - $this->post('/setup/database', ['type' => 'sqlite', 'host' => 'placeholder', 'db' => 'test']) - ->assertSee(trans('setup.database.connection-error', [ - 'type' => 'SQLite', - 'msg' => 'Database (test) does not exist.', - ])); + $this->mock(\Illuminate\Database\Connection::class, function ($mock) { + $mock->shouldReceive('getPdo')->andThrow(new \Exception()); + }); + $this->get('/setup/database')->assertViewIs('setup.wizard.database'); } public function testFinish() @@ -154,33 +158,25 @@ class SetupControllerTest extends TestCase 'password_confirmation' => '12345678', ])->assertDontSee(trans('setup.wizard.finish.title')); - // Regenerate keys - Artisan::shouldReceive('call') - ->with('key:generate') - ->once() - ->andReturn(true); - Artisan::shouldReceive('call') - ->with('salt:random') - ->once() - ->andReturn(true); - Artisan::shouldReceive('call') - ->with('jwt:secret', ['--no-interaction' => true]) - ->once() - ->andReturn(true); - Artisan::shouldReceive('call') - ->with('passport:keys', ['--no-interaction' => true]) - ->once() - ->andReturn(true); - Artisan::shouldReceive('call') - ->with('migrate', [ - '--force' => true, - '--path' => [ - 'database/migrations', - 'vendor/laravel/passport/database/migrations', - ], - ]) - ->once() - ->andReturn(true); + $this->spy(Artisan::class, function ($spy) { + $spy->shouldReceive('call')->with('key:generate')->once(); + $spy->shouldReceive('call')->with('salt:random')->once(); + $spy->shouldReceive('call') + ->with('jwt:secret', ['--no-interaction' => true]) + ->once(); + $spy->shouldReceive('call') + ->with('passport:keys', ['--no-interaction' => true]) + ->once(); + $spy->shouldReceive('call') + ->with('migrate', [ + '--force' => true, + '--path' => [ + 'database/migrations', + 'vendor/laravel/passport/database/migrations', + ], + ]) + ->once(); + }); $this->post('/setup/finish', [ 'email' => 'a@b.c', 'nickname' => 'nickname', @@ -229,7 +225,9 @@ class SetupControllerTest extends TestCase ->with('/100.0.0.php') ->once(); }); - Artisan::shouldReceive('call')->with('view:clear')->once(); + $this->spy(Artisan::class, function ($spy) { + $spy->shouldReceive('call')->with('view:clear')->once(); + }); config(['app.version' => '100.0.0']); $this->actAs('superAdmin')