Added assert_num_queries fixture

[lukaszb]

It provides a fixture with functionality that is given by Django's TestCase class (assertNumQueries). It currently supports only default connection but can be easily extended in future.

[lukaszb]

Not sure why some tests failed. It seems unrelated to my change but I might be wrong - help with investigating appreciated.

[blueyed]

All tests (except for QA) appear to fail.
Try to run the tests locally using tox -e python3.5-3.0.0-1.10-postgres.

[blueyed]

These imports might cause the test failures.
Try importing it in the new fixture.

[lukaszb]

@blueyed thanks for comments, I've applied post review fixes, please check

[blueyed]

Still looks good to me in general.
I think the fixture should be named django_assert_db_queries (especially for the prefix) though.

Then it could later be enhanced to accept an optional arg (list) to match the actual SQL - probably using pytest's fnmatch functionality maybe.

We're using something similar already and it would be nice to have it upstream in pytest-django, and it's always annoying to figure out what the diff of the actual queries is.
Made me remember https://github.com/yourlabs/django-dbdiff, which has a mode where it generates the expected data that you then can copy and paste into your test initially. But this is all a bit out of scope for this PR probably?! :)

[blueyed]

s/of queries/of DB queries/

[blueyed]

better:

Note that it currently only supports the default database.

[blueyed]

@pelme
What do you think?

[lukaszb]

@blueyed thanks for review, I've applied changes. As for comparing queries - I believe it's out of scope of this PR.

[blueyed]

As for comparing queries - I believe it's out of scope of this PR.

Sure. I was just throwing around ideas, hoping for feedback.. :)

[lukaszb]

bump: @pelme @blueyed, any comments?

[blueyed]

I've looked at this last week IIRC, e.g. to support not just the default database etc..

I just wanted to leave a remark that the following should be included probably (optionally?!) to not include ContentType queries:

# Clear ContentType cache, to make this test predictable (when run alone).
ContentType.objects.clear_cache()

What about a flag/kwarg for this, defaulting to that behavior?
With a generic name, in case there are other places that need to be handled for stable query counts.

[lukaszb]

@blueyed I wanted this to be same as what Django provides. Looking their TestCase.assertNumQueries implementation I don't see if they do anything fancy to protect the user from miscalculating those queries. I would rather say it would be unexpected if we would try to do it here.

[blueyed]

Here is a local patch from reviewing this a while ago, please consider applying it (no need to credit me).

 docs/helpers.rst          |  4 ++--
 pytest_django/fixtures.py | 13 +++++++------
 tests/test_fixtures.py    |  6 ++++--
 3 files changed, 13 insertions(+), 10 deletions(-)

diff --git i/docs/helpers.rst w/docs/helpers.rst
index 3985f6e..7948742 100644
--- i/docs/helpers.rst
+++ w/docs/helpers.rst
@@ -221,8 +221,8 @@ Example
 ``assert_num_queries``
 ~~~~~~~~~~~~~~~~~~~~~~

-This fixture allows to check that expected number DB of queries are performed.
-Note that it currently only supports the default database.
+This fixture allows to check for an expected number of DB queries.
+It currently only supports the default database.


 Example
diff --git i/pytest_django/fixtures.py w/pytest_django/fixtures.py
index cbd1323..5be8e7f 100644
--- i/pytest_django/fixtures.py
+++ w/pytest_django/fixtures.py
@@ -336,13 +336,14 @@ def django_assert_num_queries(pytestconfig):
     def _assert_num_queries(num):
         with CaptureQueriesContext(connection) as context:
             yield
-            msg = "Expected to perform %s queries but %s were done" % (num, len(context))
-            if pytestconfig.getoption('verbose') > 0:
-                sqls = (q['sql'] for q in context.captured_queries)
-                msg += '\n\nQueries:\n========\n\n%s' % '\n\n'.join(sqls)
-            else:
-                msg += " (add -v option to show queries)"
             if num != len(context):
+                msg = 'Expected to perform %s queries but %s were done' % (
+                    num, len(context))
+                if pytestconfig.getoption('verbose') > 0:
+                    sqls = (q['sql'] for q in context.captured_queries)
+                    msg += '\n\nQueries:\n========\n\n%s' % '\n\n'.join(sqls)
+                else:
+                    msg += ' (add -v option to show queries)'
                 pytest.fail(msg)

     return _assert_num_queries
diff --git i/tests/test_fixtures.py w/tests/test_fixtures.py
index 331f92a..f06d3d2 100644
--- i/tests/test_fixtures.py
+++ w/tests/test_fixtures.py
@@ -62,7 +62,8 @@ def test_django_assert_num_queries_db(django_assert_num_queries):


 @pytest.mark.django_db(transaction=True)
-def test_django_assert_num_queries_transactional_db(transactional_db, django_assert_num_queries):
+def test_django_assert_num_queries_transactional_db(transactional_db,
+                                                    django_assert_num_queries):
     with transaction.atomic():

         with django_assert_num_queries(3):
@@ -87,7 +88,8 @@ def test_queries(django_assert_num_queries):
                 ContentType.objects.count()
     """)
     result = django_testdir.runpytest_subprocess('--tb=short')
-    result.stdout.fnmatch_lines(['*Expected to perform 1 queries but 2 were done*'])
+    result.stdout.fnmatch_lines([
+        '*Expected to perform 1 queries but 2 were done*'])
     assert result.ret == 1

[lukaszb]

@blueyed applied (apart from line length changes as at setup.cfg flake section 99 chars are allowed so no need to make it shorter imo

[blueyed]

Needs some small update and a rebase.

[blueyed]

s/assert_num_queries/django_assert_num_queries/

[blueyed]

@pelme
What do you think about it?
I'd like to merge it once it's updated, so please provide some feedback.

[pelme]

Looks good to me! It is certainly a very useful feature that I can see myself using!

[blueyed]

Cool.
@lukaszb
Let me know if I should update the PR myself, but then please allow write access for maintainers.

[lukaszb]

@blueyed aligned with master and updated docs. Do you need me to rebase or you'd simply "squash and merge"?

[blueyed]

@lukaszb
I'll squash-merge it once it's green.
Thanks for this nice feature, and sorry for the delay!

[lukaszb]

@blueyed cool, no problem btw, I know how it works ;-)

[merwok]

Hello! Is there a release planned that would include this feature?

[coady]

Have you considered whether this should really be a fixture? It's static; there's no setup or teardown. The _assert_num_queries context manager could just be an imported utility.

[blueyed]

@coady
A valid point, but having it as a fixture allows for using it without importing anything.

[lukaszb]

@coady yeah, I thought about this but using it as a fixture is more pytest-way it seems, as @blueyed already mentioned

[merwok]

Hello again. Can someone say when this will be part of a release?