Would you be ok with making Prosopite.pause, Prosopite.scan, and Prosopite.resume handle nesting?

[oskarpearson]

Hi @charkost

I've a situation where my specs do something like this:

let(:x) { create(:something) }
it "does that thing" do
  SomeService.new(something).call
end

The create(:something) uses FactoryBot to build a whole series of nested objects and relationships. I've wrapped specs in Prosopite.scan blocks as per https://github.com/charkost/prosopite

In my factories, some parts of the build code perform N+1 queries, and the let clause is triggering them. Since I'm trying to find N+1s in SomeService (not in my FactoryBot Factories) I'd like to ignore them. First question: Do you know of some way to avoid that?

The second problem that I'm encountering is that it doesn't seem that the pause/resume functionality is built with nesting in mind. If I pause at the top level, and then pause/resume in a 'lower' level, the second resume will un-pause the first pause. So if I Prosopite.pause inside a factorybot builder, and that builder uses another factorybot builder that also uses Prosopite.pause/Prosopite.resume, the second Prosopite.resume un-pauses the first one.

Here's a test that shows the behaviour.

  def test_pause_reentrant
    # 20 chairs, 4 legs each
    chairs = create_list(:chair, 20)
    chairs.each { |c| create_list(:leg, 4, chair: c) }

    Prosopite.scan
    # Immediately pause
    Prosopite.pause

    Chair.last(20).each do |c|
      Prosopite.pause
      c.legs.last
      Prosopite.resume
    end

    # This shouldn't trigger N+1s because we paused before the each loop above
    Chair.last(20).each do |c|
      c.legs.last
    end
    Prosopite.resume

    assert_no_n_plus_ones
  end

One way to avoid this would be to change pause and resume to increment a counter, and only resume fully on when the most-outmost resume is set. That'd mean undoing 98bcb2b though. I think there are other ways to deal with that problem though (someone calling resume when scan hadn't been called.

[oskarpearson]

Related: flyerhzm/bullet#435 and flyerhzm/bullet#120