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

Refs #18586 -- Split up model_inheritance.ModelInheritanceTest #3658

Conversation

alexanderad
Copy link
Contributor

@timgraham
Copy link
Member

Waiting until #3464 is merged so we can use setUpTestData() instead of setUp().

s = Supplier.objects.create(name='s1', address='a2')
def test_filter_inherited_on_null(self):
"""
Test isnull lookup in filter() for inherited models.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

preferred format is "#12567 - Test isnull lookup for inherited models" (although since the test name is fairly descriptive, I'm not sure the docstring really adds much)

@alexanderad alexanderad force-pushed the ticket_18586_model_inheritance_tests branch from 46fd135 to eab7509 Compare December 3, 2014 19:18
@alexanderad
Copy link
Contributor Author

@timgraham rebased & updated.

@timgraham
Copy link
Member

buildbot, test this please.

@alexanderad
Copy link
Contributor Author

Hmm, not sure what went wrong with build.

@charettes
Copy link
Member

@alexanderad a commit was forced pushed after the build was triggered.

@charettes
Copy link
Member

Buildbot, retest this please.

@timgraham
Copy link
Member

Sorry, one more request I thought of while reviewing some of the other related patches. It would be helpful if only the tests that actually use the test data appear in the test class with setUpTestData(). Could you create a separate test class for the others and move them out?

@alexanderad alexanderad force-pushed the ticket_18586_model_inheritance_tests branch from eab7509 to b83c184 Compare December 3, 2014 21:24
@alexanderad
Copy link
Contributor Author

That make sense. I've updated. I see that previous "build is green" label disappeared with rebased changes, clearly indicating that this needs another test suite run. Neat :-)

@timgraham
Copy link
Member

buildbot, test this please.

@timgraham
Copy link
Member

merged in 2cd19f3, thanks!

@timgraham timgraham closed this Dec 3, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants