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

Test models should exist in corresponding test case, not in a global models.py. #1777

Closed
xordoquy opened this issue Aug 19, 2014 · 13 comments
Closed
Labels

Comments

@xordoquy
Copy link
Collaborator

Due to a side effect with Django 1.7, some test models have been moved within the models file of the test application.
Following what has been said in #1398 this may not be what we want.

I'm fine with models being defined in the test application models file. This is open to discussion.

@tomchristie
Copy link
Member

"test application models file"

Sorry, I didn't fully understand this. Specifically which test files are we talking about here?
Yes to models being within the test module itself, rather than a seperate models.py module, if possible.

@xordoquy
Copy link
Collaborator Author

By "test application models file" I meant in the models.py that is contained in the test project/app (tests/models.py)
If you really want them separated we will have some renaming to do to preserve unique name.

@tomchristie
Copy link
Member

Happy to approach it incrementally.
Related to this is the oddities of having a tests/serializers.py and other similar modules.

@xordoquy
Copy link
Collaborator Author

This one will be easier to deal with

@tomchristie
Copy link
Member

"This one"

You mean the serializers.py module, or something else?

@tomchristie
Copy link
Member

This doesn't particularly need to block a 2.4 release - removing the milestone.

@tomchristie tomchristie removed this from the 2.4 Release milestone Aug 19, 2014
@tomchristie tomchristie changed the title Push back the test models into the associated test file Test models exist in corresponding test case, not in a global models.py. Aug 19, 2014
@tomchristie tomchristie changed the title Test models exist in corresponding test case, not in a global models.py. Test models should exist in corresponding test case, not in a global models.py. Aug 19, 2014
@maryokhin
Copy link
Contributor

I was a bit curious and took a peek at tests/models.py and kind of broke a leg. In the sense that I'm either not understanding something, or that module is seriously outdated. I found about 10 models that are not used at all, and some models that are used only in *.md files.

For example BlogPost model is only referenced in UniqueValidator section in validators.md, but nowhere in the code. And there's a couple of them like that. Was that the purpose of defining it in models.py or is it a coincidence in naming?

@tomchristie
Copy link
Member

Note it's almost certainly just because there's a bunch of stuff in there that's no longer used.

(Kinda the motivation of this issue to prevent this sort of thing)

Delete those models at will! :)

@maryokhin
Copy link
Contributor

Opened #2161 to start cleaning them up.

@tomchristie
Copy link
Member

V nice!

@tomchristie
Copy link
Member

The relational models in there probably need to stay - tried moving them into seperate test case modules, but the reverse relationships raised conflicts due to duplicated names or something.
Still a few BasicModel cases that can be removed tho, and then we should comment at the top of the module pointing folks towards implementing in the test case module as a preference.

@xordoquy
Copy link
Collaborator Author

xordoquy commented Jun 1, 2015

Was #2161 enough to close this issue or do we still have some related work ?

@tomchristie
Copy link
Member

Mostly resolved. Remaining stuff isn't worth keeping on our todo list anymore.
Perhaps if we reach a static point with very little ongoing feature work then we could consider further test cleanups.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants