diff options
author | Kamil Trzcinski <ayufan@ayufan.eu> | 2015-11-05 14:51:03 +0100 |
---|---|---|
committer | Kamil Trzcinski <ayufan@ayufan.eu> | 2015-11-05 14:51:03 +0100 |
commit | 3c94dbcf9df7c21b09f0254624da030d19b8035a (patch) | |
tree | f73c82a22ef7ffb9edbd0cb30f8b503639e9d509 | |
parent | 4f574388fc8594ec6f6e9d719f524912d17c4059 (diff) | |
download | gitlab-ce-sidekiq-upgrade.tar.gz |
Upgrade sidekiq to newer versionsidekiq-upgrade
Fix mail tests
-rw-r--r-- | Gemfile | 4 | ||||
-rw-r--r-- | Gemfile.lock | 46 | ||||
-rw-r--r-- | spec/models/ci/project_services/mail_service_spec.rb | 43 | ||||
-rw-r--r-- | spec/services/notification_service_spec.rb | 125 |
4 files changed, 134 insertions, 84 deletions
@@ -122,7 +122,7 @@ gem 'acts-as-taggable-on', '~> 3.4' # Background jobs gem 'sinatra', '~> 1.4.4', require: nil -gem 'sidekiq', '3.3.0' +gem 'sidekiq', '~> 3.5.0' gem 'sidetiq', '~> 0.6.3' # HTTP requests @@ -216,7 +216,7 @@ group :development do gem "letter_opener", '~> 1.1.2' gem 'quiet_assets', '~> 1.0.2' gem 'rack-mini-profiler', '~> 0.9.0', require: false - gem 'rerun', '~> 0.10.0' + gem 'rerun', '~> 0.11.0' gem 'bullet', require: false gem 'active_record_query_trace', require: false gem 'rack-lineprof', platform: :mri diff --git a/Gemfile.lock b/Gemfile.lock index dce728baf18..859dc7b6c14 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -106,8 +106,23 @@ GEM activemodel (>= 3.2.0) activesupport (>= 3.2.0) json (>= 1.7) - celluloid (0.16.0) - timers (~> 4.0.0) + celluloid (0.17.2) + celluloid-essentials + celluloid-extras + celluloid-fsm + celluloid-pool + celluloid-supervision + timers (>= 4.1.1) + celluloid-essentials (0.20.5) + timers (>= 4.1.1) + celluloid-extras (0.20.5) + timers (>= 4.1.1) + celluloid-fsm (0.20.5) + timers (>= 4.1.1) + celluloid-pool (0.20.5) + timers (>= 4.1.1) + celluloid-supervision (0.20.5) + timers (>= 4.1.1) charlock_holmes (0.6.9.4) chunky_png (1.3.4) cliver (0.3.2) @@ -375,8 +390,7 @@ GEM addressable (~> 2.3) letter_opener (1.1.2) launchy (~> 2.2) - listen (2.10.1) - celluloid (~> 0.16.0) + listen (3.0.3) rb-fsevent (>= 0.9.3) rb-inotify (>= 0.9) macaddr (1.7.1) @@ -530,7 +544,7 @@ GEM raindrops (0.15.0) rake (10.4.2) raphael-rails (2.1.2) - rb-fsevent (0.9.5) + rb-fsevent (0.9.6) rb-inotify (0.9.5) ffi (>= 0.5.0) rblineprof (0.3.6) @@ -562,8 +576,8 @@ GEM redis-store (1.1.6) redis (>= 2.2) request_store (1.2.0) - rerun (0.10.0) - listen (~> 2.7, >= 2.7.3) + rerun (0.11.0) + listen (~> 3.0) responders (1.1.2) railties (>= 3.2, < 4.2) rest-client (1.8.0) @@ -644,12 +658,12 @@ GEM rack shoulda-matchers (2.8.0) activesupport (>= 3.0.0) - sidekiq (3.3.0) - celluloid (>= 0.16.0) - connection_pool (>= 2.0.0) - json - redis (>= 3.0.6) - redis-namespace (>= 1.3.1) + sidekiq (3.5.3) + celluloid (~> 0.17.2) + connection_pool (~> 2.2, >= 2.2.0) + json (~> 1.0) + redis (~> 3.2, >= 3.2.1) + redis-namespace (~> 1.5, >= 1.5.2) sidetiq (0.6.3) celluloid (>= 0.14.1) ice_cube (= 0.11.1) @@ -713,7 +727,7 @@ GEM thor (0.19.1) thread_safe (0.3.5) tilt (1.4.1) - timers (4.0.4) + timers (4.1.1) hitimes timfel-krb5-auth (0.8.3) tinder (1.10.1) @@ -887,7 +901,7 @@ DEPENDENCIES redcarpet (~> 3.3.3) redis-rails (~> 4.0.0) request_store (~> 1.2.0) - rerun (~> 0.10.0) + rerun (~> 0.11.0) rqrcode-rails3 (~> 0.1.7) rspec-rails (~> 3.3.0) rubocop (~> 0.28.0) @@ -900,7 +914,7 @@ DEPENDENCIES settingslogic (~> 2.0.9) sham_rack shoulda-matchers (~> 2.8.0) - sidekiq (= 3.3.0) + sidekiq (~> 3.5.0) sidetiq (~> 0.6.3) simplecov (~> 0.10.0) sinatra (~> 1.4.4) diff --git a/spec/models/ci/project_services/mail_service_spec.rb b/spec/models/ci/project_services/mail_service_spec.rb index d9b3d34ff15..1c1d8f90b2b 100644 --- a/spec/models/ci/project_services/mail_service_spec.rb +++ b/spec/models/ci/project_services/mail_service_spec.rb @@ -15,6 +15,12 @@ require 'spec_helper' describe Ci::MailService do + let(:deliveries) { Ci::Notify.deliveries } + + before do + deliveries.clear + end + describe "Associations" do it { is_expected.to belong_to :project } end @@ -44,13 +50,13 @@ describe Ci::MailService do end it do - should_email("git@example.com") mail.execute(build) + should_email("git@example.com") end def should_email(email) - expect(Ci::Notify).to receive(:build_fail_email).with(build.id, email) - expect(Ci::Notify).not_to receive(:build_success_email).with(build.id, email) + expect(sent?(build, 'failed', email)).to be_truthy + expect(sent?(build, 'success', email)).to be_falsey end end @@ -67,13 +73,13 @@ describe Ci::MailService do end it do - should_email("git@example.com") mail.execute(build) + should_email("git@example.com") end def should_email(email) - expect(Ci::Notify).to receive(:build_success_email).with(build.id, email) - expect(Ci::Notify).not_to receive(:build_fail_email).with(build.id, email) + expect(sent?(build, 'success', email)).to be_truthy + expect(sent?(build, 'failed', email)).to be_falsey end end @@ -95,14 +101,14 @@ describe Ci::MailService do end it do + mail.execute(build) should_email("git@example.com") should_email("jeroen@example.com") - mail.execute(build) end def should_email(email) - expect(Ci::Notify).to receive(:build_success_email).with(build.id, email) - expect(Ci::Notify).not_to receive(:build_fail_email).with(build.id, email) + expect(sent?(build, 'success', email)).to be_truthy + expect(sent?(build, 'failed', email)).to be_falsey end end @@ -124,14 +130,14 @@ describe Ci::MailService do end it do + mail.execute(build) if mail.can_execute?(build) should_email(commit.git_author_email) should_email("jeroen@example.com") - mail.execute(build) if mail.can_execute?(build) end def should_email(email) - expect(Ci::Notify).not_to receive(:build_success_email).with(build.id, email) - expect(Ci::Notify).not_to receive(:build_fail_email).with(build.id, email) + expect(sent?(build, 'success', email)).to be_falsey + expect(sent?(build, 'failed', email)).to be_falsey end end @@ -177,15 +183,22 @@ describe Ci::MailService do it do Ci::Build.retry(build) + mail.execute(build) if mail.can_execute?(build) should_email(commit.git_author_email) should_email("jeroen@example.com") - mail.execute(build) if mail.can_execute?(build) end def should_email(email) - expect(Ci::Notify).not_to receive(:build_success_email).with(build.id, email) - expect(Ci::Notify).not_to receive(:build_fail_email).with(build.id, email) + expect(sent?(build, 'success', email)).to be_falsey + expect(sent?(build, 'failed', email)).to be_falsey end end end + + def sent?(build, status, email) + deliveries.any? do |delivery| + delivery.subject.include?("Build #{status} for #{build.gl_project.name_with_namespace}") && + delivery.to.include?(email) + end + end end diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 520140917aa..618c0ea5a57 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -2,6 +2,11 @@ require 'spec_helper' describe NotificationService do let(:notification) { NotificationService.new } + let(:deliveries) { Notify.deliveries } + + before(:each) do + deliveries.clear + end describe 'Keys' do describe :new_key do @@ -10,8 +15,8 @@ describe NotificationService do it { expect(notification.new_key(key)).to be_truthy } it 'should sent email to key owner' do - expect(Notify).to receive(:new_ssh_key_email).with(key.id) notification.new_key(key) + expect(sent?('SSH key was added to your account', key.user_id)).to be_truthy end end end @@ -23,8 +28,8 @@ describe NotificationService do it { expect(notification.new_email(email)).to be_truthy } it 'should send email to email owner' do - expect(Notify).to receive(:new_email_email).with(email.id) notification.new_email(email) + expect(sent?('Email was added to your account', email.user_id)).to be_truthy end end end @@ -47,6 +52,8 @@ describe NotificationService do it do add_users_with_subscription(note.project, issue) + notification.new_note(note) + should_email(@u_watcher.id) should_email(note.noteable.author_id) should_email(note.noteable.assignee_id) @@ -57,15 +64,13 @@ describe NotificationService do should_not_email(@u_disabled.id) should_not_email(@unsubscriber.id) should_not_email(@u_outsider_mentioned) - - notification.new_note(note) end it 'filters out "mentioned in" notes' do mentioned_note = SystemNoteService.cross_reference(mentioned_issue, issue, issue.author) - expect(Notify).not_to receive(:note_issue_email) notification.new_note(mentioned_note) + expect(deliveries.length).to eq(0) end end @@ -85,6 +90,8 @@ describe NotificationService do end it do + notification.new_note(note) + should_email(note.noteable.author_id) should_email(note.noteable.assignee_id) should_email(@u_mentioned.id) @@ -92,16 +99,15 @@ describe NotificationService do should_not_email(note.author_id) should_not_email(@u_participating.id) should_not_email(@u_disabled.id) - notification.new_note(note) end end def should_email(user_id) - expect(Notify).to receive(:note_issue_email).with(user_id, note.id) + expect(sent?(note.noteable.title, user_id)).to be_truthy end def should_not_email(user_id) - expect(Notify).not_to receive(:note_issue_email).with(user_id, note.id) + expect(sent?(note.noteable.title, user_id)).to be_falsey end end @@ -117,6 +123,8 @@ describe NotificationService do describe :new_note do it do + notification.new_note(note) + # Notify all team members note.project.team.members.each do |member| # User with disabled notification should not be notified @@ -130,23 +138,22 @@ describe NotificationService do should_not_email(@u_mentioned.id) should_not_email(@u_disabled.id) should_not_email(@u_not_mentioned.id) - notification.new_note(note) end it 'filters out "mentioned in" notes' do mentioned_note = SystemNoteService.cross_reference(mentioned_issue, issue, issue.author) - expect(Notify).not_to receive(:note_issue_email) notification.new_note(mentioned_note) + expect(deliveries.length).to eq(0) end end def should_email(user_id) - expect(Notify).to receive(:note_issue_email).with(user_id, note.id) + expect(sent?(note.noteable.title, user_id)).to be_truthy end def should_not_email(user_id) - expect(Notify).not_to receive(:note_issue_email).with(user_id, note.id) + expect(sent?(note.noteable.title, user_id)).to be_falsey end end @@ -161,38 +168,41 @@ describe NotificationService do describe :new_note do it do + notification.new_note(note) + should_email(@u_committer.id, note) should_email(@u_watcher.id, note) should_not_email(@u_mentioned.id, note) should_not_email(note.author_id, note) should_not_email(@u_participating.id, note) should_not_email(@u_disabled.id, note) - notification.new_note(note) end it do note.update_attribute(:note, '@mention referenced') + notification.new_note(note) + should_email(@u_committer.id, note) should_email(@u_watcher.id, note) should_email(@u_mentioned.id, note) should_not_email(note.author_id, note) should_not_email(@u_participating.id, note) should_not_email(@u_disabled.id, note) - notification.new_note(note) end it do @u_committer.update_attributes(notification_level: Notification::N_MENTION) - should_not_email(@u_committer.id, note) notification.new_note(note) + + should_not_email(@u_committer.id, note) end def should_email(user_id, n) - expect(Notify).to receive(:note_commit_email).with(user_id, n.id) + expect(sent?(n.noteable.title, user_id)).to be_truthy end def should_not_email(user_id, n) - expect(Notify).not_to receive(:note_commit_email).with(user_id, n.id) + expect(sent?(n.noteable.title, user_id)).to be_falsey end end end @@ -209,32 +219,36 @@ describe NotificationService do describe :new_issue do it do + notification.new_issue(issue, @u_disabled) + should_email(issue.assignee_id) should_email(@u_watcher.id) should_email(@u_participant_mentioned.id) should_not_email(@u_mentioned.id) should_not_email(@u_participating.id) should_not_email(@u_disabled.id) - notification.new_issue(issue, @u_disabled) end it do issue.assignee.update_attributes(notification_level: Notification::N_MENTION) - should_not_email(issue.assignee_id) notification.new_issue(issue, @u_disabled) + + should_not_email(issue.assignee_id) end def should_email(user_id) - expect(Notify).to receive(:new_issue_email).with(user_id, issue.id) + expect(sent?(issue.title, user_id)).to be_truthy end def should_not_email(user_id) - expect(Notify).not_to receive(:new_issue_email).with(user_id, issue.id) + expect(sent?(issue.title, user_id)).to be_falsey end end describe :reassigned_issue do it 'should email new assignee' do + notification.reassigned_issue(issue, @u_disabled) + should_email(issue.assignee_id) should_email(@u_watcher.id) should_email(@u_participant_mentioned.id) @@ -242,21 +256,21 @@ describe NotificationService do should_not_email(@unsubscriber.id) should_not_email(@u_participating.id) should_not_email(@u_disabled.id) - - notification.reassigned_issue(issue, @u_disabled) end def should_email(user_id) - expect(Notify).to receive(:reassigned_issue_email).with(user_id, issue.id, nil, @u_disabled.id) + expect(sent?(issue.title, user_id)).to be_truthy end def should_not_email(user_id) - expect(Notify).not_to receive(:reassigned_issue_email).with(user_id, issue.id, issue.assignee_id, @u_disabled.id) + expect(sent?(issue.title, user_id)).to be_falsey end end describe :close_issue do it 'should sent email to issue assignee and issue author' do + notification.close_issue(issue, @u_disabled) + should_email(issue.assignee_id) should_email(issue.author_id) should_email(@u_watcher.id) @@ -265,21 +279,21 @@ describe NotificationService do should_not_email(@unsubscriber.id) should_not_email(@u_participating.id) should_not_email(@u_disabled.id) - - notification.close_issue(issue, @u_disabled) end def should_email(user_id) - expect(Notify).to receive(:closed_issue_email).with(user_id, issue.id, @u_disabled.id) + expect(sent?(issue.title, user_id)).to be_truthy end def should_not_email(user_id) - expect(Notify).not_to receive(:closed_issue_email).with(user_id, issue.id, @u_disabled.id) + expect(sent?(issue.title, user_id)).to be_falsey end end describe :reopen_issue do it 'should send email to issue assignee and issue author' do + notification.reopen_issue(issue, @u_disabled) + should_email(issue.assignee_id) should_email(issue.author_id) should_email(@u_watcher.id) @@ -288,16 +302,14 @@ describe NotificationService do should_not_email(@unsubscriber.id) should_not_email(@u_participating.id) should_not_email(@u_disabled.id) - - notification.reopen_issue(issue, @u_disabled) end def should_email(user_id) - expect(Notify).to receive(:issue_status_changed_email).with(user_id, issue.id, 'reopened', @u_disabled.id) + expect(sent?(issue.title, user_id)).to be_truthy end def should_not_email(user_id) - expect(Notify).not_to receive(:issue_status_changed_email).with(user_id, issue.id, 'reopened', @u_disabled.id) + expect(sent?(issue.title, user_id)).to be_falsey end end end @@ -313,25 +325,28 @@ describe NotificationService do describe :new_merge_request do it do + notification.new_merge_request(merge_request, @u_disabled) + should_email(merge_request.assignee_id) should_email(@u_watcher.id) should_email(@u_participant_mentioned.id) should_not_email(@u_participating.id) should_not_email(@u_disabled.id) - notification.new_merge_request(merge_request, @u_disabled) end def should_email(user_id) - expect(Notify).to receive(:new_merge_request_email).with(user_id, merge_request.id) + expect(sent?(merge_request.title, user_id)).to be_truthy end def should_not_email(user_id) - expect(Notify).not_to receive(:new_merge_request_email).with(user_id, merge_request.id) + expect(sent?(merge_request.title, user_id)).to be_falsey end end describe :reassigned_merge_request do it do + notification.reassigned_merge_request(merge_request, merge_request.author) + should_email(merge_request.assignee_id) should_email(@u_watcher.id) should_email(@u_participant_mentioned.id) @@ -339,20 +354,20 @@ describe NotificationService do should_not_email(@unsubscriber.id) should_not_email(@u_participating.id) should_not_email(@u_disabled.id) - notification.reassigned_merge_request(merge_request, merge_request.author) end def should_email(user_id) - expect(Notify).to receive(:reassigned_merge_request_email).with(user_id, merge_request.id, nil, merge_request.author_id) + expect(sent?(merge_request.title, user_id)).to be_truthy end def should_not_email(user_id) - expect(Notify).not_to receive(:reassigned_merge_request_email).with(user_id, merge_request.id, merge_request.assignee_id, merge_request.author_id) + expect(sent?(merge_request.title, user_id)).to be_falsey end end describe :closed_merge_request do it do + notification.close_mr(merge_request, @u_disabled) should_email(merge_request.assignee_id) should_email(@u_watcher.id) should_email(@u_participant_mentioned.id) @@ -360,20 +375,20 @@ describe NotificationService do should_not_email(@unsubscriber.id) should_not_email(@u_participating.id) should_not_email(@u_disabled.id) - notification.close_mr(merge_request, @u_disabled) end def should_email(user_id) - expect(Notify).to receive(:closed_merge_request_email).with(user_id, merge_request.id, @u_disabled.id) + expect(sent?(merge_request.title, user_id)).to be_truthy end def should_not_email(user_id) - expect(Notify).not_to receive(:closed_merge_request_email).with(user_id, merge_request.id, @u_disabled.id) + expect(sent?(merge_request.title, user_id)).to be_falsey end end describe :merged_merge_request do it do + notification.merge_mr(merge_request, @u_disabled) should_email(merge_request.assignee_id) should_email(@u_watcher.id) should_email(@u_participant_mentioned.id) @@ -381,20 +396,20 @@ describe NotificationService do should_not_email(@unsubscriber.id) should_not_email(@u_participating.id) should_not_email(@u_disabled.id) - notification.merge_mr(merge_request, @u_disabled) end def should_email(user_id) - expect(Notify).to receive(:merged_merge_request_email).with(user_id, merge_request.id, @u_disabled.id) + expect(sent?(merge_request.title, user_id)).to be_truthy end def should_not_email(user_id) - expect(Notify).not_to receive(:merged_merge_request_email).with(user_id, merge_request.id, @u_disabled.id) + expect(sent?(merge_request.title, user_id)).to be_falsey end end describe :reopen_merge_request do it do + notification.reopen_mr(merge_request, @u_disabled) should_email(merge_request.assignee_id) should_email(@u_watcher.id) should_email(@u_participant_mentioned.id) @@ -402,15 +417,14 @@ describe NotificationService do should_not_email(@unsubscriber.id) should_not_email(@u_participating.id) should_not_email(@u_disabled.id) - notification.reopen_mr(merge_request, @u_disabled) end def should_email(user_id) - expect(Notify).to receive(:merge_request_status_email).with(user_id, merge_request.id, 'reopened', @u_disabled.id) + expect(sent?(merge_request.title, user_id)).to be_truthy end def should_not_email(user_id) - expect(Notify).not_to receive(:merge_request_status_email).with(user_id, merge_request.id, 'reopened', @u_disabled.id) + expect(sent?(merge_request.title, user_id)).to be_falsey end end end @@ -424,18 +438,18 @@ describe NotificationService do describe :project_was_moved do it do + notification.project_was_moved(project, "gitlab/gitlab") should_email(@u_watcher.id) should_email(@u_participating.id) should_not_email(@u_disabled.id) - notification.project_was_moved(project, "gitlab/gitlab") end def should_email(user_id) - expect(Notify).to receive(:project_was_moved_email).with(project.id, user_id, "gitlab/gitlab") + expect(sent?('Project was moved', user_id)).to be_truthy end def should_not_email(user_id) - expect(Notify).not_to receive(:project_was_moved_email).with(project.id, user_id, "gitlab/gitlab") + expect(sent?('Project was moved', user_id)).to be_falsey end end end @@ -469,4 +483,13 @@ describe NotificationService do issuable.subscriptions.create(user: @subscriber, subscribed: true) issuable.subscriptions.create(user: @unsubscriber, subscribed: false) end + + def sent?(subject, recipient_id) + recipient = User.find(recipient_id) + + deliveries.any? do |delivery| + delivery.subject.include?(subject) && + delivery.to.include?(recipient.notification_email) + end + end end |