Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ActionMailbox spec helpers and test type #2119

Merged
merged 2 commits into from May 9, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 14 additions & 0 deletions lib/generators/rspec/mailbox/mailbox_generator.rb
@@ -0,0 +1,14 @@
require 'generators/rspec'

module Rspec
module Generators
# @private
class MailboxGenerator < Base
def create_mailbox_spec
template('mailbox_spec.rb.erb',
File.join('spec/mailboxes', class_path, "#{file_name}_mailbox_spec.rb")
)
end
end
end
end
7 changes: 7 additions & 0 deletions lib/generators/rspec/mailbox/templates/mailbox_spec.rb.erb
@@ -0,0 +1,7 @@
require 'rails_helper'

<% module_namespacing do -%>
RSpec.describe <%= class_name %>Mailbox, <%= type_metatag(:mailbox) %> do
pending "add some examples to (or delete) #{__FILE__}"
end
<% end -%>
7 changes: 6 additions & 1 deletion lib/rspec/rails/configuration.rb
Expand Up @@ -34,7 +34,8 @@ class Configuration
:routing => %w[spec routing],
:view => %w[spec views],
:feature => %w[spec features],
:system => %w[spec system]
:system => %w[spec system],
:mailbox => %w[spec mailboxes]
}

# Sets up the different example group modules for the different spec types
Expand Down Expand Up @@ -140,6 +141,10 @@ def filter_rails_from_backtrace!
if defined?(ActiveJob)
config.include RSpec::Rails::JobExampleGroup, :type => :job
end

if defined?(ActionMailbox)
config.include RSpec::Rails::MailboxExampleGroup, :type => :mailbox
end
end
# rubocop:enable Style/MethodLength

Expand Down
1 change: 1 addition & 0 deletions lib/rspec/rails/example.rb
Expand Up @@ -9,3 +9,4 @@
require 'rspec/rails/example/job_example_group'
require 'rspec/rails/example/feature_example_group'
require 'rspec/rails/example/system_example_group'
require 'rspec/rails/example/mailbox_example_group'
74 changes: 74 additions & 0 deletions lib/rspec/rails/example/mailbox_example_group.rb
@@ -0,0 +1,74 @@
module RSpec
module Rails
# @api public
# Container module for mailbox spec functionality.
module MailboxExampleGroup
extend ActiveSupport::Concern

if RSpec::Rails::FeatureCheck.has_action_mailbox?
require 'action_mailbox/test_helper'
extend ::ActionMailbox::TestHelper
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't love this, but ActionMailbox appears to expect to operate on a particular named ActionMailbox::InboundEmail < ActiveRecord::Base, and I'd be worried that using some kind of double here might be brittle.

Copy link
Member

@benoittgt benoittgt May 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with you but why don't you like it?

Copy link
Contributor

@jamesdabbs jamesdabbs May 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is here mostly because ActionMailbox::InboundEmail.create_and_extract_message_id! is doing a lot of work that I didn't want to duplicate here, but this feels like I'm importing implementation details, not clean abstractions.

My inclination here is to instead do something like

  • define a lightweight e.g. RSpec::Rails::ActionMailbox::MockInboundEmail that we use internally in process_inbound_email
  • add shared examples for ::ActionMailbox::InboundEmail and MockInboundEmail to make sure they are interchangeable
  • consider using MockInboundEmail for the process helper (either by default or opt-in somehow?)
  • work on landing MockInboundEmail upstream

How does that sound to y'all?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love the idea of a MockInboundEmail but I think it can be another PR?


def self.create_inbound_email(arg)
case arg
when Hash
create_inbound_email_from_mail(arg)
else
create_inbound_email_from_source(arg.to_s)
end
end
else
def self.create_inbound_email(_arg)
raise "Could not load ActionMailer::TestHelper"
end
end

class_methods do
# @private
def mailbox_class
described_class
end
end

included do
subject { described_class }
end

# Verify the status of any inbound email
#
# @example
# describe ForwardsMailbox do
# it "can describe what happened to the inbound email" do
# mail = process(args)
#
# # can use any of:
# expect(mail).to have_been_delivered
# expect(mail).to have_bounced
# expect(mail).to have_failed
# end
# end
def have_been_delivered
satisfy('have been delivered', &:delivered?)
end

