From 99d116f4ce6902612625a59bdda9d043fdfdb852 Mon Sep 17 00:00:00 2001 From: James Cole Date: Sat, 3 Mar 2018 10:15:39 +0100 Subject: [PATCH] Improve test coverage. --- app/Api/V1/Controllers/UserController.php | 2 +- app/Api/V1/Requests/UserRequest.php | 4 +- test.sh | 27 +-- .../Controllers/TransactionControllerTest.php | 4 +- .../Api/V1/Controllers/UserControllerTest.php | 166 ++++++++++++++++++ 5 files changed, 187 insertions(+), 16 deletions(-) diff --git a/app/Api/V1/Controllers/UserController.php b/app/Api/V1/Controllers/UserController.php index 642c7c01c4..29acb5307e 100644 --- a/app/Api/V1/Controllers/UserController.php +++ b/app/Api/V1/Controllers/UserController.php @@ -78,7 +78,7 @@ class UserController extends Controller return response()->json([], 204); } - throw new AccessDeniedException(''); + throw new AccessDeniedException(''); // @codeCoverageIgnore } /** diff --git a/app/Api/V1/Requests/UserRequest.php b/app/Api/V1/Requests/UserRequest.php index 81c7a54703..9be50cef8c 100644 --- a/app/Api/V1/Requests/UserRequest.php +++ b/app/Api/V1/Requests/UserRequest.php @@ -38,12 +38,12 @@ class UserRequest extends Request { // Only allow authenticated users if (!auth()->check()) { - return false; + return false; // @codeCoverageIgnore } /** @var User $user */ $user = auth()->user(); if (!$user->hasRole('owner')) { - return false; + return false; // @codeCoverageIgnore } return true; diff --git a/test.sh b/test.sh index 4b4580b8f8..deaae5758b 100755 --- a/test.sh +++ b/test.sh @@ -17,11 +17,14 @@ featuretestclass='' unitflag='' unittestclass='' +apiflag='' +apitestclass='' + verbalflag='' testsuite='' configfile='phpunit.xml'; -while getopts 'vcrtf:u:s:' flag; do +while getopts 'vcrtfa:u:s:' flag; do case "${flag}" in r) resetTestFlag='true' @@ -47,6 +50,11 @@ while getopts 'vcrtf:u:s:' flag; do unittestclass=./tests/Unit/$OPTARG echo "Will only run Unit test $OPTARG" ;; + a) + apiflag='true' + apitestclass=./tests/Api/$OPTARG + echo "Will only run Api test $OPTARG" + ;; s) testsuite="--testsuite $OPTARG" echo "Will only run test suite '$OPTARG'" @@ -55,13 +63,12 @@ while getopts 'vcrtf:u:s:' flag; do esac done -if [[ $coverageflag == "true" && ($featureflag == "true" || $unitflag == "true") ]] +if [[ $coverageflag == "true" && ($featureflag == "true" || $unitflag == "true" || $apiflag == "true") ]] then echo "Use config file specific.xml" configfile='phpunit.coverage.specific.xml' fi - # backup current config (if it exists): if [ -f $ORIGINALENV ]; then mv $ORIGINALENV $BACKUPENV @@ -110,10 +117,10 @@ cp $DATABASECOPY $DATABASE echo "clear caches and what-not.." php artisan cache:clear -php artisan config:clear -php artisan route:clear +# php artisan config:clear +# php artisan route:clear # php artisan twig:clean -php artisan view:clear +# php artisan view:clear # run PHPUnit if [[ $testflag == "" ]] @@ -125,16 +132,14 @@ else if [[ $coverageflag == "" ]] then echo "Must run PHPUnit without coverage:" - - echo "./vendor/bin/phpunit $verbalflag --configuration $configfile $featuretestclass $unittestclass $testsuite" - ./vendor/bin/phpunit $verbalflag --configuration $configfile $featuretestclass $unittestclass $testsuite else echo "Must run PHPUnit with coverage" - echo "./vendor/bin/phpunit $verbalflag --configuration $configfile $featuretestclass $unittestclass $testsuite" - ./vendor/bin/phpunit $verbalflag --configuration $configfile $featuretestclass $unittestclass $testsuite fi fi +echo "./vendor/bin/phpunit $verbalflag --configuration $configfile $featuretestclass $unittestclass $apitestclass $testsuite" +./vendor/bin/phpunit $verbalflag --configuration $configfile $featuretestclass $unittestclass $apitestclass $testsuite + # restore current config: if [ -f $BACKUPENV ]; then mv $BACKUPENV $ORIGINALENV diff --git a/tests/Api/V1/Controllers/TransactionControllerTest.php b/tests/Api/V1/Controllers/TransactionControllerTest.php index 1378e28d44..5e87865985 100644 --- a/tests/Api/V1/Controllers/TransactionControllerTest.php +++ b/tests/Api/V1/Controllers/TransactionControllerTest.php @@ -2394,7 +2394,7 @@ class TransactionControllerTest extends TestCase ]; do { /** @var TransactionJournal $deposit */ - $deposit = $this->user()->transactionJournals()->where('transaction_type_id', 2)->first(); + $deposit = $this->user()->transactionJournals()->inRandomOrder()->where('transaction_type_id', 2)->first(); $count = $deposit->transactions()->count(); } while ($count !== 2); @@ -2431,7 +2431,7 @@ class TransactionControllerTest extends TestCase ]; do { /** @var TransactionJournal $withdrawal */ - $withdrawal = $this->user()->transactionJournals()->where('transaction_type_id', 1)->first(); + $withdrawal = $this->user()->transactionJournals()->inRandomOrder()->where('transaction_type_id', 1)->first(); $count = $withdrawal->transactions()->count(); } while ($count !== 2); diff --git a/tests/Api/V1/Controllers/UserControllerTest.php b/tests/Api/V1/Controllers/UserControllerTest.php index da0ee82353..1c73ec0582 100644 --- a/tests/Api/V1/Controllers/UserControllerTest.php +++ b/tests/Api/V1/Controllers/UserControllerTest.php @@ -24,6 +24,10 @@ declare(strict_types=1); namespace Tests\Api\V1\Controllers; +use FireflyIII\Repositories\User\UserRepositoryInterface; +use FireflyIII\User; +use Laravel\Passport\Passport; +use Log; use Tests\TestCase; /** @@ -32,4 +36,166 @@ use Tests\TestCase; class UserControllerTest extends TestCase { + /** + * + */ + public function setUp() + { + parent::setUp(); + Passport::actingAs($this->user()); + Log::debug('Now in Api/UserControllerTest.'); + + } + + /** + * Delete a user. + * + * @covers \FireflyIII\Api\V1\Controllers\UserController::__construct + * @covers \FireflyIII\Api\V1\Controllers\UserController::delete + * @covers \FireflyIII\Api\V1\Requests\UserRequest + */ + public function testDelete() + { + // create a user first: + $user = User::create(['email' => 'some@newu' . rand(1, 1000) . 'ser.nl', 'password' => 'hello', 'blocked' => 0]); + + // call API + $response = $this->delete('/api/v1/users/' . $user->id); + $response->assertStatus(204); + $this->assertDatabaseMissing('users', ['id' => $user->id]); + } + + /** + * Delete a user as non admin + * + * @covers \FireflyIII\Api\V1\Controllers\UserController::__construct + * @covers \FireflyIII\Api\V1\Controllers\UserController::delete + * @covers \FireflyIII\Api\V1\Requests\UserRequest + */ + public function testDeleteNoAdmin() + { + Passport::actingAs($this->emptyUser()); + + // create a user first: + $user = User::create(['email' => 'some@newu' . rand(1, 1000) . 'ser.nl', 'password' => 'hello', 'blocked' => 0]); + + // call API + $response = $this->delete('/api/v1/users/' . $user->id); + $response->assertStatus(302); + $this->assertDatabaseHas('users', ['id' => $user->id]); + } + + /** + * @covers \FireflyIII\Api\V1\Controllers\UserController::__construct + * @covers \FireflyIII\Api\V1\Controllers\UserController::index + */ + public function testIndex() + { + // create stuff + $users = factory(User::class, 10)->create(); + // mock stuff: + $repository = $this->mock(UserRepositoryInterface::class); + + // mock calls: + $repository->shouldReceive('all')->withAnyArgs()->andReturn($users)->once(); + + // test API + $response = $this->get('/api/v1/users'); + $response->assertStatus(200); + $response->assertJson(['data' => [],]); + $response->assertJson(['meta' => ['pagination' => ['total' => 10, 'count' => 10, 'per_page' => 50, 'current_page' => 1, 'total_pages' => 1]],]); + $response->assertJson( + ['links' => ['self' => true, 'first' => true, 'last' => true,],] + ); + $response->assertHeader('Content-Type', 'application/vnd.api+json'); + } + + /** + * @covers \FireflyIII\Api\V1\Controllers\UserController::show + */ + public function testShow() + { + $user = User::first(); + + // test API + $response = $this->get('/api/v1/users/' . $user->id); + $response->assertStatus(200); + $response->assertSee($user->email); + } + + /** + * @covers \FireflyIII\Api\V1\Controllers\UserController::store + * @covers \FireflyIII\Api\V1\Requests\UserRequest + */ + public function testStoreBasic() + { + $data = [ + 'email' => 'some_new@user' . rand(1, 1000) . '.com', + 'blocked' => 0, + ]; + + // mock + $userRepos = $this->mock(UserRepositoryInterface::class); + $userRepos->shouldReceive('store')->once()->andReturn($this->user()); + + // test API + $response = $this->post('/api/v1/users', $data); + $response->assertStatus(200); + $response->assertSee($this->user()->email); + } + + /** + * @covers \FireflyIII\Api\V1\Controllers\UserController::store + * @covers \FireflyIII\Api\V1\Requests\UserRequest + */ + public function testStoreNotUnique() + { + $data = [ + 'email' => $this->user()->email, + 'blocked' => 0, + ]; + + // mock + $userRepos = $this->mock(UserRepositoryInterface::class); + + // test API + $response = $this->post('/api/v1/users', $data, ['Accept' => 'application/json']); + $response->assertStatus(422); + $response->assertExactJson( + [ + 'message' => 'The given data was invalid.', + 'errors' => [ + 'email' => [ + 'The email address has already been taken.', + ], + ], + ] + ); + } + + /** + * @covers \FireflyIII\Api\V1\Controllers\UserController::update + * @covers \FireflyIII\Api\V1\Requests\UserRequest + */ + public function testUpdate() + { + // create a user first: + $user = User::create(['email' => 'some@newu' . rand(1, 1000) . 'ser.nl', 'password' => 'hello', 'blocked' => 0]); + + // data: + $data = [ + 'email' => 'some-new@email' . rand(1, 1000) . '.com', + 'blocked' => 0, + ]; + + // mock + $userRepos = $this->mock(UserRepositoryInterface::class); + $userRepos->shouldReceive('update')->once()->andReturn($user); + + // call API + $response = $this->put('/api/v1/users/' . $user->id, $data); + $response->assertStatus(200); + + } + } \ No newline at end of file