GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
Already on GitHub? Sign in to your account
This intends to fix the first test case reported in Trac ticket 18586 as a "too long" unit test to be separated into several, more specific ones.
In particular, this PR splits the old modeltests.generic_relations.GenericRelationsTests.test_generic_relations unit test into several different unit tests, each one related to a single feature of Django generic relations. It also adds a setUp method to the TestCase to define the common attributes.
It seems like some lines were deleted like this stuff... was it intentional?
Yeah, some delete() lines were removed because of the new layout, which creates the objects in the setUp method. I can make a further review of my fixes later today, but roughly I think it should be okay.
I think this comment can go
It was there before, so I left it, but feel free to remove it anyway.
move import to top of file
Agree. Sorry for missing that...
Thanks. After you give it another review, please go ahead and squash the commits and follow our commit message guidelines (e.g. "Split GenericRelationsTests.test_generic_relations into several tests; refs #18586."). Thanks!
Done. Reviewed the issue and applied the changes you suggested (remove old comment and move import from the method to the global imports in the top of the file).
Also squashed the 4 commits into only one and edited the commit message accordingly.
still seems like we're missing this type of query: filter(tags__content_type=ctype)
Split GenericRelationsTests.test_generic_relations into several tests…
…; refs #18586.
Refactored the first big unit test of GenericRelationsTests test case
into several, more granular unit tests checking just one feature.
This is the missing test case that @timgraham noticed I've forgot to add.
Added that missing test. Thanks!
merged in 7b064e5.