creatoro / jelly

A flexible ORM for Kohana 3.1+
http://jelly.jonathan-geiger.com
MIT License
72 stars 13 forks source link

Merge tests in main repo #65

Closed creatoro closed 13 years ago

creatoro commented 13 years ago

I don't see the advantages of having the tests in a different repo. It would be easier to maintain having them here.

leth commented 13 years ago

I can see some advantages to not having them in the main branches at least.

Having the test models in the main branch class folder feels quite messy.

creatoro commented 13 years ago

Hmm, might be true, but test models were renamed and they are in the test folder, so I didn't think that this could be an issue.

But your idea seems to be a good one, I'm thinking about creating a master branch anyway, so we could have tests in the develop branches only.

Just FYI: Sprig has its' tests in the main repo.

loonies commented 13 years ago

I have the following structure for my test models

MODPATH
+--classes
+--tests
|  +--classes
|  |  +--model
|  |     +--test
|  |        +-foomodel.php
|  |        +-barmodel.php
|  |        +-...
|  +--jelly
|  |  +--CollectionTest.php
|  |  +--CoreTest.php
|  |  +--...
|  +--test_data
...

plus I attach my custom unittest autoloader from the test bootstrap.

Maybe, we can organize a directory structure something like that.

leth commented 13 years ago

Having tests only in the develop branch would mean commits couldn't touch both the tests dir and the classes dir, otherwise merging into main would be a pain.

It might be better to keep them in both branches and structure as @loonies said.

leth commented 13 years ago

Actually, it might be worth looking how it's done in this module: https://github.com/cbandy/real-database

creatoro commented 13 years ago

To summarize...

Sprig and AutoModeler also has it's tests in the main model folder, and I don't see a problem with this either. Real Database doesn't have models, so I'm not sure what was interesting about that, @leth.

@loonies: the only solution to "hide" the models from the main folders would be yours, however I'm not familiar with that solution. How do you set up your bootstrap and run tests for models that are no in the default folder? Can you create a gist to show it?

Atm I'm leaning towards to including the tests just like Spig and AutoModeler.

leth commented 13 years ago

I was mostly thinking about how schemas and test data might be better dealt with. Currently I only bother to maintain test data dumps for sqlite for the stuff I've been adding, because I'm a bit lazy :P

It'd be nice if we could cut out the duplication of the test data. Separate schema files might still be needed, but there's no need to maintain the test data in 3 places.

Sprig has SQL data and schema inside it's one test file. Automodeller has an xml-based data dum, but I'd imagine handles the schema internally. RealDB has a whole buch of stuff, I haven't had the time to work out what exactly it's doing though.

leth commented 13 years ago

Looking at how the kohana unittest module works, it:

The trail starts at Kohana_Unittest_Tests::suite().

This looks like it might be easier than anticipated :)

creatoro commented 13 years ago

@leth: so your primary concern is not having the models in the default model folder, you're talking about the way the tests are made. In this case I agree, the tests need to be polished and restructured. However, the first step will be the merger.

loonies commented 13 years ago

https://gist.github.com/1143654

Note that this is an old legacy application written with K 3.0. I'm using custom bootstrap.test.php and custom autoloader.

Anyway, I think @leth has found the solution :) we should create test suits and blacklist classes that we don't want in tests.

creatoro commented 13 years ago

@loonies: I might be missing something here... What should we blacklist?

I was thinking about dealing with setting up the test environment and I think we should create a custom testcase for Jelly, so this way we can have the db schemas in their own .sql schema file and the data to be inserted in an .xml file. And all we need for the test setup is this:

https://gist.github.com/1144751

loonies commented 13 years ago

hehe.. I was thinking about models under MODPATH/classes/models. I spoke to soon, sorry my bad :)

However, we have to have test models, no?

creatoro commented 13 years ago

We have to have test models, of course. What's the point in having the folder you mentioned blacklisted?

loonies commented 13 years ago

no point, as I said, I spoke to soon without thinking :)

so, we can organize models as suggested in the comment above

creatoro commented 13 years ago

It's done, thanks for the help guys! Can you confirm it's working (don't forget to read the guide)?

As @leth pointed out Kohana's list_files() method was the key to "hide" the test files from the main folders. It adds the files from tests/classes before it adds the files from tests/jelly (because of alphabetical ordering) which means all the required files are added before they are needed.

Thanks to this we can have the file structure @loonies suggested.

Running the tests is slower then before, because the environment setup is run even if the tests are in the same group, but this might be sorted out later... However, I don't think this is a big issue.

3.2 branch is closer then ever :).

leth commented 13 years ago

I'm a bit confused about database types.

Is the only way to test it with sqlite to use PDO (setting the connection type to 'pdo', and having sqlite in the dsn)?

leth commented 13 years ago

Ok I've worked out what's broken.

You can't use the dsn sqlite:memory: because the test suite opens multiple connections and gets a new database each time.

leth commented 13 years ago

I've fixed it, commit shortly :)

Edit: see #73