Improved database managing for tests #1521

Closed
Ragazzo opened this Issue Dec 14, 2013 · 8 comments

Comments

Projects
None yet
1 participant
@Ragazzo
Contributor

Ragazzo commented Dec 14, 2013

Will duplicate here this comment (#1393 (comment))

  • Implement Transaction strategy for database cleanup (start transaction for connection and then rollback)
  • Implement Reset strategy for database cleanup (tables truncate)
  • Implement Drop/Create strategy for database cleanup (drop tables then create them <->)

I think that we definitely need to improve this to make tdd very native and easy in Yii2.

@Ragazzo

This comment has been minimized.

Show comment
Hide comment
@Ragazzo

Ragazzo Dec 14, 2013

Contributor

My comment:

I also think that we need a better solution for database managing in Yii2, i mean that currently we only have ability to load fixtures in tables, but we dont have tools for creating database structure.
The problem is that in the tests that use fixtures it can take some time to load/unload database dump (especially if there are 20 tables there, not a big number though) and we dont have solutions for this and for creating database structure (and also dont have solution like transaction rollback in tests). Current possible solutions are:
Load all big dump before test suite and then use it, but dont drop tables just reset them (this solution was suggested long time ago by @samdark in chat). Pros and cons: simple implementation, big problems with data collisions especially when have to deal with some triggers and procedures.
Create only needed tables (each table structure represented by separated sql dump file). Pros and cons: not so simple to implement (have one solution though), no troubles with data collisions, triggers, procedures, sequences. So in test case user can select which tables to create by specifying

public $tables = array();

@samdark comment (#1393 (comment))

Personally I always had separate dump for structure only w/o data and either data dump or data fixtures.

Contributor

Ragazzo commented Dec 14, 2013

My comment:

I also think that we need a better solution for database managing in Yii2, i mean that currently we only have ability to load fixtures in tables, but we dont have tools for creating database structure.
The problem is that in the tests that use fixtures it can take some time to load/unload database dump (especially if there are 20 tables there, not a big number though) and we dont have solutions for this and for creating database structure (and also dont have solution like transaction rollback in tests). Current possible solutions are:
Load all big dump before test suite and then use it, but dont drop tables just reset them (this solution was suggested long time ago by @samdark in chat). Pros and cons: simple implementation, big problems with data collisions especially when have to deal with some triggers and procedures.
Create only needed tables (each table structure represented by separated sql dump file). Pros and cons: not so simple to implement (have one solution though), no troubles with data collisions, triggers, procedures, sequences. So in test case user can select which tables to create by specifying

public $tables = array();

@samdark comment (#1393 (comment))

Personally I always had separate dump for structure only w/o data and either data dump or data fixtures.

@Ragazzo

This comment has been minimized.

Show comment
Hide comment
@Ragazzo

Ragazzo Dec 14, 2013

Contributor

Other developers opinions are very welcome :)

Contributor

Ragazzo commented Dec 14, 2013

Other developers opinions are very welcome :)

@Ragazzo

This comment has been minimized.

Show comment
Hide comment
Contributor

Ragazzo commented Dec 15, 2013

@Ragazzo Ragazzo referenced this issue Dec 16, 2013

Closed

Testing, Development and Automation #1543

0 of 4 tasks complete
@Ragazzo

This comment has been minimized.

Show comment
Hide comment
@Ragazzo

Ragazzo Dec 16, 2013

Contributor

About transaction:

@qiangxue i think that possible solution can be like this (just a demo, ofcourse this code should not be in the test-case it will be in yii\codeception\db\Transaction):

# DbTestCase < TestCase
class DbTestCase extends yii\codeception\TestCase
{
      protected $dbConnections = ['db'];

     public static function setUpBeforeClass()
     {
            parent::setUpBeforeClass();
            $this->checkConnections(); //check if they all are available
            $this->initTransactions(); //start transaction per each connection
     }

     public static function tearDownAfterClass()
     {
           $this->rollbackTransactions();
           parent::tearDownAfterClass();
     }

     protected function initTransactions()
     {
         foreach($this->dbConnections as $connection)
               $this->transactions[] = $connection->beginTransaction();
     }

     protected function rollbackTransactions()
     {
         foreach($this->transactions as $transaction)
             $transaction->rollback();
     }

}

Also to avoid commit transactions by developer code we may need to modify beginTransaction (should return same transaction maybe that was started first in the setUpBeforeClass) and Transaction::commit in the way so they will not be doing anything if the YII_ENV is set to "test". This is only a sketch

Contributor

Ragazzo commented Dec 16, 2013

About transaction:

@qiangxue i think that possible solution can be like this (just a demo, ofcourse this code should not be in the test-case it will be in yii\codeception\db\Transaction):

# DbTestCase < TestCase
class DbTestCase extends yii\codeception\TestCase
{
      protected $dbConnections = ['db'];

     public static function setUpBeforeClass()
     {
            parent::setUpBeforeClass();
            $this->checkConnections(); //check if they all are available
            $this->initTransactions(); //start transaction per each connection
     }

     public static function tearDownAfterClass()
     {
           $this->rollbackTransactions();
           parent::tearDownAfterClass();
     }

     protected function initTransactions()
     {
         foreach($this->dbConnections as $connection)
               $this->transactions[] = $connection->beginTransaction();
     }

     protected function rollbackTransactions()
     {
         foreach($this->transactions as $transaction)
             $transaction->rollback();
     }

}

Also to avoid commit transactions by developer code we may need to modify beginTransaction (should return same transaction maybe that was started first in the setUpBeforeClass) and Transaction::commit in the way so they will not be doing anything if the YII_ENV is set to "test". This is only a sketch

@Ragazzo

This comment has been minimized.

Show comment
Hide comment
@Ragazzo

Ragazzo Dec 16, 2013

Contributor

Other solutions like Reset (simple truncate table) and DROP/CREATE are not so hard to implement.

Contributor

Ragazzo commented Dec 16, 2013

Other solutions like Reset (simple truncate table) and DROP/CREATE are not so hard to implement.

@Ragazzo Ragazzo referenced this issue Dec 18, 2013

Merged

Yii2 basic codeception #1393

5 of 5 tasks complete
@Ragazzo

This comment has been minimized.

Show comment
Hide comment
@Ragazzo

Ragazzo Dec 26, 2013

Contributor

@qiangxue what do you think if we change https://github.com/yiisoft/yii2/blob/master/framework/yii/db/Connection.php#L399 and add check if there is already an active transaction, a lot of dbms dont have nested transaction support and handle this only with save-points that is a hack. By adding

                $this->open();
                if (!$this->getTransaction()) {
                     $this->_transaction = new Transaction(['db' => $this]);
                     $this->_transaction->begin();
                }
                return $this->_transaction;

we will achieve:

  1. Removing exceptions because of nesting transactions, and keep DB in good state without some troubles like not commited/aborted transactions
  2. It will also help us in testing because there will be only 1 transaction, it will let us very easy rollback changes.

But there are some notes in implementing transaction strategy for example like (http://www.php.net/manual/en/pdo.begintransaction.php):

Some databases, including MySQL, automatically issue an implicit COMMIT when a database definition language (DDL) statement such as DROP TABLE or CREATE TABLE is issued within a transaction. The implicit COMMIT will prevent you from rolling back any other changes within the transaction boundary.

I think this check is a welcome improvement.

Contributor

Ragazzo commented Dec 26, 2013

@qiangxue what do you think if we change https://github.com/yiisoft/yii2/blob/master/framework/yii/db/Connection.php#L399 and add check if there is already an active transaction, a lot of dbms dont have nested transaction support and handle this only with save-points that is a hack. By adding

                $this->open();
                if (!$this->getTransaction()) {
                     $this->_transaction = new Transaction(['db' => $this]);
                     $this->_transaction->begin();
                }
                return $this->_transaction;

we will achieve:

  1. Removing exceptions because of nesting transactions, and keep DB in good state without some troubles like not commited/aborted transactions
  2. It will also help us in testing because there will be only 1 transaction, it will let us very easy rollback changes.

But there are some notes in implementing transaction strategy for example like (http://www.php.net/manual/en/pdo.begintransaction.php):

Some databases, including MySQL, automatically issue an implicit COMMIT when a database definition language (DDL) statement such as DROP TABLE or CREATE TABLE is issued within a transaction. The implicit COMMIT will prevent you from rolling back any other changes within the transaction boundary.

I think this check is a welcome improvement.

@Ragazzo

This comment has been minimized.

Show comment
Hide comment
@Ragazzo

Ragazzo Dec 26, 2013

Contributor

I also think think that introducing Connection::$transactionClass will help us in testing, because of we can easily reconfigure this to new transaction class that imlements commit differently (simply dont do anything), to avoid commiting. rollback will be the same and will rollback all changes. This feature is more for testing support. Also is good to decouple :)

Contributor

Ragazzo commented Dec 26, 2013

I also think think that introducing Connection::$transactionClass will help us in testing, because of we can easily reconfigure this to new transaction class that imlements commit differently (simply dont do anything), to avoid commiting. rollback will be the same and will rollback all changes. This feature is more for testing support. Also is good to decouple :)

@Ragazzo

This comment has been minimized.

Show comment
Hide comment
@Ragazzo

Ragazzo Jan 19, 2014

Contributor

Closed in order of #1956. Also reasons why it was closed:

  1. This approach dont give such flexibility as #1956 in creating database env.
  2. #1956 lets creating not database env. but and other fixtures that can be need by user (nosql, etc).
  3. Not all databases (for example postgresql) let you truncate table with fk (in this case it should be simple DELETE). So the edge between p 2,3 is not so strict in this case.

If someone will think this issue is needed and some of the reasons are incorrect, please feel free to write here and ask for reopen this one.

Contributor

Ragazzo commented Jan 19, 2014

Closed in order of #1956. Also reasons why it was closed:

  1. This approach dont give such flexibility as #1956 in creating database env.
  2. #1956 lets creating not database env. but and other fixtures that can be need by user (nosql, etc).
  3. Not all databases (for example postgresql) let you truncate table with fk (in this case it should be simple DELETE). So the edge between p 2,3 is not so strict in this case.

If someone will think this issue is needed and some of the reasons are incorrect, please feel free to write here and ask for reopen this one.

@Ragazzo Ragazzo closed this Jan 19, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment