yiisoft / yii2

Yii 2: The Fast, Secure and Professional PHP Framework
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
14.23k stars 6.91k forks source link

migrations and tests #1772

Closed dizews closed 10 years ago

dizews commented 10 years ago

Hello, condeception.yml has a dump section (dump: tests/_data/dump.sql). it will be actual dump of project (which should recreate manually if database was changed). But yii has migrations and I think tests should use migrations instead of using dump.

Form one hand it would be great if we have only one place which to actualize the database. From the other hand tests shouldn't have a tough relations with framework.

what do you think?

cebe commented 10 years ago

:+1:

Ragazzo commented 10 years ago

But yii has migrations and I think tests should use migrations instead of using dump.

i dont think so. Currently i am working on db_cleaner for Yii2 it is a port of ruby db_cleaner gem. Your solution for now is: load dump with Db codeception module (set settings to not to reload dump each time before test), and use truncate command in your test setUp(). Similar to what you have in Yii1 CDbFixtureManager (https://github.com/yiisoft/yii/blob/master/framework/test/CDbFixtureManager.php#L245 https://github.com/yiisoft/yii/blob/master/framework/test/CDbFixtureManager.php#L265).

creocoder commented 10 years ago

But yii has migrations and I think tests should use migrations instead of using dump.

Migrations is a tool for team developement. And are convenient exclusively for that purposes.

dizews commented 10 years ago

@Ragazzo I don't talk about fixtures I talk about structure of database.

Ragazzo commented 10 years ago

you can load your database structure with Db codeception module. FYI, we also will provide test-case for testing migrations.

Ragazzo commented 10 years ago

so if problem is solved for @dizews i think this one can be closed.

cebe commented 10 years ago

@Ragazzo @creocoder so you think a new db dump should be created for testing every time I write a migration? Imo using migrations for creating the DB for testing purpose is the better way. Migrations for structure and fixtures for test data. Otherwise you introduce another file that has to be maintained which means more work and errors.

Ragazzo commented 10 years ago

so you think a new db dump should be created for testing every time I write a migration?

we are not talking about it. It was asked to how load dump to the db. And i also mentioned that: FYI, we also will provide test-case for testing migrations. (answer for your question). Migrations have nothing to deal with db structure for unit-tests. Codeception have Db module, and i've described how to use it, for now while we dont have some db_cleaner.

dizews commented 10 years ago

@Ragazzo, I talk about what @cebe explained.

Ragazzo commented 10 years ago

Ok, to clarify everything, what do you want?

  1. how to load dump to db and clean tables?
  2. how to test migrations?

what is your question is?

dizews commented 10 years ago

@Ragazzo, should we use migrations to create database for testing?

Ragazzo commented 10 years ago

no, there is already Db module in Codeception, and later we add extension db_cleaner.

dizews commented 10 years ago

@Ragazzo how should we support in an actual state of database dump for testing? what is your workflow?

creocoder commented 10 years ago

@dizews Why you suggest use migrations for structure? Can you show any benefits?

Ragazzo commented 10 years ago

@dizews the workflow is to manually update data.sql when you need it. using migrations is not that case.

creocoder commented 10 years ago

@Ragazzo Absoluttely agree.

dizews commented 10 years ago

@creocoder I want to avoid mistiming in structure of databases (app and test)

cebe commented 10 years ago

@dizews the workflow is to manually update data.sql when you need it. using migrations is not that case.

@Ragazzo @creocoder can you give some arguments against using migrations for this?

Ragazzo commented 10 years ago

well i thought it was obvious, i dont see any benefit of using migrations here. the only thing you will do is just over complicate things, thats all. But of course i cant stop from doing so, if you want you can :)

cebe commented 10 years ago

I don't think there is anything complicated here.

now we do something like cat dump.sql | mysql which builds the initial db structure, while when using migrations we do ./yii migrate/up --interactive=0. Using migrations we avoid the step of creating the dump file each time we change the db.

Ragazzo commented 10 years ago

nope, you mixing together application logic that is for migrations and testing. Also when we will have that one db_cleaner that will support truncate, drop/create, delete i dont think your use-case would be valid. And for end-user it is more useful to create database dump that he needs rather then creating special migrations. Database state after migrations could be different to what user wants, thats why you should use old simple data.sql. There are a lot of other arguments can be said but to be true it is useless talk since everybody testing like they want. Just avoid likeihood testing.

Ragazzo commented 10 years ago

@dizews also small note, we added yii2-faker integration, very useful for fixtures, try it out. Maybe you will have some additions or suggestions for it :)

creocoder commented 10 years ago

@cebe I strictly belive migrations is an instrumment for team developement. Applications developement. Using migrations for other purposes have a lot of drawbacks. I see migrations as an some kind of VCS for db. Primitive VCS that make life a little easier when working in team, but making it harder in all other scenarios (e.g. solo developement, testing purposes, etc).

