Opened 8 years ago

Closed 5 years ago

Last modified 4 years ago

#5416 closed (fixed)

Add assertNumQueries() to testing framework

Reported by: adrian Owned by: Alex
Component: Testing framework Version: master
Severity: Keywords: feature_request
Cc: egmanoj@…, alexkoshelev, kmike Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: UI/UX:

Description

The Django testing framework should provide assertNumQueries(), which would assert that a given action executed a given number of database queries. This should work at the view level ("view X uses 3 queries total") or the individual statement level ("model method X uses only 1 query").

This might have to be implemented as two methods -- a "start counting" method and a "stop counting" method:

    def test_something(self):
        self.startCountingQueries()
        do_something_that_should_only_use_two_queries()
        self.assertNumQueries(2)

In this example, startCountingQueries() would reset the counter to 0, and every query would be tallied from that point on. assertNumQueries() would simply assert the query count was the given number.

Note that this depends on #5415, which provides signals for every SQL query.

Attachments (4)

assert_number_of_queries.diff (1.1 KB) - added by Manoj Govindan <egmanoj@…> 6 years ago.
Patch adding assertNumberOfQueries method to TransactionTestCase.
django-assert-num-queries.diff (23.7 KB) - added by Alex 5 years ago.
Implementation still using connection.queries, and application throughout the codebase.
django-assert-num-queries.2.diff (24.9 KB) - added by Alex 5 years ago.
django-assert-num-queries.3.diff (29.8 KB) - added by Alex 5 years ago.

Download all attachments as: .zip

Change History (26)

comment:1 Changed 8 years ago by adrian

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 8 years ago by Simon G. <dev@…>

Malcolm suggests that this wait until after his queryset refactor as it'll be easier then.

comment:3 Changed 8 years ago by Nis Jørgensen <nis@…>

I am wondering whether we would want to provide this. The number of queries being run is an internal part of the framework, not something external code should ever rely on, nor expect to be stable.

comment:4 Changed 8 years ago by ikelly

In some cases, DDL in particular, this may be backend-dependent. For example, creating a model with an AutoField requires one query in PostgreSQL (CREATE TABLE) but three in Oracle (CREATE TABLE, CREATE SEQUENCE, CREATE TRIGGER).

comment:5 Changed 8 years ago by adrian

Nis: While SQL statements are indeed an internal part of the framework, they're also something that developers should be able to test -- particular as we add "lazily loaded" features such as in #5420.

If you don't want to test the number of queries in your own unit tests, you don't have to. :)

comment:6 Changed 8 years ago by PhiR

  • Keywords feature_request added

comment:7 Changed 7 years ago by anonymous

We are now post-QSRF. If the patches get updated, can we fold #5415 and this into the mix?

comment:8 Changed 6 years ago by ericholscher

This seems to be a very fragile test to provide, and changing the number of queries that something takes doesn't mean that it is functionally different. This seems a bit too specific for something to include in Django, especially considering that it is inherently knowing about internals.

comment:9 follow-up: Changed 6 years ago by mtredinnick

@ericholscher: it's actually something that's worth testing. If it increases, you need to understand why. Django shouldn't be (and doesn't) throw around queries just for fun; it's something that's mostly under the developer's control. Agree with Adrian that this is useful to have (we already test this kind of thing a lot now and it's slightly crufty to do so).

comment:10 Changed 6 years ago by kkubasik

  • Owner changed from nobody to kkubasik

comment:11 in reply to: ↑ 9 Changed 6 years ago by Manoj Govindan <egmanoj@…>

Replying to mtredinnick:

(we already test this kind of thing a lot now and it's slightly crufty to do so).


Do you mind sharing how you do this? I am writing some test cases right now and would like to borrow your mechanism if you don't mind.


comment:12 Changed 6 years ago by Manoj Govindan <egmanoj@…>

  • Cc egmanoj@… added

Changed 6 years ago by Manoj Govindan <egmanoj@…>

Patch adding assertNumberOfQueries method to TransactionTestCase.

comment:13 Changed 6 years ago by Manoj Govindan <egmanoj@…>

  • Has patch set

comment:14 Changed 6 years ago by Manoj Govindan <egmanoj@…>

I have added a custom method to TransactionTestCase for asserting that a given callable executed exactly n queries. You can use it thus:

class HomePageTests(django.test.TestCase):
    def test_GET_home_page_uses_exactly_five_queries(self):
        self.assertNumberofQueries(5, self.client.get, reverse('home'))

    def test_POST_home_page_uses_exactly_seven_queries(self):
        self.assertNumberofQueries(7, self.client.post, reverse('home'), data = {'param': 'value'})

Hat tip: Jarret Hardie.

comment:15 Changed 6 years ago by russellm

@Manoj - thanks for the patch, but I'm not sure this the right way to address the problem.

  • Your patch requires enabling settings.DEBUG - an action that can have unintended (and potentially query induing) consequences.
  • Your patch only works when wrapping a single callable. It's not something you could easily deploy in a test case, where you may want to do a query count check at several points in a complex test case. It also precludes (or makes impractical) the testing of a function that has side effects (so it can't be run twice) but also requires other test assertions.

As noted in the ticket comments, the preferred solution is to introduce a signal (vis #5415) and use that signal as the basis for logging the number of queries. This would allow logging regardless of the value of settings.DEBUG. The query count could be cleared at the test setUp; the assertion then validates that the callback has been invoked the right number of times. This would allow for checking the query count without the need to use callable wrapping.

There have been some recent Django-dev discussion about query logging, and the use of signals for this purpose was raised. It may be worth getting involved in this discussion if you're interested in pursuing this idea.

comment:16 Changed 6 years ago by alexkoshelev

  • Cc alexkoshelev added
  • Owner changed from kkubasik to alexkoshelev
  • Status changed from new to assigned

comment:17 Changed 5 years ago by adamnelson

  • Patch needs improvement set

comment:18 Changed 5 years ago by anonymous

  • Cc kmike added

comment:19 Changed 5 years ago by ericholscher

  • milestone set to 1.3

Changed 5 years ago by Alex

Implementation still using connection.queries, and application throughout the codebase.

Changed 5 years ago by Alex

comment:20 Changed 5 years ago by Alex

  • Owner changed from alexkoshelev to Alex
  • Status changed from assigned to new

Changed 5 years ago by Alex

comment:21 Changed 5 years ago by Alex

  • Resolution set to fixed
  • Status changed from new to closed

(In [14183]) Fixed #5416 -- Added TestCase.assertNumQueries, which tests that a given function executes the correct number of queries.

comment:22 Changed 4 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

Note: See TracTickets for help on using tickets.
Back to Top