Optimise tests and coverage.

This commit is contained in:
James Cole 2018-05-13 09:01:10 +02:00
parent 528da3f08e
commit 1aae84a4d0
No known key found for this signature in database
GPG Key ID: C16961E655E74B5E
13 changed files with 154 additions and 106 deletions

View File

@ -26,9 +26,10 @@ use FireflyIII\Exceptions\FireflyException;
use FireflyIII\Http\Controllers\Controller;
use FireflyIII\Import\Prerequisites\PrerequisitesInterface;
use FireflyIII\Repositories\ImportJob\ImportJobRepositoryInterface;
use FireflyIII\Repositories\User\UserRepositoryInterface;
use Log;
use View;
/**
* Class FileController.
*/
@ -37,6 +38,9 @@ class IndexController extends Controller
/** @var ImportJobRepositoryInterface */
public $repository;
/** @var UserRepositoryInterface */
public $userRepository;
/**
*
*/
@ -48,7 +52,8 @@ class IndexController extends Controller
function ($request, $next) {
app('view')->share('mainTitleIcon', 'fa-archive');
app('view')->share('title', trans('firefly.import_index_title'));
$this->repository = app(ImportJobRepositoryInterface::class);
$this->repository = app(ImportJobRepositoryInterface::class);
$this->userRepository = app(UserRepositoryInterface::class);
return $next($request);
}
@ -66,6 +71,8 @@ class IndexController extends Controller
*/
public function create(string $importProvider)
{
// can only create "fake" for demo user.
if (
!(bool)config('app.debug')
&& !(bool)config(sprintf('import.enabled.%s', $importProvider)) === true
@ -124,34 +131,59 @@ class IndexController extends Controller
*/
public function index()
{
// get all import routines:
/** @var array $config */
$config = config('import.enabled');
$providers = [];
foreach ($config as $name => $enabled) {
if ($enabled || (bool)config('app.debug') || \in_array(config('app.env'), ['demo', 'testing'])) {
$providers[$name] = [];
}
}
// has prereq or config?
foreach (array_keys($providers) as $name) {
$providers[$name]['has_prereq'] = (bool)config('import.has_prereq.' . $name);
$providers[$name]['has_config'] = (bool)config('import.has_config.' . $name);
$class = (string)config('import.prerequisites.' . $name);
$result = false;
if ($class !== '' && class_exists($class)) {
/** @var PrerequisitesInterface $object */
$object = app($class);
$object->setUser(auth()->user());
$result = $object->isComplete();
}
$providers[$name]['prereq_complete'] = $result;
}
$providers = $this->getProviders();
$subTitle = trans('import.index_breadcrumb');
$subTitleIcon = 'fa-home';
return view('import.index', compact('subTitle', 'subTitleIcon', 'providers'));
}
/**
* @return array
*/
private function getProviders(): array
{
// get and filter all import routines:
/** @var array $config */
$providerNames = array_keys(config('import.enabled'));
$providers = [];
$isDemoUser = $this->userRepository->hasRole(auth()->user(), 'demo');
foreach ($providerNames as $providerName) {
Log::debug(sprintf('Now with provider %s', $providerName));
// only consider enabled providers
$enabled = (bool)config(sprintf('import.enabled.%s', $providerName));
$allowedForDemo = (bool)config(sprintf('import.allowed_for_demo.%s', $providerName));
$allowedForUser = (bool)config(sprintf('import.allowed_for_user.%s', $providerName));
if ($enabled === false) {
Log::debug('Provider is not enabled. NEXT!');
continue;
}
if ($isDemoUser === true && $allowedForDemo === false) {
Log::debug('User is demo and this provider is not allowed for demo user. NEXT!');
continue;
}
if ($isDemoUser === false && $allowedForUser === false) {
Log::debug('User is not demo and this provider is not allowed for such users. NEXT!');
continue;
}
$providers[$providerName] = [
'has_prereq' => (bool)config('import.has_prereq.' . $providerName),
'has_config' => (bool)config('import.has_config.' . $providerName),
];
$class = (string)config(sprintf('import.prerequisites.%s', $providerName));
$result = false;
if ($class !== '' && class_exists($class)) {
Log::debug('Will not check prerequisites.');
/** @var PrerequisitesInterface $object */
$object = app($class);
$object->setUser(auth()->user());
$result = $object->isComplete();
}
$providers[$providerName]['prereq_complete'] = $result;
}
return $providers;
}
}

View File

@ -117,12 +117,9 @@ class JobStatusController extends Controller
{
// catch impossible status:
$allowed = ['ready_to_run', 'need_job_config'];
// todo remove error and running.
if (null !== $importJob && !\in_array($importJob->status, $allowed, true)) {
Log::error('Job is not ready.');
// kill the job:
$this->repository->setStatus($importJob, 'error');
return response()->json(['status' => 'NOK', 'message' => 'JobStatusController::start expects status "ready_to_run".']);
@ -138,9 +135,6 @@ class JobStatusController extends Controller
// @codeCoverageIgnoreEnd
}
// set job to be running:
$this->repository->setStatus($importJob, 'running');
/** @var RoutineInterface $routine */
$routine = app($className);
$routine->setImportJob($importJob);

View File

@ -49,7 +49,7 @@ class FakeRoutine implements RoutineInterface
* "ahoy": will log some nonsense and then drop job into status:"need_job_config" to force it back to the job config routine.
* "final": will do some logging, sleep for 10 seconds and then finish. Generates 5 random transactions.
*
* @return bool
* @return void
* @throws FireflyException
*/
public function run(): void
@ -77,7 +77,7 @@ class FakeRoutine implements RoutineInterface
/** @var StageAhoyHandler $handler */
$handler = app(StageAhoyHandler::class);
$handler->run();
$this->repository->setStatus($this->importJob, 'ready_to_run');
$this->repository->setStatus($this->importJob, 'need_job_config');
$this->repository->setStage($this->importJob, 'final');
break;
case 'final':

View File

@ -62,7 +62,7 @@ class FileRoutine implements RoutineInterface
return;
}
throw new FireflyException(sprintf('Import routine cannot handle stage "%s"', $this->importJob->stage)); // @codeCoverageIgnore
throw new FireflyException(sprintf('Import routine cannot handle status "%s"', $this->importJob->status)); // @codeCoverageIgnore
}
/**

View File

@ -40,14 +40,8 @@ class ImportProvider implements BinderInterface
*/
public static function routeBinder(string $value, Route $route): string
{
$providers = (array)config('import.enabled');
$allowed = [];
foreach ($providers as $name => $enabled) {
if ($enabled || (bool)config('app.debug') === true) {
$allowed[] = $name;
}
}
if (\in_array($value, $allowed, true)) {
$providers = array_keys((array)config('import.enabled'));
if (\in_array($value, $providers, true)) {
return $value;
}
throw new NotFoundHttpException;

View File

@ -22,30 +22,45 @@
declare(strict_types=1);
use FireflyIII\Import\Configuration\BunqConfigurator;
use FireflyIII\Import\Configuration\SpectreConfigurator;
use FireflyIII\Import\JobConfiguration\FakeJobConfiguration;
use FireflyIII\Import\JobConfiguration\FileJobConfiguration;
use FireflyIII\Import\Prerequisites\BunqPrerequisites;
use FireflyIII\Import\Prerequisites\FakePrerequisites;
use FireflyIII\Import\Prerequisites\SpectrePrerequisites;
use FireflyIII\Import\Routine\BunqRoutine;
use FireflyIII\Import\Routine\FakeRoutine;
use FireflyIII\Import\Routine\FileRoutine;
use FireflyIII\Import\Routine\SpectreRoutine;
use FireflyIII\Support\Import\Routine\File\CSVProcessor;
return [
'enabled' => [
// these import providers are available:
'enabled' => [
'fake' => true,
'file' => true,
'bunq' => false,
'spectre' => false,
'plaid' => false,
'quovo' => false,
'yodlee' => false,
],
// demo user can use these import providers (when enabled):
'allowed_for_demo' => [
'fake' => true,
'file' => false,
'bunq' => false,
'spectre' => false,
'plaid' => false,
'quovo' => false,
'yodlee' => false,
],
// a normal user user can use these import providers (when enabled):
'allowed_for_user' => [
'fake' => false,
'file' => true,
'bunq' => true,
'spectre' => true,
'plaid' => false,
'quovo' => false,
'yodlee' => false,
'plaid' => true,
'quovo' => true,
'yodlee' => true,
],
'has_prereq' => [
'has_prereq' => [
'fake' => true,
'file' => false,
'bunq' => true,
@ -54,16 +69,16 @@ return [
'quovo' => true,
'yodlee' => true,
],
'prerequisites' => [
'prerequisites' => [
'fake' => FakePrerequisites::class,
'file' => false,
'bunq' => BunqPrerequisites::class,
'spectre' => SpectrePrerequisites::class,
'bunq' => false,
'spectre' => false,
'plaid' => false,
'quovo' => false,
'yodlee' => false,
],
'has_config' => [
'has_config' => [
'fake' => true,
'file' => true,
'bunq' => true,
@ -72,26 +87,27 @@ return [
'quovo' => true,
'yodlee' => true,
],
'configuration' => [
'configuration' => [
'fake' => FakeJobConfiguration::class,
'file' => FileJobConfiguration::class,
'bunq' => BunqConfigurator::class,
'spectre' => SpectreConfigurator::class,
'bunq' => false,
'spectre' => false,
'plaid' => false,
'quovo' => false,
'yodlee' => false,
],
'routine' => [
'routine' => [
'fake' => FakeRoutine::class,
'file' => FileRoutine::class,
'bunq' => BunqRoutine::class,
'spectre' => SpectreRoutine::class,
'bunq' => false,
'spectre' => false,
'plaid' => false,
'quovo' => false,
'yodlee' => false,
],
'options' => [
'options' => [
'fake' => [],
'file' => [
'import_formats' => ['csv'], // mt940
'default_import_format' => 'csv',
@ -100,29 +116,17 @@ return [
],
],
'bunq' => [
'server' => 'sandbox.public.api.bunq.com', // sandbox.public.api.bunq.com - api.bunq.com
'version' => 'v1',
'live' => [
'server' => 'api.bunq.com',
'version' => 'v1',
],
'sandbox' => [
'server' => 'sandbox.public.api.bunq.com', // sandbox.public.api.bunq.com - api.bunq.com
'version' => 'v1',
],
],
'spectre' => [
'server' => 'www.saltedge.com',
],
],
'default_config' => [
'file' => [
'has-config-file' => true,
'auto-start' => false,
],
'bunq' => [
'has-config-file' => false,
'auto-start' => true,
],
'spectre' => [
'has-config-file' => false,
'auto-start' => true,
],
'plaid' => [
'has-config-file' => false,
'auto-start' => true,
],
],
];

View File

@ -31,6 +31,7 @@
</div>
<div class="row">
{#
<div class="col-lg-4 col-md-12 col-sm-12 col-xs-12">
<div class="box box-default">
<div class="box-header with-border">
@ -43,6 +44,7 @@
</div>
</div>
</div>
#}
<div class="col-lg-4 col-md-12 col-sm-12 col-xs-12">
<div class="box box-default">
<div class="box-header with-border">

View File

@ -22,11 +22,10 @@ declare(strict_types=1);
namespace Tests\Feature\Controllers\Import;
use FireflyIII\Import\Prerequisites\BunqPrerequisites;
use FireflyIII\Import\Prerequisites\FakePrerequisites;
use FireflyIII\Import\Prerequisites\SpectrePrerequisites;
use FireflyIII\Models\ImportJob;
use FireflyIII\Repositories\ImportJob\ImportJobRepositoryInterface;
use FireflyIII\Repositories\User\UserRepositoryInterface;
use Log;
use Mockery;
use Tests\TestCase;
@ -56,6 +55,7 @@ class IndexControllerTest extends TestCase
{
// mock stuff:
$repository = $this->mock(ImportJobRepositoryInterface::class);
$userRepository = $this->mock(UserRepositoryInterface::class);
$fakePrerequisites = $this->mock(FakePrerequisites::class);
// fake job:
@ -85,6 +85,7 @@ class IndexControllerTest extends TestCase
// mock stuff:
$repository = $this->mock(ImportJobRepositoryInterface::class);
$fakePrerequisites = $this->mock(FakePrerequisites::class);
$userRepository = $this->mock(UserRepositoryInterface::class);
// fake job:
$importJob = new ImportJob;
@ -105,23 +106,40 @@ class IndexControllerTest extends TestCase
$response->assertRedirect(route('import.job.configuration.index', ['fake_job_2']));
}
/**
* @covers \FireflyIII\Http\Controllers\Import\IndexController
*/
public function testIndex(): void
{
$this->be($this->user());
// fake prerequisites providers:
$fake = $this->mock(FakePrerequisites::class);
$bunq = $this->mock(BunqPrerequisites::class);
$spectre = $this->mock(SpectrePrerequisites::class);
// fake stuff:
$userRepository = $this->mock(UserRepositoryInterface::class);
// call methods:
$userRepository->shouldReceive('hasRole')->withArgs([Mockery::any(), 'demo'])->andReturn(false);
$response = $this->get(route('import.index'));
$response->assertStatus(200);
$response->assertSee('<ol class="breadcrumb">');
}
/**
* @covers \FireflyIII\Http\Controllers\Import\IndexController
*/
public function testIndexDemo(): void
{
$this->be($this->user());
// fake stuff:
$fake = $this->mock(FakePrerequisites::class);
$userRepository = $this->mock(UserRepositoryInterface::class);
// call methods:
$fake->shouldReceive('setUser')->once();
$bunq->shouldReceive('setUser')->once();
$spectre->shouldReceive('setUser')->once();
$fake->shouldReceive('isComplete')->once()->andReturn(true);
$bunq->shouldReceive('isComplete')->once()->andReturn(true);
$spectre->shouldReceive('isComplete')->once()->andReturn(true);
$userRepository->shouldReceive('hasRole')->withArgs([Mockery::any(), 'demo'])->andReturn(true);
$response = $this->get(route('import.index'));

View File

@ -197,7 +197,6 @@ class JobStatusControllerTest extends TestCase
$routine = $this->mock(FakeRoutine::class);
// mock calls:
$repository->shouldReceive('setStatus')->once()->withArgs([Mockery::any(), 'running']);
$routine->shouldReceive('setImportJob')->once();
$routine->shouldReceive('run')->once();
@ -227,7 +226,6 @@ class JobStatusControllerTest extends TestCase
$routine = $this->mock(FakeRoutine::class);
// mock calls:
$repository->shouldReceive('setStatus')->once()->withArgs([Mockery::any(), 'running']);
$repository->shouldReceive('setStatus')->once()->withArgs([Mockery::any(), 'error']);
$routine->shouldReceive('setImportJob')->once();
$routine->shouldReceive('run')->andThrow(new Exception('Unknown exception'));
@ -258,7 +256,6 @@ class JobStatusControllerTest extends TestCase
$routine = $this->mock(FakeRoutine::class);
// mock calls:
$repository->shouldReceive('setStatus')->once()->withArgs([Mockery::any(), 'running']);
$repository->shouldReceive('setStatus')->once()->withArgs([Mockery::any(), 'error']);
$routine->shouldReceive('setImportJob')->once();
$routine->shouldReceive('run')->andThrow(new FireflyException('Unknown exception'));

View File

@ -46,7 +46,7 @@ class FakeRoutineTest extends TestCase
$job = new ImportJob;
$job->user_id = $this->user()->id;
$job->key = 'a_route_' . random_int(1, 1000);
$job->status = 'running';
$job->status = 'ready_to_run';
$job->stage = 'ahoy';
$job->provider = 'fake';
$job->file_type = '';
@ -59,6 +59,7 @@ class FakeRoutineTest extends TestCase
// calls
$repository->shouldReceive('setUser')->once();
$repository->shouldReceive('setStatus')->withArgs([Mockery::any(), 'running'])->once();
$repository->shouldReceive('setStatus')->withArgs([Mockery::any(), 'need_job_config'])->once();
$repository->shouldReceive('setStage')->withArgs([Mockery::any(), 'final'])->once();
$handler->shouldReceive('run')->once();
@ -81,7 +82,7 @@ class FakeRoutineTest extends TestCase
$job = new ImportJob;
$job->user_id = $this->user()->id;
$job->key = 'a_route_' . random_int(1, 1000);
$job->status = 'running';
$job->status = 'ready_to_run';
$job->stage = 'final';
$job->provider = 'fake';
$job->file_type = '';
@ -98,6 +99,7 @@ class FakeRoutineTest extends TestCase
$repository->shouldReceive('setStage')->withArgs([Mockery::any(), 'final'])->once();
$repository->shouldReceive('setTransactions')->withArgs([Mockery::any(), []])->once();
$handler->shouldReceive('getTransactions')->once()->andReturn([]);
$repository->shouldReceive('setStatus')->withArgs([Mockery::any(), 'running'])->once();
$handler->shouldReceive('setImportJob')->once();
$routine = new FakeRoutine;
@ -117,7 +119,7 @@ class FakeRoutineTest extends TestCase
$job = new ImportJob;
$job->user_id = $this->user()->id;
$job->key = 'a_route_' . random_int(1, 1000);
$job->status = 'running';
$job->status = 'ready_to_run';
$job->stage = 'new';
$job->provider = 'fake';
$job->file_type = '';
@ -132,6 +134,7 @@ class FakeRoutineTest extends TestCase
$repository->shouldReceive('setUser')->once();
$repository->shouldReceive('setStage')->withArgs([Mockery::any(), 'ahoy'])->once();
$repository->shouldReceive('setStatus')->withArgs([Mockery::any(), 'ready_to_run'])->once();
$repository->shouldReceive('setStatus')->withArgs([Mockery::any(), 'running'])->once();
$handler->shouldReceive('run')->once();

View File

@ -46,7 +46,7 @@ class FileRoutineTest extends TestCase
$job = new ImportJob;
$job->user_id = $this->user()->id;
$job->key = 'a_fr_' . random_int(1, 1000);
$job->status = 'running';
$job->status = 'ready_to_run';
$job->stage = 'ready_to_run';
$job->provider = 'file';
$job->file_type = '';
@ -59,6 +59,7 @@ class FileRoutineTest extends TestCase
// calls
$repository->shouldReceive('setUser')->once();
$repository->shouldReceive('setStatus')->withArgs([Mockery::any(), 'running'])->once();
$repository->shouldReceive('setStatus')->withArgs([Mockery::any(), 'provider_finished'])->once();
$repository->shouldReceive('setStage')->withArgs([Mockery::any(), 'final'])->once();
$repository->shouldReceive('setTransactions')->withArgs([Mockery::any(), ['a' => 'b']])->once();

View File

@ -128,7 +128,7 @@ class ImportArrayStorageTest extends TestCase
$journalRepos->shouldReceive('store')->once()->andReturn($journal);
$journalRepos->shouldReceive('findByHash')->andReturn(null, $meta)->twice();
$repository->shouldReceive('addErrorMessage')->once()
->withArgs([Mockery::any(), 'Entry #1 ("' . $transactions[1]['description'] . '") could not be imported. It already exists.']);
->withArgs([Mockery::any(), 'Row #1 ("' . $transactions[1]['description'] . '") could not be imported. It already exists.']);
$storage = new ImportArrayStorage;
$storage->setImportJob($job);
@ -374,6 +374,7 @@ class ImportArrayStorageTest extends TestCase
$journalRepos = $this->mock(JournalRepositoryInterface::class);
// mock calls:
$collector->shouldReceive('setUser')->once();
$repository->shouldReceive('setUser')->once();
$repository->shouldReceive('setStatus')->withAnyArgs();
$ruleRepos->shouldReceive('setUser')->once();
@ -449,6 +450,7 @@ class ImportArrayStorageTest extends TestCase
$journalRepos = $this->mock(JournalRepositoryInterface::class);
// mock calls:
$collector->shouldReceive('setUser')->once();
$repository->shouldReceive('setUser')->once();
$repository->shouldReceive('setStatus')->withAnyArgs();
$ruleRepos->shouldReceive('setUser')->once();
@ -460,7 +462,7 @@ class ImportArrayStorageTest extends TestCase
$journalRepos->shouldReceive('store')->twice()->andReturn($journal);
$journalRepos->shouldReceive('findByHash')->andReturn(null)->times(3);
$repository->shouldReceive('addErrorMessage')->withArgs(
[Mockery::any(), 'Entry #2 ("' . $transfer->description . '") could not be imported. Such a transfer already exists.']
[Mockery::any(), 'Row #2 ("' . $transfer->description . '") could not be imported. Such a transfer already exists.']
)->once();

View File

@ -59,6 +59,7 @@ class CSVProcessorTest extends TestCase
$validator = $this->mock(MappedValuesValidator::class);
$validator->shouldReceive('setImportJob')->once();
$validator->shouldReceive('validate')->andReturn([]);
$creator = $this->mock(ImportableCreator::class);