search

charkost / prosopite

:mag: Rails N+1 queries auto-detection with zero false positives / false negatives
Apache License 2.0
1.54k stars 46 forks source link

N+1 Errors raised in RSpec test without associations #22

Closed typhoon2099 closed 2 years ago

typhoon2099 commented 3 years ago

I have a controller that loads two records of the same model, does some work on both of them, then returns an OK. The test for this is failing because Prosopite thinks that the two load queries are an N+1, when in fact they're just the same query being run twice.

I don't see a simple way to prevent this.

typhoon2099 commented 3 years ago

Ah, now that I've read the README properly it seems like this is intended, and indeed makes sense. I was hoping to replace Bullet with Prosopite, but it doesn't cover everything that Bullet does (eager loading, for example).

Maybe there's a way to run alongside Bullet (disable N+1 checking in Bullet), if I find something neat I'll share my findings.

lsantobuono commented 2 years ago

Sorry to bring this up, but why exactly does this make sense? What should I do if a require the creation of many of the same objects in one spec? I know I could just not raise or ignore the cases by adding strings to the ignore list, but I'd like to have this on to find other n+1 queries.

typhoon2099 commented 2 years ago

I think the better thing to do, rather than wrapping the spec with Prosopite, is to wrap the part of the spec which isn't setup with Prosipite. This might not work if you're making use of let (since they're lazy instantiated), but reducing the amount of setup inside the Prosopite block is the way to do it.

My specific problem was due to a controller loading two records, which triggers an N+1. If the spec is triggering the N+1 then the best bet would be to not check with Prosopite until after that setup has happened. Most likely you'll want specific tests for this behaviour rather than trying to check it on every spec like I was attempting.