cebe commented 10 years ago

@creocoder I also see migrations as you see them but I do not understand what would be bad about running the migrations that are already there to create the db structure in a test db just like you run them when you deploy your app. Why do we need a dump file for testing? What is worse about using migrations?

qiangxue commented 10 years ago

I think having a dump file to generate the initial database is fine and preferred. Large projects often have complicate DB objects, such as views, triggers, stored procedures, and static lookup table data. While it's still fine to create them using migration, it's just too cumbersome to do so.

Ragazzo commented 10 years ago

yep, @qiangxue is absolutely right.

dizews commented 10 years ago

for example ruby on rails recommends update migrations and copy this database into testing environment. but I see you are against this improvement.

thanks for your attention.

klimov-paul commented 10 years ago

@dizews, I agree with you that migrations should be used to setup and update test database structure. Although Codeception has its own flow to handle it. At this point a valid question is what actually is integrated into what? Does Codeception is integrated into Yii or Yii is integrated in Codeception…

Anyway if framework does not provide some feature you can always bypass it. I use the following approach to reuse migrations for the unit/functional tests:

abstract class TestCaseDb extends TestCase
{
    /**
     * @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 static function setUpBeforeClass()
    {
        if (!static::$_isDbMigrationApplied) {
            static::applyDbMigrations();
            static::$_isDbMigrationApplied = true;
        }
    }
}
dizews commented 10 years ago

@klimov-paul thanks!

dynasource commented 10 years ago

With regards to this discussion, I'm not convinced about not enabling the option to use migrations for setup db structure. I am having the same preference as @dizews, @cebe & @klimov-paul

Of course I thing dump.sql should still be there as an option, but the same should be applied for the solution of @klimov-paul.

From a ambiquity/double work perspective, the use of dump.sql when already using migrations to setup a db, outweigh arguments like:

Ragazzo commented 10 years ago

@dynasource it is old question, migrations usage already was adopted long time ago, see yii2-advanced README.md

dynasource commented 10 years ago

I see. Couldnt find the solution of @klimov-paul implemented. Is there a config option or is running the console required?

Ragazzo commented 10 years ago

running console is required (but only once as you may know, if new migrations were not added), as you can see in yii2-advanced and in yii2-basic in each of tests tier (acceptance, functional, unit) there is an yii file, it is same file as using for console but its config is taken from _console.php in the same folder, so you can easy run migrations to needed application tests tier as usually you do it for main database with console. You also can adjust _console.php if needed

dynasource commented 10 years ago

clear. thanks!

Ragazzo commented 10 years ago

no problem, tests docs guide are still in development so if some moments are unclear feel free to submit PR to docs ;D

dynasource commented 10 years ago

Im still searching for a way to automatically run the migrations before testing (without the shell). Do I have to write the script myself or is it available in core.

Ragazzo commented 10 years ago

no, such functionality not provided by core, since people can load dump not only by migrations, so we cant force them to use only this option

dizews commented 10 years ago

@dynasource I am using InitDbFixture::initScript and initdb.php contain

$migrateController = new \yii\console\controllers\MigrateController('migrate', Yii::$app);
$migrateController->migrationPath = '@console/migrations';
$migrateController->runAction('up', ['interactive' => 0]);

when I run codecept run functional:

it is approach not require to actualize a dump.sql after each change of database schema

dynasource commented 10 years ago

@dizews, thanks for your solution. How & where do you call InitDbFixture::initScript?

dizews commented 10 years ago

@dynasource I am using FixtureHelper::globalFixtures

return [
    [
        'class' => InitDbFixture::className(),
        'initScript' => '@common/tests/fixtures/initdb.php'
    ]
];

read more about fixtures

martingold commented 4 months ago

@klimov-paul Thanks for solution. It messes with codeception test output. You need to discard other output

$discardOutputFilter = new class extends php_user_filter
{
    public string $filtername = 'discard_output';

    /**
     * @param resource $in
     * @param resource $out
     * @param int $consumed
     */
    public function filter($in, $out, &$consumed, bool $closing): int
    {
        while ($bucket = stream_bucket_make_writeable($in)) {
            $bucket->data = '';
            $consumed += $bucket->datalen;
            stream_bucket_append($out, $bucket);
        }
        return PSFS_PASS_ON;
    }
};
stream_filter_register('discard_output', get_class($discardOutputFilter));
$filter = stream_filter_append(STDOUT, 'discard_output');
ob_start();
ob_implicit_flush();

$migrateController = new MigrateController('migrate', Yii::$app);
$migrateController->interactive = false;

$migrateController->runAction('up');

ob_clean();
stream_filter_remove($filter);