Move models to respective test modules

[maryokhin]

What needs to be sorted out:

1. What to do with models that are referenced in multiple test modules. Cloning them into each test module feels bad because of unDRYing the code massively.
2. Same as the 1st, but relating to RESTFrameworkModel, since multiple models use it from all over the place, and the only thing it does is set app_label, maybe remove it completely and set app label specifically on each model them from a common constant TESTS_APP_LABEL='tests' somewhere in settings i.e. conftest.py or utils.py.

P. S. As a side question. why don't asserts in tests set response.data as the message arg i.e. to assertEqual() so that when it fails the developer can see the error message, not just the assertion failure message?

[tomchristie]

1. Explicit probably beats less repetition.
2. Yup, removing it sounds good.
3. Probably just cos I hadn't looked into how you can do that - sounds good.

[tomchristie]

Want to merge this as it currently stands or wait for the other bits too?
Either way okay with me.

[maryokhin]

I will probably push in the other bits too first, plus try to think of approaches on how to solve this with as little repetition as possible.

[xordoquy]

What annoys me the most with having the models outside the models.py file is that I have absolutely no idea of the side effects it could bring, in particular regarding the Django auto discovery.

[maryokhin]

We can try to sit on 2 chairs at once, and have models very basic and stale in models.py and use something like factory_boy and expose factories inside each test module. Where they can dance however they want with the factories. As a positive side effect we get the advantage of fuzzy attributes, batch creation, etc. But that's just me pointing a finger at the sky, not sure how that will solve the actual problem though.

[xordoquy]

Na, if the tests pass it's fine. I just need to go through the Django code to understand about this.

[tomchristie]

Wasn't aware there's any difference with them being in test modules or models.py?

[maryokhin]

why don't asserts in tests set response.data as the message arg i.e. to assertEqual() so that when it fails the developer can see the error message, not just the assertion failure message?

Turned out to be useless with RequestFactory because it doesn't give a pretty error message like Client does.

As far as I know the difference is that if the models are in models.py then you don't need app_label because Django would get it automatically from the enclosing app, in Django 1.7 it would even do that for models that are outside of models.py.

[xordoquy]

As far as I know the difference is that if the models are in models.py then you don't need app_label because Django would get it automatically from the enclosing app

It gets it from the python package (for < 1.7). If you put your models in a models directory, they'll be linked to the application named "models" unless you give them an app_label.

Wasn't aware there's any difference with them being in test modules or models.py?

Django auto discovery expects them to be in the models file.
My feeling is that as long as the models aren't loaded, they aren't "discovered" and thus will not be processed during the setup. By default Django loads the models file to make sure Models declaration have been interpreted and therefore their metaclass has registered them.
I suspect it does work here because py.test does auto discovery,
NB: this is what I think based on what I've already seen and remember about Django. This needs to be confirmed.

[tomchristie]

Defining models only for tests in their test modules is standard usage surely tho. It's always what we used to do with rest framework originally, only more recently that models.py got created. Certainly nothing to do with py.test.

[maryokhin]

Update:

Stupidly moving models to respective test modules doesn't really work out because in Django 1.7 all models go through register_model which checks if it's been registered or not, and if it has, if it's the same model belonging to the same module, first of all, for some reason this checks fails under pytest, and secondly it would still give a warning, so not all that great.

Yes, of course, you can just rename the model, but that would imply names like HypelinkRelationsNullableForeignKeySource which would be soo long that you would even need to set a custom verbose_name as the default one fails because it's > 39 characters.

The only more or less valid idea I have is specifying a custom DjangoRESTFrameworkOptions class that will set the model_name as %s_%s % __module__, <default_model_name>, using a DjangoRESTFrameworkMeta class on DjangoRESTFrameworkModel. Which would basically be the same way that model_name is set by default, but prefix it with a module name. And although model_name is considered to be private stuff, it was so widely abused that it even has a deprecation policy for it's predecessor - https://code.djangoproject.com/ticket/19689.

In any case, this one should be merged, as it's a completely different story now.

[tomchristie]

In any case, this one should be merged, as it's a completely different story now.

Grand, thanks @maryokhin

[xordoquy]

I'm not sure what this issue status is now.

[tomchristie]

Think there's a few minor bits of clean up there that could still be addressed.
The models used by the relationship tests do need to be shared, but anything like BasicModel should still be cleaned up.