def have_bounced
satisfy('have bounced', &:bounced?)
end

def have_failed
satisfy('have failed', &:failed?)
end

# Process an inbound email message directly, bypassing routing.
#
# @param message [Hash, Mail::Message] a mail message or hash of
# attributes used to build one
# @return [ActionMaibox::InboundMessage]
def process(message)
MailboxExampleGroup.create_inbound_email(message).tap do |mail|
self.class.mailbox_class.receive(mail)
end
end
end
end
end
4 changes: 4 additions & 0 deletions lib/rspec/rails/feature_check.rb
Expand Up @@ -38,6 +38,10 @@ def has_action_mailer_show_preview?
::ActionMailer::Base.respond_to?(:show_previews=)
end

def has_action_mailbox?
defined?(::ActionMailbox)
end

def has_1_9_hash_syntax?
::Rails::VERSION::STRING > '4.0'
end
Expand Down
5 changes: 5 additions & 0 deletions lib/rspec/rails/matchers.rb
Expand Up @@ -20,6 +20,11 @@ module Matchers
require 'rspec/rails/matchers/relation_match_array'
require 'rspec/rails/matchers/be_valid'
require 'rspec/rails/matchers/have_http_status'

if RSpec::Rails::FeatureCheck.has_active_job?
require 'rspec/rails/matchers/active_job'
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we're going to add a blankline to our require block, let's add one here too.


if RSpec::Rails::FeatureCheck.has_action_mailbox?
require 'rspec/rails/matchers/action_mailbox'
end
64 changes: 64 additions & 0 deletions lib/rspec/rails/matchers/action_mailbox.rb
@@ -0,0 +1,64 @@
module RSpec
module Rails
module Matchers
# Namespace for various implementations of ActionMailbox features
#
# @api private
module ActionMailbox
# @private
class Base < RSpec::Rails::Matchers::BaseMatcher
private

def create_inbound_email(message)
RSpec::Rails::MailboxExampleGroup.create_inbound_email(message)
end
end

# @private
class ReceiveInboundEmail < Base
def initialize(message)
super()

@inbound_email = create_inbound_email(message)
end

def matches?(mailbox)
@mailbox = mailbox
@receiver = ApplicationMailbox.router.send(:match_to_mailbox, inbound_email)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actionmailbox's public API does not expose a way to check which mailbox an email routes to that doesn't fully send the email to the mailbox for processing. expect(mailbox).to receive(:receive) also seems fraught because of the method name there.

I'll open up a PR upstream to expose something like this in the public API. It seems like a generally useful thing for testing, and I hope won't be too hard to land. If it does, I'll update the implementation here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR for that - rails/rails#36181


@receiver == @mailbox
end

def failure_message
"expected #{describe_inbound_email} to route to #{mailbox}".tap do |msg|
if receiver
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice :)

msg << ", but routed to #{receiver} instead"
end
end
end

def failure_message_when_negated
"expected #{describe_inbound_email} not to route to #{mailbox}"
end

private

attr_reader :inbound_email, :mailbox, :receiver

def describe_inbound_email
"mail to #{inbound_email.mail.to.to_sentence}"
end
end
end

# @api public
# Passes if the given inbound email would be routed to the subject inbox.
#
# @param message [Hash, Mail::Message] a mail message or hash of
# attributes used to build one
def receive_inbound_email(message)
ActionMailbox::ReceiveInboundEmail.new(message)
end
end
end
end
18 changes: 18 additions & 0 deletions spec/generators/rspec/mailbox/mailbox_generator_spec.rb
@@ -0,0 +1,18 @@
# Generators are not automatically loaded by Rails
require 'generators/rspec/mailbox/mailbox_generator'
require 'support/generators'

RSpec.describe Rspec::Generators::MailboxGenerator, :type => :generator, :skip => !RSpec::Rails::FeatureCheck.has_action_mailbox? do
setup_default_destination

describe 'the generated files' do
before { run_generator %w[forwards] }

