Improve fixtures and console environment for tests

[Ragazzo]

Changes to be done:

• [x] improve fixtures by removing schema data from it. All database schema now will be up by the migrate command. Cover this in fixtures docs.
• [x] improve console support for unit, functional, acceptance testing, so user can manage all console commands for this env. configs.

[Ragazzo]

Hm, after some thinking i guess we need discussion on this one. @qiangxue question is:

1. For test-case its more likely to create tables, schema, etc. in setUpBeforeClass and then drop all this in tearDownAfterClass, so how to implement this one in current implementation of fixtures?

[Ragazzo]

Ok, after some play-around i got this results in tests time:

• Time: 585 ms, Memory: 13.00Mb
• Time: 745 ms, Memory: 13.00Mb
• Time: 606 ms, Memory: 13.00Mb
• Time: 634 ms, Memory: 12.75Mb
• Time: 636 ms, Memory: 12.75Mb
• Time: 651 ms, Memory: 12.75Mb
• Time: 706 ms, Memory: 12.75Mb
• Time: 732 ms, Memory: 12.75Mb
• Time: 595 ms, Memory: 12.75Mb
• Time: 574 ms, Memory: 12.75Mb
• Time: 760 ms, Memory: 12.75Mb
• Time: 699 ms, Memory: 12.75Mb

Tests cases and fixtures are below:

CREATE TABLE users
(
  id integer NOT NULL DEFAULT nextval('tbl_user_id_seq'::regclass),
  login character(255) NOT NULL,
  CONSTRAINT tbl_user_pkey PRIMARY KEY (id)
)

CREATE TABLE tbl_user_profile
(
  id serial NOT NULL,
  first_name character(255) NOT NULL,
  last_name character(255) NOT NULL,
  user_id integer,
  CONSTRAINT tbl_user_profile_pkey PRIMARY KEY (id)
)

Fixtures:

<?php

namespace tests\unit\fixtures;

use yii\test\ActiveFixture;
use yii\db\Schema;

class UsersFixture extends ActiveFixture
{

    public $modelClass = 'app\models\Users';

    protected function loadSchema()
    {
        $this->createTable('users', [
            'id' => Schema::TYPE_PK,
            'login' => Schema::TYPE_STRING . ' NOT NULL',
        ]);
    }

    public function unload()
    {
        $this->dropTable('users');
    }

}

<?php

namespace tests\unit\fixtures;

use yii\test\ActiveFixture;
use yii\db\Schema;

class UsersProfilesFixture extends ActiveFixture
{

    public $modelClass = 'app\models\UsersProfiles';

    public $depends = [
        'tests\unit\fixtures\UsersFixture',
    ];

    protected function loadSchema()
    {
        $this->createTable('tbl_user_profile', [
            'id' => Schema::TYPE_PK,
            'first_name' => Schema::TYPE_STRING . ' NOT NULL',
            'last_name' => Schema::TYPE_STRING . ' NOT NULL',
            'user_id' => Schema::TYPE_INTEGER . ' NOT NULL',
        ]);
        $this->addForeignKey('users_profiles_users_fk', 'tbl_user_profile', 'user_id', 'users', 'id');
    }

    public function unload()
    {
        $this->dropTable('tbl_user_profile');
    }

}

test-case:

<?php

namespace tests\unit\models;

use yii\codeception\DbTestCase;
use tests\unit\fixtures\UsersFixture;
use tests\unit\fixtures\UsersProfilesFixture;
use app\models\Users;
use app\models\UsersProfiles;

class UserTest extends DbTestCase
{

    use \Codeception\Specify;

    public function testCreateUser()
    {
        $this->specify('create user with profile', function() {
            $user = new Users();
            $user->login = 'some_login';
            $user->save();
            $profile = new UsersProfiles();
            $profile->attributes = [
                'first_name' => 'some_first_name_1',
                'last_name' => 'some_last_name_1',
            ];
            $user->link('profile', $profile);
        });

        $this->specify('find created user with profile', function(){
            $user = Users::find(['login' => 'some_login']);
            $this->assertInstanceOf('app\models\Users', $user);
            $this->assertInstanceOf('app\models\UsersProfiles', $user->profile);
        });
    }

