Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fixed #18927 - Fix bulk_create tests when not has_bulk_insert #350

Closed
wants to merge 2 commits into from

3 participants

@manfre
Collaborator

Skipped some tests when not has_bulk_insert and others only skip the asserts that depend on the explicit number of queries.

@alex
Collaborator

This is not the correct fix, all tests should be split into two tests, one which checks efficiency, and the other which checks correctness. The efficiency version should be only run on databases which support bulk insert.

@manfre
Collaborator

What is the benefit of doing this split instead of skipping invalid inserts? Wouldn't it be better to not duplicate most of the code and time for some tests only to check another assert for some databases?

@alex
Collaborator

The benefit is that we have two tests for two different things which can then fail independently, and give us more information for when they break, which is the goal of a test suite.

@manfre
Collaborator

Updated the pull request to split the efficiency tests.

@akaariai
Collaborator

Committed manually.

@akaariai akaariai closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 43 additions and 3 deletions.
  1. +43 −3 tests/regressiontests/bulk_create/tests.py
View
46 tests/regressiontests/bulk_create/tests.py
@@ -3,7 +3,7 @@
from operator import attrgetter
from django.db import connection
-from django.test import TestCase, skipIfDBFeature
+from django.test import TestCase, skipIfDBFeature, skipUnlessDBFeature
from django.test.utils import override_settings
from .models import Country, Restaurant, Pizzeria, State, TwoFields
@@ -29,6 +29,7 @@ def test_simple(self):
self.assertEqual(created, [])
self.assertEqual(Country.objects.count(), 4)
+ @skipUnlessDBFeature('has_bulk_insert')
def test_efficiency(self):
with self.assertNumQueries(1):
Country.objects.bulk_create(self.data)
@@ -50,6 +51,16 @@ def test_inheritance(self):
], attrgetter("name"))
def test_non_auto_increment_pk(self):
+ State.objects.bulk_create([
+ State(two_letter_code=s)
+ for s in ["IL", "NY", "CA", "ME"]
+ ])
+ self.assertQuerysetEqual(State.objects.order_by("two_letter_code"), [
+ "CA", "IL", "ME", "NY",
+ ], attrgetter("two_letter_code"))
+
+ @skipUnlessDBFeature('has_bulk_insert')
+ def test_non_auto_increment_pk_efficiency(self):
with self.assertNumQueries(1):
State.objects.bulk_create([
State(two_letter_code=s)
@@ -77,13 +88,21 @@ def test_large_batch(self):
TwoFields.objects.bulk_create([
TwoFields(f1=i, f2=i+1) for i in range(0, 1001)
])
- self.assertTrue(len(connection.queries) < 10)
self.assertEqual(TwoFields.objects.count(), 1001)
self.assertEqual(
TwoFields.objects.filter(f1__gte=450, f1__lte=550).count(),
101)
self.assertEqual(TwoFields.objects.filter(f2__gte=901).count(), 101)
+ @skipUnlessDBFeature('has_bulk_insert')
+ def test_large_batch_efficiency(self):
+ with override_settings(DEBUG=True):
+ connection.queries = []
+ TwoFields.objects.bulk_create([
+ TwoFields(f1=i, f2=i+1) for i in range(0, 1001)
+ ])
+ self.assertTrue(len(connection.queries) < 10)
+
def test_large_batch_mixed(self):
"""
Test inserting a large batch with objects having primary key set
@@ -94,7 +113,6 @@ def test_large_batch_mixed(self):
TwoFields.objects.bulk_create([
TwoFields(id=i if i % 2 == 0 else None, f1=i, f2=i+1)
for i in range(100000, 101000)])
- self.assertTrue(len(connection.queries) < 10)
self.assertEqual(TwoFields.objects.count(), 1000)
# We can't assume much about the ID's created, except that the above
# created IDs must exist.
@@ -102,7 +120,29 @@ def test_large_batch_mixed(self):
self.assertEqual(TwoFields.objects.filter(id__in=id_range).count(), 500)
self.assertEqual(TwoFields.objects.exclude(id__in=id_range).count(), 500)
+ @skipUnlessDBFeature('has_bulk_insert')
+ def test_large_batch_mixed_efficiency(self):
+ """
+ Test inserting a large batch with objects having primary key set
+ mixed together with objects without PK set.
+ """
+ with override_settings(DEBUG=True):
+ connection.queries = []
+ TwoFields.objects.bulk_create([
+ TwoFields(id=i if i % 2 == 0 else None, f1=i, f2=i+1)
+ for i in range(100000, 101000)])
+ self.assertTrue(len(connection.queries) < 10)
+
def test_explicit_batch_size(self):
+ objs = [TwoFields(f1=i, f2=i) for i in range(0, 4)]
+ TwoFields.objects.bulk_create(objs, 2)
+ self.assertEqual(TwoFields.objects.count(), len(objs))
+ TwoFields.objects.all().delete()
+ TwoFields.objects.bulk_create(objs, len(objs))
+ self.assertEqual(TwoFields.objects.count(), len(objs))
+
+ @skipUnlessDBFeature('has_bulk_insert')
+ def test_explicit_batch_size_efficiency(self):
objs = [TwoFields(f1=i, f2=i) for i in range(0, 100)]
with self.assertNumQueries(2):
TwoFields.objects.bulk_create(objs, 50)
Something went wrong with that request. Please try again.