From 2af639217295c731732cb9b17072fe7f8bd56012 Mon Sep 17 00:00:00 2001 From: Ryan Lue Date: Wed, 5 Dec 2018 13:31:13 +0800 Subject: [PATCH] Migrate email_process_identify_sender_max_test to RSpec --- Gemfile | 3 +- Gemfile.lock | 3 + spec/models/channel/email_parser_spec.rb | 180 +++++++++++------- spec/support/faker.rb | 5 + .../email_process_identify_sender_max_test.rb | 25 --- 5 files changed, 119 insertions(+), 97 deletions(-) create mode 100644 spec/support/faker.rb delete mode 100644 test/unit/email_process_identify_sender_max_test.rb diff --git a/Gemfile b/Gemfile index 447dd2303..cc24e5572 100644 --- a/Gemfile +++ b/Gemfile @@ -164,8 +164,9 @@ group :development, :test do # changelog generation gem 'github_changelog_generator' - # Use Factory Bot for generating random test data + # generate random test data gem 'factory_bot_rails' + gem 'faker' # mock http calls gem 'webmock' diff --git a/Gemfile.lock b/Gemfile.lock index c7ff844a4..b850f3494 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -165,6 +165,8 @@ GEM factory_bot_rails (4.8.2) factory_bot (~> 4.8.2) railties (>= 3.0.0) + faker (1.9.1) + i18n (>= 0.7) faraday (0.12.2) multipart-post (>= 1.2, < 3) faraday-http-cache (2.0.0) @@ -533,6 +535,7 @@ DEPENDENCIES eventmachine execjs factory_bot_rails + faker github_changelog_generator guard guard-livereload diff --git a/spec/models/channel/email_parser_spec.rb b/spec/models/channel/email_parser_spec.rb index 512efc858..80562efda 100644 --- a/spec/models/channel/email_parser_spec.rb +++ b/spec/models/channel/email_parser_spec.rb @@ -1,103 +1,141 @@ require 'rails_helper' RSpec.describe Channel::EmailParser, type: :model do + let(:subject) { described_class.new } let(:mail_file) { Rails.root.join('test', 'data', 'mail', 'mail001.box') } - let(:raw_mail) { File.read(mail_file) } + let(:raw_mail) { File.read(mail_file) } describe '#process' do - let(:raw_mail) { File.read(mail_file).sub(/(?<=^Subject: ).*$/, test_string) } - let(:test_string) do - Setting.get('ticket_hook') + Setting.get('ticket_hook_divider') + ticket.number - end - let(:ticket) { create(:ticket) } + let(:raw_mail) { File.read(mail_file).sub(/(?<=^Subject: ).*$/, test_string) } + let(:test_string) { Setting.get('ticket_hook') + Setting.get('ticket_hook_divider') + ticket.number } + let(:ticket) { create(:ticket) } - context 'when email subject contains ticket reference' do - it 'adds message to ticket' do - expect { described_class.new.process({}, raw_mail) } - .to change { ticket.articles.length } + context 'for creating new users' do + context 'with one unrecognized email address' do + let(:raw_mail) { <<~RAW } + From: #{Faker::Internet.unique.email} + To: #{User.pluck(:email).reject(&:blank?).sample} + Subject: Foo bar + + Lorem ipsum dolor + RAW + + it 'creates one new user' do + expect { Channel::EmailParser.new.process({}, raw_mail) } + .to change { User.count }.by(1) + end + end + + context 'with a large number (42+) of unrecognized email addresses' do + let(:raw_mail) { <<~RAW } + From: #{Faker::Internet.unique.email} + To: #{Array.new(22) { Faker::Internet.unique.email }.join(', ')} + Cc: #{Array.new(22) { Faker::Internet.unique.email }.join(', ')} + Subject: test max sender identify + + Some Text + RAW + + it 'never creates more than 41 users per email' do + expect { Channel::EmailParser.new.process({}, raw_mail) } + .to change { User.count }.by(41) + end end end - context 'when configured to search body' do - before { Setting.set('postmaster_follow_up_search_in', 'body') } - - context 'when body contains ticket reference' do - context 'in visible text' do - let(:raw_mail) { File.read(mail_file).sub(/Hallo =\nMartin,(?=)/, test_string) } - - it 'adds message to ticket' do - expect { described_class.new.process({}, raw_mail) } - .to change { ticket.articles.length } - end + context 'for associating emails to tickets' do + context 'when email subject contains ticket reference' do + it 'adds message to ticket' do + expect { described_class.new.process({}, raw_mail) } + .to change { ticket.articles.length } end + end - context 'as part of a larger word' do - let(:raw_mail) { File.read(mail_file).sub(/(?<=Hallo) =\n(?=Martin,)/, test_string) } + context 'when configured to search body' do + before { Setting.set('postmaster_follow_up_search_in', 'body') } - it 'creates a separate ticket' do - expect { described_class.new.process({}, raw_mail) } - .not_to change { ticket.articles.length } + context 'when body contains ticket reference' do + context 'in visible text' do + let(:raw_mail) { File.read(mail_file).sub(/Hallo =\nMartin,(?=)/, test_string) } + + it 'adds message to ticket' do + expect { described_class.new.process({}, raw_mail) } + .to change { ticket.articles.length } + end end - end - context 'in html attributes' do - let(:raw_mail) { File.read(mail_file).sub(%r{}m, %(
)) } + context 'as part of a larger word' do + let(:raw_mail) { File.read(mail_file).sub(/(?<=Hallo) =\n(?=Martin,)/, test_string) } - it 'creates a separate ticket' do - expect { described_class.new.process({}, raw_mail) } - .not_to change { ticket.articles.length } + it 'creates a separate ticket' do + expect { described_class.new.process({}, raw_mail) } + .not_to change { ticket.articles.length } + end + end + + context 'in html attributes' do + let(:raw_mail) { File.read(mail_file).sub(%r{}m, %(
)) } + + it 'creates a separate ticket' do + expect { described_class.new.process({}, raw_mail) } + .not_to change { ticket.articles.length } + end end end end end - # see https://github.com/zammad/zammad/issues/2198 - context 'when sender address contains spaces (#2198)' do - let(:mail_file) { Rails.root.join('test', 'data', 'mail', 'mail071.box') } - let(:sender_email) { 'powerquadrantsystem@example.com' } + context 'for sender/recipient address formatting' do + # see https://github.com/zammad/zammad/issues/2198 + context 'when sender address contains spaces (#2198)' do + let(:mail_file) { Rails.root.join('test', 'data', 'mail', 'mail071.box') } + let(:sender_email) { 'powerquadrantsystem@example.com' } - it 'removes them before creating a new user' do - expect { described_class.new.process({}, raw_mail) } - .to change { User.where(email: sender_email).count }.to(1) + it 'removes them before creating a new user' do + expect { described_class.new.process({}, raw_mail) } + .to change { User.where(email: sender_email).count }.to(1) + end + + it 'marks new user email as invalid' do + described_class.new.process({}, raw_mail) + + expect(User.find_by(email: sender_email).preferences) + .to include('mail_delivery_failed' => true) + .and include('mail_delivery_failed_reason' => 'invalid email') + .and include('mail_delivery_failed_data' => a_kind_of(ActiveSupport::TimeWithZone)) + end end - it 'marks new user email as invalid' do - described_class.new.process({}, raw_mail) + # see https://github.com/zammad/zammad/issues/2254 + context 'when sender address contains > (#2254)' do + let(:mail_file) { Rails.root.join('test', 'data', 'mail', 'mail076.box') } + let(:sender_email) { 'millionslotteryspaintransfer@example.com' } - expect(User.find_by(email: sender_email).preferences) - .to include('mail_delivery_failed' => true) - .and include('mail_delivery_failed_reason' => 'invalid email') - .and include('mail_delivery_failed_data' => a_kind_of(ActiveSupport::TimeWithZone)) + it 'removes them before creating a new user' do + expect { described_class.new.process({}, raw_mail) } + .to change { User.where(email: sender_email).count }.to(1) + end + + it 'marks new user email as invalid' do + described_class.new.process({}, raw_mail) + + expect(User.find_by(email: sender_email).preferences) + .to include('mail_delivery_failed' => true) + .and include('mail_delivery_failed_reason' => 'invalid email') + .and include('mail_delivery_failed_data' => a_kind_of(ActiveSupport::TimeWithZone)) + end end end - # see https://github.com/zammad/zammad/issues/2254 - context 'when sender address contains > (#2254)' do - let(:mail_file) { Rails.root.join('test', 'data', 'mail', 'mail076.box') } - let(:sender_email) { 'millionslotteryspaintransfer@example.com' } + context 'for charset handling' do + # see https://github.com/zammad/zammad/issues/2224 + context 'when header specifies Windows-1258 charset (#2224)' do + let(:mail_file) { Rails.root.join('test', 'data', 'mail', 'mail072.box') } - it 'removes them before creating a new user' do - expect { described_class.new.process({}, raw_mail) } - .to change { User.where(email: sender_email).count }.to(1) - end - - it 'marks new user email as invalid' do - described_class.new.process({}, raw_mail) - - expect(User.find_by(email: sender_email).preferences) - .to include('mail_delivery_failed' => true) - .and include('mail_delivery_failed_reason' => 'invalid email') - .and include('mail_delivery_failed_data' => a_kind_of(ActiveSupport::TimeWithZone)) - end - end - - # see https://github.com/zammad/zammad/issues/2224 - context 'when header specifies Windows-1258 charset (#2224)' do - let(:mail_file) { Rails.root.join('test', 'data', 'mail', 'mail072.box') } - - it 'does not raise Encoding::ConverterNotFoundError' do - expect { described_class.new.process({}, raw_mail) } - .not_to raise_error + it 'does not raise Encoding::ConverterNotFoundError' do + expect { described_class.new.process({}, raw_mail) } + .not_to raise_error + end end end end diff --git a/spec/support/faker.rb b/spec/support/faker.rb new file mode 100644 index 000000000..00d3734f6 --- /dev/null +++ b/spec/support/faker.rb @@ -0,0 +1,5 @@ +RSpec.configure do |config| + config.after(:each) do + Faker::UniqueGenerator.clear + end +end diff --git a/test/unit/email_process_identify_sender_max_test.rb b/test/unit/email_process_identify_sender_max_test.rb deleted file mode 100644 index ee7ffe0b0..000000000 --- a/test/unit/email_process_identify_sender_max_test.rb +++ /dev/null @@ -1,25 +0,0 @@ -require 'test_helper' - -class EmailProcessIdentifySenderMax < ActiveSupport::TestCase - - test 'text max created recipients per email' do - current_users = User.count - email_raw_string = "From: #{generate_recipient} -To: #{generate_recipient(22)} -Cc: #{generate_recipient(22)} -Subject: test max sender identify - -Some Text" - - ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process({}, email_raw_string) - ticket = Ticket.find(ticket_p.id) - assert_equal('test max sender identify', ticket.title) - assert_equal(current_users + 41, User.count) - end - - def generate_recipient(count = 1) - uid = -> { rand(999_999_999_999_999) } - Array.new(count) { "#{uid.call}@#{uid.call}.example.com" }.join(', ') - end - -end