    public function testFindUsers()
    {
        $this->specify('find existing user', function() {
            $user = Users::find(['login' => 'lurline21']);
            $this->assertInstanceOf('app\models\Users', $user);
            $this->assertInstanceOf('app\models\UsersProfiles', $user->profile);
            $this->assertEquals($this->profiles[0]['first_name'], $user->profile->first_name);
            $this->assertEquals($this->profiles[0]['last_name'], $user->profile->last_name);
        });

        $this->specify('find existing user', function() {
            $user = Users::find(['login' => 'ashleigh37']);
            $this->assertInstanceOf('app\models\Users', $user);
            $this->assertInstanceOf('app\models\UsersProfiles', $user->profile);
            $this->assertEquals($this->profiles[1]['first_name'], $user->profile->first_name);
            $this->assertEquals($this->profiles[1]['last_name'], $user->profile->last_name);
        });
    }

    public function fixtures()
    {
        return [
            'users' => UsersFixture::className(),
            'profiles' => UsersProfilesFixture::className(),
        ];
    }

}

[Ragazzo]

So overall as for me its ok, but whats your opinion @qiangxue on this timings? ofcourse the more tables you use the bigger it will be, its like O(n^2) i guess.

[Ragazzo]

Ok, after some long discussion with @samdark i think we should go by the road of migrations (but also should keep loadSchema method). So overall testing instructions would be like:

1. run migrate up --silent=true;
2. run tests, if needed load fixtures;
3. clear fixtures (unload method) and other data if needed;
4. go to step 2.

By this we eliminate step 1 that is more time consuming in testing. Will correct docs about it.

[samdark]

I've applied it in practice without noticeable drawbacks.

[Ragazzo]

@klimov-paul suggestion can be taken as a work-around + DbInitHelper in codeception with calling migrate command in beforeSuite hook, like:

class DbInitHelper extends \Codeception\Module
{

    /**
     * @var boolean indicates if database migrations have been applied.
     */
    protected static $_isDbMigrationApplied = false;

    /**
     * Apply db migrations to the current database.
     * This method allows to keep test database up to date.
     */
    public static function applyDbMigrations()
    {
        $migrateController = new MigrateController('migrate', Yii::$app);
        $migrateController->interactive = false;
        ob_start();
        ob_implicit_flush();
        $migrateController->runAction('up');
        ob_clean();
    }

     public function _beforeSuite()
     {
        if (!static::$_isDbMigrationApplied) {
            static::applyDbMigrations();
            static::$_isDbMigrationApplied = true;
        }
     }

}

@qiangxue should we add this helper in yii2-codeception or just mention it in the docs? Ignore all above comments about creating schema in fixture, we are going this approach as much time saving in testing.

[qiangxue]

So we go back to the "old way"? Why do we still need to keep loadSchema() then?

1. run migrate up --silent=true;
2. run tests, if needed load fixtures;
3. clear fixtures (unload method) and other data if needed;
4. go to step 2.

DbInitHelper proposed above is dealing with Step 1. But I don't see much benefits of having this helper because running the migration is just a console command which can be just part of the test script.

Also Step 3 doesn't seem to be needed or reliable. It's better done when loading a fixture before its data is loaded (i.e. resetTable()).

[Ragazzo]

So we go back to the "old way"?

yes.

Why do we still need to keep loadSchema() then?

to let developer do some schema modifications, we can remove this method, but unfortunatly developers like to forget to call parent method, like in loadData if needed. So i think we can keep this.

DbInitHelper proposed above is dealing with Step 1. But I don't see much benefits of having this helper because running the migration is just a console command which can be just part of the test script.

yes, but currently we cant do yii migrate up for unit-tests becasue we dont have console script like yii for tests. Should we have one?

Also Step 3 doesn't seem to be needed or reliable. It's better done when loading a fixture before its data is loaded (i.e. resetTable())

hm, not sure, anyway we got unload method for this purposes.

[qiangxue]

to let developer do some schema modifications, we can remove this method, but unfortunatly developers like to forget to call parent method, like in loadData if needed. So i think we can keep this.

