zooniverse / talk-api

Apache License 2.0
6 stars 0 forks source link

rails-5.0-comment_spec #332

Closed Tooyosi closed 2 months ago

Tooyosi commented 2 months ago

Issue is caused by different ways of triggering the after_commit hook in rails 4 and 5 In rails 4, running :run_callbacks would trigger the after_commit hook. In rails 5 however, the after_commit is only triggered after the database transaction is completed and would not respond to :run_callbacks.

Relying on building a comment and saving to trigger the callback

yuenmichelle1 commented 2 months ago

See: https://stackoverflow.com/a/30901628/15768801 The change in behavior from Rails 4 to Rails 5 is due to tests running within transactional fixtures in Rails versions < 5. This behavior gets fixed in Rails 5+.

This is a pretty good candidate for version checks. I like what you have so far. We can add version checks for the specs that use trigger :commit via run_callbacks. One thing I do suggest is that you preface with a comment with a TODO so that once we are on Rails 5, we can ctrl+f all our TODOs and remove them.

eg. comment_spec.rb L556 can look like

it 'should queue the notification' do
      # TODO: Once on Rails 5, Can Remove this Version Check
      # In Rails Versions < 5, commit callbacks are not getting called in transactional tests.
      # See https://stackoverflow.com/a/30901628/15768801 for more details.
      if Rails.version.starts_with?('4.2')
        expect(CommentSubscriptionWorker).to receive(:perform_async).with comment.id
        comment.run_callbacks :commit
      else
        allow(CommentSubscriptionWorker).to receive(:perform_async)
        expect(CommentSubscriptionWorker).to have_received(:perform_async).with comment.id
      end
end