search

opensafely-core / job-server

A server for mediating jobs that can be run in an OpenSAFELY secure environment. q.v. job-runner
https://jobs.opensafely.org
Other
5 stars 10 forks source link

Track number of queries in tests #3347

Open ghickman opened 11 months ago

ghickman commented 11 months ago

We're starting to see a few performance issues recently, and particularly with the removal of an implicit query (#3323), and the removal of orgs from urls (#3312).

It would be useful to have some query counts in our tests, but it's not clear how many pages we should do this for. Lets err on the side of adding them to the success test case for each page to start with.

The homepage has various objects in it so it's an obvious first target to try this out.

Templates:

ghickman commented 11 months ago

This is complicated by us doing several queries in our templates.

To track both we have to wrap both the view call and rendered_template in the count call:

with django_assert_num_queries(n):
    response = MyView.as_view()(request)

…

with django_assert_num_queries(m):
    response.rendered_content

This is a good time to try out methods for ensuring we don't pass rich objects to the template. Maybe a method on a model that produces a dict? (or a dataclass/attr's class?)

lucyb commented 10 months ago

Could we narrow down the scope of this ticket to just one of these ideas, please? Perhaps focusing on adding a test for the homepage for now or for one of the pages that Tom O is about to change?

This change could then be used as a pattern for applying to future tests when we next come to make changes to any views/templates.

ghickman commented 10 months ago

There's only two ideas here, count queries and push all queries to the view to avoid paying the cost of template rendering in tests. The work for this ticket is really working out a method or tools that we like for doing these and then applying them to each view, using the list above to track progress.