-
Notifications
You must be signed in to change notification settings - Fork 11
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
Tidy tests #22
base: master
Are you sure you want to change the base?
Tidy tests #22
Changes from 7 commits
bfeb979
7b173b8
eeddab1
099b266
331b1ab
0f81a07
f052230
70934ff
5fde6e7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,12 @@ | ||
from django.core.paginator import Paginator | ||
import random | ||
|
||
from .arm_wells_support.models import * | ||
from ._utils import (add_n_random_stories_to_well, add_n_random_images_to_well, | ||
generate_random_image, generate_random_story, generate_random_well, | ||
TestCase) | ||
from ._utils import (TestCase, | ||
add_n_random_stories_to_well, | ||
add_n_random_images_to_well, | ||
generate_random_image, | ||
generate_random_story, | ||
generate_random_well) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I prefer the previous style here, but whatever. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto. I've reverted some of these in joncotton#1 to keep it consistent with imports elsewhere. They should really either be one import, one line; or multi-line imports with |
||
|
||
from ..querysets import (GenericForeignKeyQuerySet, MergeQuerySet, | ||
FilterException) | ||
|
@@ -24,10 +26,12 @@ def setUp(self): | |
for i in range(self.number_of_extra_stories): | ||
self.extra_stories.append(generate_random_story()) | ||
|
||
def test_raises_NotImplementedError_on_exclude(self): | ||
def test_raises_NotImplementedError_on_all_and_exclude(self): | ||
gfk_qs = GenericForeignKeyQuerySet(self.well.nodes.all()\ | ||
.select_related()) | ||
with self.assertRaises(NotImplementedError): | ||
gfk_qs.all() | ||
with self.assertRaises(NotImplementedError): | ||
gfk_qs = GenericForeignKeyQuerySet(self.well.nodes.all()\ | ||
.select_related()) | ||
gfk_qs.exclude() | ||
|
||
def test_raises_NotImplementedError_on_misc_functions(self): | ||
|
@@ -50,7 +54,6 @@ def test_raises_AttributeError_on_unknown_attribute(self): | |
with self.assertRaises(AttributeError): | ||
getattr(gfk_qs, "unknown_and_unknowable") | ||
|
||
|
||
def test_gathers_all_nodes_of_one_type_with_two_queries(self): | ||
gfk_qs = GenericForeignKeyQuerySet(self.well.nodes.all()\ | ||
.select_related()) | ||
|
@@ -104,7 +107,7 @@ def test_does_not_ignore_same_ids_in_different_types(self): | |
self.assertEqual(2, len(queryset)) | ||
|
||
def test_non_standard_node(self): | ||
num_nodes = random.randint(3,5) | ||
num_nodes = random.randint(3, 5) | ||
for i in range(num_nodes): | ||
OddNode.objects.create(baz=generate_random_story()) | ||
gfk_qs = GenericForeignKeyQuerySet( | ||
|
@@ -116,14 +119,14 @@ def test_non_standard_node(self): | |
self.assertEqual(obj.__class__, Story) | ||
|
||
def test_non_standard_node_failure(self): | ||
num_nodes = random.randint(3,5) | ||
num_nodes = random.randint(3, 5) | ||
for i in range(num_nodes): | ||
OddNode.objects.create(baz=generate_random_story()) | ||
with self.assertRaises(ValueError): | ||
gfk_qs = GenericForeignKeyQuerySet( | ||
OddNode.objects.all().select_related(), | ||
gfk='bad_field_name' | ||
) | ||
GenericForeignKeyQuerySet( | ||
OddNode.objects.all().select_related(), | ||
gfk='bad_field_name' | ||
) | ||
|
||
def test_works_with_duplicate_nodes(self): | ||
well = generate_random_well() | ||
|
@@ -162,7 +165,8 @@ def test_non_empty_filter(self): | |
|
||
queryset = well.items | ||
with self.assertRaises(FilterException): | ||
self.assertEqual(3, len(queryset.filter(title__in=['foo','bar']))) | ||
self.assertEqual(3, len(queryset.filter(title__in=['foo', 'bar']))) | ||
|
||
|
||
class MergeQuerySetTestCase(TestCase): | ||
def setUp(self): | ||
|
@@ -178,9 +182,11 @@ def setUp(self): | |
generate_random_image() | ||
self.qs_b = Image.objects.all() | ||
|
||
def test_raises_NotImplementedError_on_exclude(self): | ||
def test_raises_NotImplementedError_on_all_and_exclude(self): | ||
merge_qs = MergeQuerySet(self.qs_a, self.qs_b) | ||
with self.assertRaises(NotImplementedError): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you please break these out into multiple test methods? If this |
||
merge_qs.all() | ||
with self.assertRaises(NotImplementedError): | ||
merge_qs = MergeQuerySet(self.qs_a, self.qs_b) | ||
merge_qs.exclude() | ||
|
||
def test_raises_NotImplementedError_on_misc_functions(self): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
django>=1.3 | ||
django-reversion==1.3.3 | ||
django-reversion==1.4 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Better change. Let's nix this file entirely and update |
||
armstrong.hatband |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,2 @@ | ||
-r ./base.txt | ||
armstrong.dev>=1.4 | ||
armstrong.dev>=1.11.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is significant. Importing in Python is broken down into three groups. System level, apps that require installation, and finally any specific to this package. In testing, I try to break them into a fourth -- parts of the system under test.