openfoodfoundation / openfoodnetwork

Connect suppliers, distributors and consumers to trade local produce.
https://www.openfoodnetwork.org
GNU Affero General Public License v3.0
1.12k stars 725 forks source link

Add gem for automated regression tests for N+1 queries #12088

Open filipefurtad0 opened 9 months ago

filipefurtad0 commented 9 months ago

What we should change and why (this is tech debt)

There's a gem which could be added to signal N+1 queries:

https://evilmartians.com/chronicles/squash-n-plus-one-queries-early-with-n-plus-one-control-test-matchers-for-ruby-and-rails

Context

This came up here: https://github.com/openfoodfoundation/openfoodnetwork/issues/6731#issuecomment-839639636

Impact and timeline

This could help detect existing and new performance pitfalls :sparkles:

abdellani commented 6 months ago

@filipefurtad0

I read the article and checked the documentation of the gem

I was thinking about how to implement that, do we want to convert some existing tests? or do we need to create new tests?

filipefurtad0 commented 6 months ago

Hey @abdellani ,

do we want to convert some existing tests? or do we need to create new tests?

I don't think we have tests to detect N+1 queries (@openfoodfoundation/core-devs please correct me if I'm wrong). So I think we'd need new tests.

rioug commented 6 months ago

I just had a quick look at it. I think we would be better to add new test, it will allow us to evaluate if the gem is useful or not. For instance, we could pick something where we suspect a N+1 query and start there. If we think the gem is useful, we should probably come up with some guidelines to specify where such test should be used.

cillian commented 5 months ago

One option for testing for N+1 queries is @openfoodfoundation/rspec-sql which was added recently in https://github.com/openfoodfoundation/openfoodnetwork/pull/12221