subject { file('spec/mailboxes/forwards_mailbox_spec.rb') }

it { is_expected.to exist }
it { is_expected.to contain(/require 'rails_helper'/) }
it { is_expected.to contain(/describe ForwardsMailbox, #{type_metatag(:mailbox)}/) }

end
end
83 changes: 83 additions & 0 deletions spec/rspec/rails/example/mailbox_example_group_spec.rb
@@ -0,0 +1,83 @@
require "spec_helper"
require "rspec/rails/feature_check"

class ApplicationMailbox
class << self
attr_accessor :received

def receive(*)
self.received += 1
end
end

self.received = 0
end

module RSpec
module Rails
describe MailboxExampleGroup, :skip => !RSpec::Rails::FeatureCheck.has_active_job? do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should check for has_action_mailbox? instead of has_active_job?..

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Opened in #2123

it_behaves_like "an rspec-rails example group mixin", :mailbox,
'./spec/mailboxes/', '.\\spec\\mailboxes\\'

def group_for(klass)
RSpec::Core::ExampleGroup.describe klass do
include MailboxExampleGroup
end
end

let(:group) { group_for(::ApplicationMailbox) }
let(:example) { group.new }

describe '#have_been_delivered' do
it 'raises on undelivered mail' do
expect {
expect(double('IncomingEmail', :delivered? => false)).to example.have_been_delivered
}.to raise_error(/have been delivered/)
end

it 'does not raise otherwise' do
expect(double('IncomingEmail', :delivered? => true)).to example.have_been_delivered
end
end

describe '#have_bounced' do
it 'raises on unbounced mail' do
expect {
expect(double('IncomingEmail', :bounced? => false)).to example.have_bounced
}.to raise_error(/have bounced/)
end

it 'does not raise otherwise' do
expect(double('IncomingEmail', :bounced? => true)).to example.have_bounced
end
end

describe '#have_failed' do
it 'raises on unfailed mail' do
expect {
expect(double('IncomingEmail', :failed? => false)).to example.have_failed
}.to raise_error(/have failed/)
end

it 'does not raise otherwise' do
expect(double('IncomingEmail', :failed? => true)).to example.have_failed
end
end

describe '#process' do
before do
allow(RSpec::Rails::MailboxExampleGroup).to receive(:create_inbound_email) do |attributes|
mail = double('Mail::Message', attributes)
double('InboundEmail', :mail => mail)
end
end

it 'sends mail to the mailbox' do
expect {
example.process(:to => ['test@example.com'])
}.to change(::ApplicationMailbox, :received).by(1)
end
end
end
end
end
52 changes: 52 additions & 0 deletions spec/rspec/rails/matchers/action_mailbox_spec.rb
@@ -0,0 +1,52 @@
require "spec_helper"
require "rspec/rails/feature_check"

class ApplicationMailbox
class Router
def match_to_mailbox(*)
Inbox
end
end

def self.router
Router.new
end
end

class Inbox < ApplicationMailbox; end
class Otherbox < ApplicationMailbox; end

RSpec.describe "ActionMailbox matchers", :skip => !RSpec::Rails::FeatureCheck.has_active_job? do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

include RSpec::Rails::Matchers::ActionMailbox

describe "receive_inbound_email" do
let(:to) { ['to@example.com'] }

before do
allow(RSpec::Rails::MailboxExampleGroup).to receive(:create_inbound_email) do |attributes|
mail = double('Mail::Message', attributes)
double('InboundEmail', :mail => mail)
end
end

it "passes when it receives inbound email" do
expect(Inbox).to receive_inbound_email(:to => to)
end

it "passes when negated when it doesn't receive inbound email" do
expect(Otherbox).not_to receive_inbound_email(:to => to)
end

it "fails when it doesn't receive inbound email" do
expect {
expect(Otherbox).to receive_inbound_email(:to => to)
}.to raise_error(/expected mail to to@example.com to route to Otherbox, but routed to Inbox/)
end

it "fails when negated when it receives inbound email" do
expect {
expect(Inbox).not_to receive_inbound_email(:to => to)
}.to raise_error(/expected mail to to@example.com not to route to Inbox/)
end
end
end