have_enqueued_mail with options doesn't match

[mico]

Because of rails/rails@f5050d9
have_enqueued_mail with options is broken
for example, we have a Mailer service:

class MailerTestService
  def save
    MailtestMailer.mailtest(option: { foo: 'bar' }).deliver_later
  end
end

and test

RSpec.describe MailerTestService, type: :service do
  it do
    expect { described_class.new.save }
      .to have_enqueued_mail(MailtestMailer, :mailtest)
      .with(option: { foo: 'bar' })
  end
end

it will not match because arguments serialization in rails/rails@f5050d9#diff-e264d3f0104c3e213b6402459897ee61R144 updated, but not updated yet in rspec-rails

Already working on a solution, will push PR soon.

related #2184

[benoittgt]

Hello @mico

Thanks for the report. If you need any help feel free to ping us.

[mico]

After some research, I found test which already covers the case I described above.

at https://github.com/rspec/rspec-rails/blob/master/spec/rspec/rails/matchers/have_enqueued_mail_spec.rb#L396 you can see:

expect {
  UnifiedMailer.with('foo' => 'bar').test_email.deliver_later
}.to have_enqueued_mail(UnifiedMailer, :test_email).with(
  a_hash_including(params: {'foo' => 'bar'})
)

So matcher in first comment should be rewritten as:

RSpec.describe MailerTestService, type: :service do
  it do
    expect { described_class.new.save }
      .to have_enqueued_mail(MailtestMailer, :mailtest)
      .with(a_hash_including(params: { option: { foo: 'bar' }}))
  end
end

and it would pass then, but the only place where I can see this way of argument matching is this test. No documentation available for this.
Also, arguments matching looks strange for me. why I should use a_hash_including and add "params:" before arguments... maybe we should drop this "params:" or not...
Any ideas?

[JonRowe]

@mico, the params: before means its part of the job arguments for the mailer, to remove it will require special casing the arguments (presumedly checking the relevant version of Rails to define them for), I'm happy to review a PR for doing that, equally happy to review an update to the docs.

[dmitry]

@mico any news on this? After upgrade to 6.0 I have to change everything to verbose and too strict version with params and args, like:

a_hash_including(params: {user: user})

I believe it might be selectable or at least working good with the serializable objects. At the moment as I understood there are no support for the activejob serialization.

[fabn]

I'm doing an upgrade to rails 6 for an existing application and if I switch to the new ActionMailer::MailDeliveryJob class all my email specs will fail. I get ton of errors for non matching arguments.

    config.load_defaults 6.0
    # Without this line all email specs will fail with non matching arguments
    # config.action_mailer.delivery_job = 'ActionMailer::DeliveryJob'

According to documentation this syntax is supposed to work, isn't it?

expect { 
  MyMailer.welcome(user).deliver_later 
}.to have_enqueued_mail(MyMailer, :welcome).with(user)

However to have this working when using rails 6 with its default configuration I need to change that spec in one of the following form to have it working:

expect { 
  MyMailer.welcome(user).deliver_later 
}.to have_enqueued_mail(MyMailer, :welcome).with(args: [user])
# or
expect { 
  MyMailer.welcome(user).deliver_later 
}.to have_enqueued_mail(MyMailer, :welcome).with { |u| expect(u).to eq(user) }

Both of these syntax are pretty annoying and will require to rewrite a lot of specs in existing applications.

Is the first syntax still supported somehow? #2125 states that rails 6 ActionMailer::MailDeliveryJob should be handled but it seems not the case.

Any hint @mico @JonRowe?

[pirj]

@fabn See Jon's comment above. PR is welcome.

[fabn]

I'm very interested in having this fixed, so I tried to sketch a PR, hope it is fine.

Thanks.