Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Database Test #104

Closed
wants to merge 7 commits into from
Closed

Refactor Database Test #104

wants to merge 7 commits into from

Conversation

toopay
Copy link
Contributor

@toopay toopay commented May 5, 2013

Since we already have the env configuration, now its pretty easy to follow those schema rather than having to create SqliteTestCase, PostgresTestCase and so on, for each database driver we want to test. DatabaseTestCase would pick-up different configuration based by loaded environment (mysql as default).

After this, anyone could get started to work with #15.

@samdark
Copy link
Member

samdark commented May 5, 2013

What if I have both and want both to be tested in one run? That's actually how I'm testing locally.

@toopay
Copy link
Contributor Author

toopay commented May 5, 2013

What if I have both and want both to be tested in one run?

@samdark With specific ar driver on each test, the only thing we need is to run the test twice (with different config) :)

This PR can avoid huge duplication at the test-side, since we will mainly test the unified layer API, rather than each driver, right? This schema is actually also used by Rails : https://github.com/rails/rails/blob/master/.travis.yml#L10

But the call is yours.

@samdark
Copy link
Member

samdark commented May 5, 2013

Check how I've solved the same issue for cache tests.

@toopay
Copy link
Contributor Author

toopay commented May 5, 2013

I agree that for QueryBuilder and Schema, we could use that route. But for completely duplicate all db classes for different drivers?

I think testing database would still need separate config, because :
1.The test-suite execution will complete faster. As we know, database execution is expensive.
2. It will allow user to test specific driver he/she just need, rather than encourage he/she to set-up all databases that mostly they doesnt use, just to run the entire test-suite. (Yes, i am aware that if they dont set it up, then the test will continue, and marked as incomplete, but is this the right approach? If he/she did not use Postgre driver, and only use the Mysql driver, which is succesfully tested, does it mean the test itself is incomplete for her/him?)

@samdark
Copy link
Member

samdark commented May 5, 2013

Not completely duplicate, just use inheritance...

I think the approach of running as many tests as possible is preferred. That's what tests are for, right?

@toopay
Copy link
Contributor Author

toopay commented May 5, 2013

Yes, running as many test as possible is preferred. But running duplicate test isn't.

@samdark
Copy link
Member

samdark commented May 5, 2013

It's not duplicate. Database backend is different.

@toopay
Copy link
Contributor Author

toopay commented May 5, 2013

Current ActiveRecordTest was extending a specific driver. If we add more test file on ActiveRecord that extends another diferrent driver, then for me, it is duplicating test on ActiveRecord unit itself, which supposed to be the base code that underlying other driver(s).

But, i will wait for some base code to work with, from you or the other, for this one.

@bwoester
Copy link
Contributor

bwoester commented May 5, 2013

Isn't there a possibility to configure the test suite in a way that it executes the the DbTest several times with different configurations? Basically, testing against an interface is a very good idea imo. But if it results in having to run the test suit several times, probably with changing config in between, it's a no-go.

@toopay
Copy link
Contributor Author

toopay commented May 6, 2013

@cebe @qiangxue @mdomba Any thought about this one?

@toopay
Copy link
Contributor Author

toopay commented May 6, 2013

Just for clarification. Having several env and configuration(s), doesn't merely mean that we test specific driver, in our continuous integration environment, on each commit/push. Travis will run each environment/driver simultaneously, like this or that.

@samdark
Copy link
Member

samdark commented May 6, 2013

Travis is fine but we're using it ourselves locally as well and we want to run it once so it will do all the tests in a single run.

@rusmaxim
Copy link
Contributor

rusmaxim commented May 9, 2013

I want to do #15.
@samdark Do you want to use same practice here like in cache tests?

@qiangxue
Copy link
Member

qiangxue commented May 9, 2013

@rusmaxim Could you handle #125 together with #15?

@rusmaxim
Copy link
Contributor

rusmaxim commented May 9, 2013

@qiangxue Yes, i'm looking in that. I want to begin from tests and do it.

At this time I want to properly organize tests and look like here important question.

Now ActiveRecordTest extends \yiiunit\MysqlTestCase and in future looks like all tests of application logic will depends on database. To migrate appliction from database to another we will need to modify all such tests in projects to use another $Database$TestCase. It seems not very good. But run all tests in single run is also important.

@samdark
Copy link
Member

samdark commented May 9, 2013

@rusmaxim yes. I think it's simpler than defining multiple test suites, environments etc. The only thing to look for is an opportunity to decouple tests from Application by creating DB connection yourself.

@rusmaxim
Copy link
Contributor

rusmaxim commented May 9, 2013

@samdark I think most tests with using database in real projects not depends which database using. Do we need DatabaseTestCase anyway, or not?

@samdark
Copy link
Member

samdark commented May 9, 2013

@rusmaxim in the ideal world it's true but we still need to test agaist all these databases.

@rusmaxim
Copy link
Contributor

rusmaxim commented May 9, 2013

@samdark I'm about real application testing, not framework testing. So for example in blog application we will use \yiiunit\MysqlTestCase for tests?

@samdark
Copy link
Member

samdark commented May 9, 2013

We can probably create a DbTestCase if there's enough generic stuff or IDbTestCase interface if not that has a getDbConnection method.

abstract class MyTest extends DbTestCase
{
   ...
}

class MySQLConnectionTest extends MyTest {
   public getDbConnection()
   {
      return // mysql connection
   }
}

This way we have a chance to expand test to more DBs easily and to override specific tests if needed.

@qiangxue
Copy link
Member

qiangxue commented May 9, 2013

Merged PR #202.

@qiangxue qiangxue closed this May 9, 2013
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 4e8dc33 on toopay:patch into * on yiisoft:master*.

cebe pushed a commit that referenced this pull request Nov 22, 2018
Update output-formatting.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants