Flaky behavior in Minitest with callbacks and ActiveJob

[evdevdev]

Before anything else, I want to say thank you for this gem. It is awesome and very helpful in our code base.

Currently, I'm running into some flaky test behavior that I have narrowed down to this gem.

Use case: We have an ActiveRecord class for which do a 1:1 sync to an external service. Whenever we destroy a record, we want to enqueue a job to remove it from an external service.

class ExampleRecord < ApplicationRecord 
  after_destroy :sync_destroy_to_external_service

  protected

    def sync_destroy_to_external_service
       ExternalServiceDeleteJob.perform_when_transaction_commits("some-key")
    end
end

To enable this method, we have the following:

class ExternalServiceDeleteJob < ActiveJob::Base
  extend AfterCommitEverywhere

  def self.perform_when_transaction_commits(*args)
    return perform_later(*args) unless in_transaction?

    after_commit { perform_later(*args) }
  end
end

We test this behavior in a really simple way:

test "enqueues destroy job on destruction" do
  example_record = example_records(:some_fixture)
  example_record.destroy!
  assert_enqueued_with(job: ExternalServiceDeleteJob)
end

Nine out of ten times, this spec passes. However, on the random outlier, we get the following:

No enqueued job found with {:job=>ExternalServiceDeleteJob}

How I am really scratching my head. I suspect this has something to do with how rails test manages transaction.

Have you ever seen something like this before? Any idea on what might be causing it?

Thank you in advance for any guidance! And thank you again for this gem. Happy to contribute to a fix in anyway.

[Envek]

That is super weird! Have never heard about such nasty behavior.

It would be awesome if you could make reproducible test case, e.g. as a gist using ActiveRecord's bug report template: https://github.com/rails/rails/blob/main/guides/bug_report_templates/active_record_gem.rb

[Envek]

Also, your code can be simplified.

1. in_transaction? check is already built-in into this gem's after_commit, so you can remove this check:

   def self.perform_when_transaction_commits(*args)
   -  return perform_later(*args) unless in_transaction?
   -
     after_commit { perform_later(*args) }
   end

2. Also, given that you're enqueueing job from model, you can use after_commit that is built-in to ActiveRecord:

   class ExampleRecord < ApplicationRecord 
   -  after_destroy :sync_destroy_to_external_service
   +  after_commit :sync_destroy_to_external_service, on: :destroy
   
     protected
   
     def sync_destroy_to_external_service
   -     ExternalServiceDeleteJob.perform_when_transaction_commits("some-key")
   +     ExternalServiceDeleteJob.perform_later("some-key")
     end
   end

   See https://guides.rubyonrails.org/active_record_callbacks.html#transaction-callbacks

Maybe any of this will eliminate your test flakiness to go away

[evdevdev]

@Envek Thanks for the feedback. I've made those changes and I'll closely watch our CI/CD for the next few days and report back.