We can remove the loadSchema and loadData flags?

yes, but currently we cant do yii migrate up for unit-tests becasue we dont have console script like yii for tests. Should we have one?

I see. There are several solutions:

1. In console application's config file, add the DB connections for the test databases. Then we can use yii migrate to upgrade test databases.
2. Like you said, add a yii script for test applications.
3. Like you proposed, add DbInitHelper.

Personally I like the first solution the best.

[Ragazzo]

We can remove the loadSchema and loadData flags?

hm, i would rather keep them than remove.

I see. There are several solutions:

i think we should go step 2 because user may want to use console for tests suites (unit, functional, acceptance) but he cant. In step1 we just cover one situation and not providing good ability for end-user, so we just cant cover all situations like in script. Also adding all db-connections in console not seems to me so right.

[qiangxue]

hm, i would rather keep them than remove.

If there is not strong reason why we need to adjust schema in tests, I think we should remove loadSchema(). Otherwise it's misleading to keep it. Adding is always easier than removing from the point of maintaining a project.

i think we should go step 2

Fine with me.

[Ragazzo]

If there is not strong reason why we need to adjust schema in tests, I think we should remove loadSchema(). Otherwise it's misleading to keep it. Adding is always easier than removing from the point of maintaining a project.

ok, but need to mention in docs that developer should not forget to call parent method or fixtures will not be loaded at all. Will you make this changes into the code? or should i?

Fine with me.

ok, so how to do it correctly? i dont like the idea of simple copy-paste of yii console php file in the basic app, is there some ability to avoid it and just configure path to the config file?

[qiangxue]

I will handle the fixture class changes.

Will see what's the best way for the yii command.

[Ragazzo]

Thanks.

[Ragazzo]

@qiangxue are you done with first part - fixtures? so can i make changes in FixtureController in safe or need to wait?

[qiangxue]

Yes, the first part is done.

[Ragazzo]

@qiangxue i changed issue name and description to make it more logically according last discussion and changes. Also added list of things TBD.

[qiangxue]

Thanks.

[Ragazzo]

@qiangxue i was thinking about console feature here, not sure but maybe we can do the following:

• create under configs folder file env.php with content:

return [
#configs under tests and other env. configs
    'unit' => require(path/to/unit/main/config),
    'functional' => require(path/to/functional/main/config),
    'acceptance' => require(path/to/acceptance/main/config),
];

• small modifications to yii.php file to take into account --env option, or whatever it will be called.

Then can run commands like php yii migrate/apply --env=unit here --env is global option and not migrate controller option, but after all configs will be merged, config from unit will take precedence and migrations will be applied to unit tests database.

Dont like currently this approach, it is just something we can think about or start from.

[qiangxue]

Interesting idea. I was thinking of supporting --config option which takes the path to the main config file. But I like your idea about env, which makes switching config file easier.

[Ragazzo]

Nope, i was thinking to rename --env to --config exactly as you said, to not to confuse users with YII_ENV_TEST and other env. constants. So i prefer --config as additional path to config that should be merged. Maybe some more thinking should be done here.

[qiangxue]

I already implemented this. However before I wanted to check in, I found I couldn't find a convincing use case for this feature.

For the migrate command, an easier solution is to include the declaration of the test databases in the console configuration. Then you can specify the db option when running the command to migrate the specified DB.

Do you have a strong use case that customized app config is needed to run some console command?

[qiangxue]

Never mind. The fixture command is a good example.

[Ragazzo]

I think it will be even better if this option can be an array, so user can join different configs without creating extra files, or it is not needed?

[Ragazzo]

And since almost all commands in Yii use aliases, here it should be used too i think. It is easily to run php yii fixture User --appConfig=@tests/unit/config.php rather then specifying and dealing with all paths.

[qiangxue]

How to specify the option as a multi-dimensional array?

[Ragazzo]

same as it is done now for commands. --appConfig=@tests/unit/config.php,@app/config/some_config.php

[qiangxue]

I don't think we should support this or do merging. Note that merging doesn't allow you to unset/remove an existing configuration entry.

[Ragazzo]

can we support alias then?

[qiangxue]

Done.

[Ragazzo]

Thanks.