diff options
36 files changed, 147 insertions, 196 deletions
@@ -81,8 +81,9 @@ gem "acts-as-taggable-on", "2.3.3" gem "draper", "~> 0.18.0" # Background jobs -gem "resque", git: "https://github.com/gitlabhq/resque.git", ref: "9ef4700306dd946a3ac000612428967ce0c32213" -gem 'resque_mailer' +gem 'slim' +gem 'sinatra', :require => nil +gem 'sidekiq', '2.6.4' # HTTP requests gem "httparty" @@ -157,7 +158,6 @@ group :test do gem "simplecov", require: false gem "shoulda-matchers", "1.3.0" gem 'email_spec' - gem 'resque_spec' gem "webmock" gem 'test_after_commit' end diff --git a/Gemfile.lock b/Gemfile.lock index 7d76bf9e75c..8fba310d4eb 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -55,17 +55,6 @@ GIT raphael-rails (2.1.0) GIT - remote: https://github.com/gitlabhq/resque.git - revision: 9ef4700306dd946a3ac000612428967ce0c32213 - ref: 9ef4700306dd946a3ac000612428967ce0c32213 - specs: - resque (2.0.0.pre.1) - json - redis-namespace (~> 1.0) - sinatra (>= 0.9.2) - vegas (~> 0.1.2) - -GIT remote: https://github.com/jonleighton/poltergeist.git revision: 5c2e092001074a8cf09f332d3714e9ba150bc8ca ref: 5c2e092001074a8cf09f332d3714e9ba150bc8ca @@ -132,6 +121,9 @@ GEM carrierwave (0.7.1) activemodel (>= 3.2.0) activesupport (>= 3.2.0) + celluloid (0.12.4) + facter (>= 1.6.12) + timers (>= 1.0.0) charlock_holmes (0.6.9) childprocess (0.3.6) ffi (~> 1.0, >= 1.0.6) @@ -150,6 +142,7 @@ GEM coffee-script-source (1.4.0) colored (1.2) colorize (0.5.8) + connection_pool (1.0.0) crack (0.3.1) daemons (1.1.9) devise (2.1.2) @@ -169,6 +162,7 @@ GEM eventmachine (1.0.0) execjs (1.4.0) multi_json (~> 1.0) + facter (1.6.17) factory_girl (4.1.0) activesupport (>= 3.0.0) factory_girl_rails (4.1.0) @@ -370,11 +364,6 @@ GEM redis (3.0.2) redis-namespace (1.2.1) redis (~> 3.0.0) - resque_mailer (2.1.0) - actionmailer (~> 3.0) - resque_spec (0.12.5) - resque (>= 1.19.0) - rspec (>= 2.5.0) rspec (2.12.0) rspec-core (~> 2.12.0) rspec-expectations (~> 2.12.0) @@ -412,6 +401,12 @@ GEM sexp_processor (4.1.3) shoulda-matchers (1.3.0) activesupport (>= 3.0.0) + sidekiq (2.6.4) + celluloid (~> 0.12.0) + connection_pool (~> 1.0) + multi_json (~> 1) + redis (~> 3) + redis-namespace simplecov (0.7.1) multi_json (~> 1.0) simplecov-html (~> 0.7.1) @@ -421,6 +416,9 @@ GEM rack-protection (~> 1.2) tilt (~> 1.3, >= 1.3.3) six (0.2.0) + slim (1.3.6) + temple (~> 0.5.5) + tilt (~> 1.3.3) slop (3.3.3) spinach (0.5.2) colorize @@ -435,6 +433,7 @@ GEM rack (~> 1.0) tilt (~> 1.1, != 1.3.0) stamp (0.3.0) + temple (0.5.5) test_after_commit (0.0.1) therubyracer (0.10.2) libv8 (~> 3.3.10) @@ -444,6 +443,7 @@ GEM rack (>= 1.0.0) thor (0.16.0) tilt (1.3.3) + timers (1.0.2) treetop (1.4.12) polyglot polyglot (>= 0.3.1) @@ -455,8 +455,6 @@ GEM kgio (~> 2.6) rack raindrops (~> 0.7) - vegas (0.1.11) - rack (>= 1.0.0) virtus (0.5.2) backports (~> 2.6.1) warden (1.2.1) @@ -534,17 +532,17 @@ DEPENDENCIES rb-fsevent rb-inotify redcarpet (~> 2.2.2) - resque! - resque_mailer - resque_spec rspec-rails sass-rails (~> 3.2.5) sdoc seed-fu settingslogic shoulda-matchers (= 1.3.0) + sidekiq (= 2.6.4) simplecov + sinatra six + slim spinach-rails stamp test_after_commit @@ -1,2 +1,2 @@ web: bundle exec rails s -p $PORT -worker: bundle exec rake environment resque:work QUEUE=* VVERBOSE=1 +worker: bundle exec sidekiq -q post_receive,mailer,system_hook,common,default diff --git a/app/controllers/admin/dashboard_controller.rb b/app/controllers/admin/dashboard_controller.rb index 827dd0cf7cd..f97c56b0b31 100644 --- a/app/controllers/admin/dashboard_controller.rb +++ b/app/controllers/admin/dashboard_controller.rb @@ -3,10 +3,6 @@ class Admin::DashboardController < AdminController @projects = Project.order("created_at DESC").limit(10) @users = User.order("created_at DESC").limit(10) - @resque_accessible = true - @workers = Resque.workers - @pending_jobs = Resque.size(:post_receive) - rescue Redis::InheritedError @resque_accessible = false end diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index cec680bc356..c672940a05e 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -1,5 +1,5 @@ class Notify < ActionMailer::Base - include Resque::Mailer + add_template_helper ApplicationHelper add_template_helper GitlabMarkdownHelper diff --git a/app/models/project.rb b/app/models/project.rb index 702f4b2bf81..32b951349a9 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -251,7 +251,7 @@ class Project < ActiveRecord::Base def send_move_instructions self.users_projects.each do |member| - Notify.project_was_moved_email(member.id).deliver + Notify.delay.project_was_moved_email(member.id) end end diff --git a/app/models/system_hook.rb b/app/models/system_hook.rb index 2ae5b1314e9..5f1bd6477c4 100644 --- a/app/models/system_hook.rb +++ b/app/models/system_hook.rb @@ -19,6 +19,6 @@ class SystemHook < WebHook end def async_execute(data) - Resque.enqueue(SystemHookWorker, id, data) + Sidekiq::Client.enqueue(SystemHookWorker, id, data) end end diff --git a/app/observers/issue_observer.rb b/app/observers/issue_observer.rb index 131336be8b6..262d0f892c4 100644 --- a/app/observers/issue_observer.rb +++ b/app/observers/issue_observer.rb @@ -3,7 +3,7 @@ class IssueObserver < ActiveRecord::Observer def after_create(issue) if issue.assignee && issue.assignee != current_user - Notify.new_issue_email(issue.id).deliver + Notify.delay.new_issue_email(issue.id) end end @@ -16,7 +16,7 @@ class IssueObserver < ActiveRecord::Observer if status Note.create_status_change_note(issue, current_user, status) [issue.author, issue.assignee].compact.each do |recipient| - Notify.issue_status_changed_email(recipient.id, issue.id, status, current_user.id).deliver + Notify.delay.issue_status_changed_email(recipient.id, issue.id, status, current_user.id) end end end @@ -27,7 +27,7 @@ class IssueObserver < ActiveRecord::Observer recipient_ids = [issue.assignee_id, issue.assignee_id_was].keep_if {|id| id && id != current_user.id } recipient_ids.each do |recipient_id| - Notify.reassigned_issue_email(recipient_id, issue.id, issue.assignee_id_was).deliver + Notify.delay.reassigned_issue_email(recipient_id, issue.id, issue.assignee_id_was) end end end diff --git a/app/observers/merge_request_observer.rb b/app/observers/merge_request_observer.rb index c4040f1542d..6d3c2bdd186 100644 --- a/app/observers/merge_request_observer.rb +++ b/app/observers/merge_request_observer.rb @@ -3,7 +3,7 @@ class MergeRequestObserver < ActiveRecord::Observer def after_create(merge_request) if merge_request.assignee && merge_request.assignee != current_user - Notify.new_merge_request_email(merge_request.id).deliver + Notify.delay.new_merge_request_email(merge_request.id) end end @@ -25,7 +25,7 @@ class MergeRequestObserver < ActiveRecord::Observer recipients_ids.delete current_user.id recipients_ids.each do |recipient_id| - Notify.reassigned_merge_request_email(recipient_id, merge_request.id, merge_request.assignee_id_was).deliver + Notify.delay.reassigned_merge_request_email(recipient_id, merge_request.id, merge_request.assignee_id_was) end end end diff --git a/app/observers/note_observer.rb b/app/observers/note_observer.rb index fe01efcaac2..3f6d1dfcb70 100644 --- a/app/observers/note_observer.rb +++ b/app/observers/note_observer.rb @@ -11,7 +11,7 @@ class NoteObserver < ActiveRecord::Observer notify_team(note) elsif note.notify_author # Notify only author of resource - Notify.note_commit_email(note.commit_author.id, note.id).deliver + Notify.delay.note_commit_email(note.commit_author.id, note.id) else # Otherwise ignore it nil @@ -26,7 +26,7 @@ class NoteObserver < ActiveRecord::Observer if Notify.respond_to? notify_method team_without_note_author(note).map do |u| - Notify.send(notify_method, u.id, note.id).deliver + Notify.delay.send(notify_method, u.id, note.id) end end end diff --git a/app/observers/user_observer.rb b/app/observers/user_observer.rb index 73a1d00ca3b..c1179ed7881 100644 --- a/app/observers/user_observer.rb +++ b/app/observers/user_observer.rb @@ -2,7 +2,7 @@ class UserObserver < ActiveRecord::Observer def after_create(user) log_info("User \"#{user.name}\" (#{user.email}) was created") - Notify.new_user_email(user.id, user.password).deliver + Notify.delay.new_user_email(user.id, user.password) end def after_destroy user diff --git a/app/observers/users_project_observer.rb b/app/observers/users_project_observer.rb index 0c9c2b2614a..b969d6a13ef 100644 --- a/app/observers/users_project_observer.rb +++ b/app/observers/users_project_observer.rb @@ -1,7 +1,7 @@ class UsersProjectObserver < ActiveRecord::Observer def after_commit(users_project) return if users_project.destroyed? - Notify.project_access_granted_email(users_project.id).deliver + Notify.delay.project_access_granted_email(users_project.id) end def after_create(users_project) diff --git a/app/views/admin/dashboard/index.html.haml b/app/views/admin/dashboard/index.html.haml index 672dad4377e..2e572aaba79 100644 --- a/app/views/admin/dashboard/index.html.haml +++ b/app/views/admin/dashboard/index.html.haml @@ -1,5 +1,5 @@ .admin_dash.row - .span3 + .span4 .ui-box %h5.title Projects .data.padded @@ -7,7 +7,7 @@ %h1= Project.count %hr = link_to 'New Project', new_project_path, class: "btn small" - .span3 + .span4 .ui-box %h5.title Groups .data.padded @@ -15,7 +15,7 @@ %h1= Group.count %hr = link_to 'New Group', new_admin_group_path, class: "btn small" - .span3 + .span4 .ui-box %h5.title Users .data.padded @@ -23,25 +23,6 @@ %h1= User.count %hr = link_to 'New User', new_admin_user_path, class: "btn small" - .span3 - .ui-box - %h5.title - Resque Workers - .data.padded - - if @resque_accessible - = link_to admin_resque_path do - %h1{class: @workers.present? ? "cgreen" : "cred"} - = @workers.count - %hr - %p - %strong{class: @pending_jobs > 0 ? "cred" : "cgreen"} - #{@pending_jobs} post receive jobs waiting - - else - = link_to admin_resque_path do - %h1.cdark ? - %hr - %p - %strong Resque status unknown .row .span6 diff --git a/app/views/admin/resque/show.html.haml b/app/views/admin/resque/show.html.haml index 41254a6b6c2..499738f9a06 100644 --- a/app/views/admin/resque/show.html.haml +++ b/app/views/admin/resque/show.html.haml @@ -1,4 +1,4 @@ -%h3.page_title Resque +%h3.page_title Background Jobs %br .ui-box - %iframe{src: resque_path, width: '100%', height: 600, style: "border: none"} + %iframe{src: sidekiq_path, width: '100%', height: 900, style: "border: none"} diff --git a/app/views/layouts/admin.html.haml b/app/views/layouts/admin.html.haml index 6b643ec8ccb..a60e7febe76 100644 --- a/app/views/layouts/admin.html.haml +++ b/app/views/layouts/admin.html.haml @@ -19,6 +19,6 @@ = nav_link(controller: :hooks) do = link_to "Hooks", admin_hooks_path = nav_link(controller: :resque) do - = link_to "Resque", admin_resque_path + = link_to "Background Jobs", admin_resque_path .content= yield diff --git a/app/workers/post_receive.rb b/app/workers/post_receive.rb index 9e3f32f835d..e74379a65dd 100644 --- a/app/workers/post_receive.rb +++ b/app/workers/post_receive.rb @@ -1,7 +1,9 @@ class PostReceive - @queue = :post_receive + include Sidekiq::Worker - def self.perform(repo_path, oldrev, newrev, ref, identifier) + sidekiq_options queue: :post_receive + + def perform(repo_path, oldrev, newrev, ref, identifier) repo_path.gsub!(Gitlab.config.gitolite.repos_path.to_s, "") repo_path.gsub!(/.git$/, "") repo_path.gsub!(/^\//, "") diff --git a/app/workers/system_hook_worker.rb b/app/workers/system_hook_worker.rb index ca154136b97..3ebc62b7e7a 100644 --- a/app/workers/system_hook_worker.rb +++ b/app/workers/system_hook_worker.rb @@ -1,7 +1,9 @@ class SystemHookWorker - @queue = :system_hook + include Sidekiq::Worker - def self.perform(hook_id, data) + sidekiq_options queue: :system_hook + + def perform(hook_id, data) SystemHook.find(hook_id).execute data end end diff --git a/config/initializers/4_resque.rb b/config/initializers/4_resque.rb deleted file mode 100644 index ab34050cb50..00000000000 --- a/config/initializers/4_resque.rb +++ /dev/null @@ -1,29 +0,0 @@ -# Custom Redis configuration -config_file = Rails.root.join('config', 'resque.yml') - -if File.exists?(config_file) - resque_config = YAML.load_file(config_file) - Resque.redis = resque_config[Rails.env] -end -Resque.redis.namespace = 'resque:gitlab' -Resque.before_fork = Proc.new { ActiveRecord::Base.establish_connection } - -# Authentication -require 'resque/server' -class ResqueAuthentication - def initialize(app) - @app = app - end - - def call(env) - account = env['warden'].authenticate!(:database_authenticatable, :rememberable, scope: :user) - raise "Access denied" if !account.admin? - @app.call(env) - end -end - -Resque::Server.use ResqueAuthentication - -# Mailer -Resque::Mailer.excluded_environments = [] - diff --git a/config/initializers/4_sidekiq.rb b/config/initializers/4_sidekiq.rb new file mode 100644 index 00000000000..6abe6e74fa0 --- /dev/null +++ b/config/initializers/4_sidekiq.rb @@ -0,0 +1,22 @@ +# Custom Redis configuration +config_file = Rails.root.join('config', 'resque.yml') + +resque_url = if File.exists?(config_file) + YAML.load_file(config_file)[Rails.env] + else + "localhost:6379" + end + +Sidekiq.configure_server do |config| + config.redis = { + url: "redis://#{resque_url}", + namespace: 'resque:gitlab' + } +end + +Sidekiq.configure_client do |config| + config.redis = { + url: "redis://#{resque_url}", + namespace: 'resque:gitlab' + } +end diff --git a/config/routes.rb b/config/routes.rb index 27188b3b89f..ffcfd1ee6f5 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,3 +1,5 @@ +require 'sidekiq/web' + Gitlab::Application.routes.draw do # # Search @@ -8,9 +10,10 @@ Gitlab::Application.routes.draw do require 'api' mount Gitlab::API => '/api' - # Optionally, enable Resque here - require 'resque/server' - mount Resque::Server => '/info/resque', as: 'resque' + constraint = lambda { |request| request.env["warden"].authenticate? and request.env['warden'].user.admin? } + constraints constraint do + mount Sidekiq::Web, at: "/admin/sidekiq", as: :sidekiq + end # Enable Grack support mount Grack::Bundle.new({ diff --git a/features/steps/admin/admin_active_tab.rb b/features/steps/admin/admin_active_tab.rb index 05a9a686e01..48ec7bac0d6 100644 --- a/features/steps/admin/admin_active_tab.rb +++ b/features/steps/admin/admin_active_tab.rb @@ -28,6 +28,6 @@ class AdminActiveTab < Spinach::FeatureSteps end Then 'the active main tab should be Resque' do - ensure_active_main_tab('Resque') + ensure_active_main_tab('Background Jobs') end end diff --git a/features/support/env.rb b/features/support/env.rb index 72531718e0c..be10ad1b8b9 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -6,6 +6,8 @@ require './config/environment' require 'rspec' require 'database_cleaner' require 'spinach/capybara' +require 'sidekiq/testing/inline' + %w(gitolite_stub stubbed_repository valid_commit).each do |f| require Rails.root.join('spec', 'support', f) diff --git a/lib/hooks/post-receive b/lib/hooks/post-receive index ebd9e1a028a..6944d3e3f72 100755 --- a/lib/hooks/post-receive +++ b/lib/hooks/post-receive @@ -1,5 +1,6 @@ #!/usr/bin/env bash +# Version 4.1 # This file was placed here by GitLab. It makes sure that your pushed commits # will be processed properly. diff --git a/lib/tasks/gitlab/check.rake b/lib/tasks/gitlab/check.rake index 252b508609a..db181cb9a96 100644 --- a/lib/tasks/gitlab/check.rake +++ b/lib/tasks/gitlab/check.rake @@ -871,7 +871,7 @@ namespace :gitlab do namespace :resque do - desc "GITLAB | Check the configuration of Resque" + desc "GITLAB | Check the configuration of Sidekiq" task check: :environment do warn_user_is_not_gitlab start_checking "Resque" @@ -888,7 +888,7 @@ namespace :gitlab do def check_resque_running print "Running? ... " - if run_and_match("ps aux | grep -i resque", /resque-[\d\.]+:.+$/) + if run_and_match("ps aux | grep -i sidekiq", /sidekiq-[\d\.]+:.+$/) puts "yes".green else puts "no".red @@ -899,7 +899,7 @@ namespace :gitlab do ) for_more_information( see_installation_guide_section("Install Init Script"), - "see log/resque.log for possible errors" + "see log/sidekiq.log for possible errors" ) fix_and_rerun end diff --git a/lib/tasks/resque.rake b/lib/tasks/resque.rake deleted file mode 100644 index 0c3b93c5bed..00000000000 --- a/lib/tasks/resque.rake +++ /dev/null @@ -1,23 +0,0 @@ -require 'resque/tasks' - -namespace :resque do - task setup: :environment do - Resque.before_fork = Proc.new { ActiveRecord::Base.establish_connection } - end - - desc "Resque | kill all workers (using -QUIT), god will take care of them" - task :stop_workers => :environment do - pids = Array.new - - Resque.workers.each do |worker| - pids << worker.to_s.split(/:/).second - end - - if pids.size > 0 - system("kill -QUIT #{pids.join(' ')}") - end - end -end - -desc "Alias for resque:work (To run workers on Heroku)" -task "jobs:work" => "resque:work" diff --git a/lib/tasks/sidekiq.rake b/lib/tasks/sidekiq.rake new file mode 100644 index 00000000000..6bbcb3da4bc --- /dev/null +++ b/lib/tasks/sidekiq.rake @@ -0,0 +1,23 @@ +namespace :sidekiq do + desc "GITLAB | Stop sidekiq" + task :stop do + run "bundle exec sidekiqctl stop #{pidfile}" + end + + desc "GITLAB | Start sidekiq" + task :start do + run "nohup bundle exec sidekiq -q post_receive,mailer,system_hook,common,default -e #{rails_env} -P #{pidfile} >> #{root_path}/log/sidekiq.log 2>&1 &" + end + + def root_path + @root_path ||= File.join(File.expand_path(File.dirname(__FILE__)), "../..") + end + + def pidfile + "#{root_path}/tmp/pids/sidekiq.pid" + end + + def rails_env + ENV['RAILS_ENV'] || "production" + end +end diff --git a/resque.sh b/resque.sh deleted file mode 100755 index 145440fa814..00000000000 --- a/resque.sh +++ /dev/null @@ -1,2 +0,0 @@ -mkdir -p tmp/pids -nohup bundle exec rake environment resque:work QUEUE=post_receive,mailer,system_hook RAILS_ENV=production PIDFILE=tmp/pids/resque_worker.pid > ./log/resque.stdout.log 2>./log/resque.stderr.log &
\ No newline at end of file diff --git a/resque_dev.sh b/resque_dev.sh deleted file mode 100755 index b250caa9aee..00000000000 --- a/resque_dev.sh +++ /dev/null @@ -1,2 +0,0 @@ -mkdir -p tmp/pids -nohup bundle exec rake environment resque:work QUEUE=post_receive,mailer,system_hook VVERBOSE=1 RAILS_ENV=development PIDFILE=tmp/pids/resque_worker.pid > ./log/resque.log & diff --git a/spec/models/system_hook_spec.rb b/spec/models/system_hook_spec.rb index cc358a2ed3b..a9ed6a5fa7c 100644 --- a/spec/models/system_hook_spec.rb +++ b/spec/models/system_hook_spec.rb @@ -23,41 +23,31 @@ describe SystemHook do end it "project_create hook" do - with_resque do - project = create(:project) - end + project = create(:project) WebMock.should have_requested(:post, @system_hook.url).with(body: /project_create/).once end it "project_destroy hook" do project = create(:project) - with_resque do - project.destroy - end + project.destroy WebMock.should have_requested(:post, @system_hook.url).with(body: /project_destroy/).once end it "user_create hook" do - with_resque do - create(:user) - end + create(:user) WebMock.should have_requested(:post, @system_hook.url).with(body: /user_create/).once end it "user_destroy hook" do user = create(:user) - with_resque do - user.destroy - end + user.destroy WebMock.should have_requested(:post, @system_hook.url).with(body: /user_destroy/).once end it "project_create hook" do user = create(:user) project = create(:project) - with_resque do - project.team << [user, :master] - end + project.team << [user, :master] WebMock.should have_requested(:post, @system_hook.url).with(body: /user_add_to_team/).once end @@ -65,11 +55,8 @@ describe SystemHook do user = create(:user) project = create(:project) project.team << [user, :master] - with_resque do - project.users_projects.clear - end + project.users_projects.clear WebMock.should have_requested(:post, @system_hook.url).with(body: /user_remove_from_team/).once end end - end diff --git a/spec/observers/issue_observer_spec.rb b/spec/observers/issue_observer_spec.rb index bbffbd342db..700c9a3a69b 100644 --- a/spec/observers/issue_observer_spec.rb +++ b/spec/observers/issue_observer_spec.rb @@ -21,8 +21,7 @@ describe IssueObserver do end it 'sends an email to the assignee' do - Notify.should_receive(:new_issue_email).with(issue.id). - and_return(double(deliver: true)) + Notify.should_receive(:new_issue_email).with(issue.id) subject.after_create(issue) end @@ -71,6 +70,7 @@ describe IssueObserver do context 'a status "closed"' do it 'note is created if the issue is being closed' do issue.should_receive(:is_being_closed?).and_return(true) + Notify.should_receive(:issue_status_changed_email).twice Note.should_receive(:create_status_change_note).with(issue, some_user, 'closed') subject.after_update(issue) @@ -85,7 +85,7 @@ describe IssueObserver do it 'notification is delivered if the issue being closed' do issue.stub(:is_being_closed?).and_return(true) - Notify.should_receive(:issue_status_changed_email).twice.and_return(stub(deliver: true)) + Notify.should_receive(:issue_status_changed_email).twice Note.should_receive(:create_status_change_note).with(issue, some_user, 'closed') subject.after_update(issue) @@ -104,7 +104,7 @@ describe IssueObserver do issue_without_assignee.stub(:is_being_reassigned?).and_return(false) issue_without_assignee.stub(:is_being_closed?).and_return(true) issue_without_assignee.stub(:is_being_reopened?).and_return(false) - Notify.should_receive(:issue_status_changed_email).once.and_return(stub(deliver: true)) + Notify.should_receive(:issue_status_changed_email).once Note.should_receive(:create_status_change_note).with(issue_without_assignee, some_user, 'closed') subject.after_update(issue_without_assignee) @@ -113,6 +113,7 @@ describe IssueObserver do context 'a status "reopened"' do it 'note is created if the issue is being reopened' do + Notify.should_receive(:issue_status_changed_email).twice issue.should_receive(:is_being_reopened?).and_return(true) Note.should_receive(:create_status_change_note).with(issue, some_user, 'reopened') @@ -128,7 +129,7 @@ describe IssueObserver do it 'notification is delivered if the issue being reopened' do issue.stub(:is_being_reopened?).and_return(true) - Notify.should_receive(:issue_status_changed_email).twice.and_return(stub(deliver: true)) + Notify.should_receive(:issue_status_changed_email).twice Note.should_receive(:create_status_change_note).with(issue, some_user, 'reopened') subject.after_update(issue) @@ -147,7 +148,7 @@ describe IssueObserver do issue_without_assignee.stub(:is_being_reassigned?).and_return(false) issue_without_assignee.stub(:is_being_closed?).and_return(false) issue_without_assignee.stub(:is_being_reopened?).and_return(true) - Notify.should_receive(:issue_status_changed_email).once.and_return(stub(deliver: true)) + Notify.should_receive(:issue_status_changed_email).once Note.should_receive(:create_status_change_note).with(issue_without_assignee, some_user, 'reopened') subject.after_update(issue_without_assignee) @@ -164,8 +165,7 @@ describe IssueObserver do end def it_sends_a_reassigned_email_to(recipient) - Notify.should_receive(:reassigned_issue_email).with(recipient, issue.id, previous_assignee.id). - and_return(double(deliver: true)) + Notify.should_receive(:reassigned_issue_email).with(recipient, issue.id, previous_assignee.id) end def it_does_not_send_a_reassigned_email_to(recipient) diff --git a/spec/observers/merge_request_observer_spec.rb b/spec/observers/merge_request_observer_spec.rb index 6b15be96630..4841bf88fc5 100644 --- a/spec/observers/merge_request_observer_spec.rb +++ b/spec/observers/merge_request_observer_spec.rb @@ -21,9 +21,7 @@ describe MergeRequestObserver do end it 'sends an email to the assignee' do - Notify.should_receive(:new_merge_request_email).with(mr.id). - and_return(double(deliver: true)) - + Notify.should_receive(:new_merge_request_email).with(mr.id) subject.after_create(mr) end @@ -158,8 +156,7 @@ describe MergeRequestObserver do end def it_sends_a_reassigned_email_to(recipient) - Notify.should_receive(:reassigned_merge_request_email).with(recipient, mr.id, previous_assignee.id). - and_return(double(deliver: true)) + Notify.should_receive(:reassigned_merge_request_email).with(recipient, mr.id, previous_assignee.id) end def it_does_not_send_a_reassigned_email_to(recipient) diff --git a/spec/observers/note_observer_spec.rb b/spec/observers/note_observer_spec.rb index 7dfa9f772e9..73da2e146cb 100644 --- a/spec/observers/note_observer_spec.rb +++ b/spec/observers/note_observer_spec.rb @@ -4,7 +4,6 @@ describe NoteObserver do subject { NoteObserver.instance } let(:team_without_author) { (1..2).map { |n| double :user, id: n } } - let(:delivery_success) { double deliver: true } describe '#after_create' do let(:note) { double :note } @@ -45,13 +44,13 @@ describe NoteObserver do note.stub(:id).and_return(42) author = double :user, id: 1 note.stub(:commit_author).and_return(author) - Notify.should_receive(:note_commit_email).and_return(delivery_success) + Notify.should_receive(:note_commit_email) subject.after_create(note) end it 'does not notify the author of a commit when not flagged to notify the author' do - Notify.should_not_receive(:note_commit_email) + notify.should_not_receive(:note_commit_email) subject.after_create(note) end @@ -71,28 +70,28 @@ describe NoteObserver do context 'notifies team of a new note on' do it 'a commit' do note.stub(:noteable_type).and_return('Commit') - Notify.should_receive(:note_commit_email).twice.and_return(delivery_success) + notify.should_receive(:note_commit_email).twice subject.send(:notify_team, note) end it 'an issue' do note.stub(:noteable_type).and_return('Issue') - Notify.should_receive(:note_issue_email).twice.and_return(delivery_success) + notify.should_receive(:note_issue_email).twice subject.send(:notify_team, note) end it 'a wiki page' do note.stub(:noteable_type).and_return('Wiki') - Notify.should_receive(:note_wiki_email).twice.and_return(delivery_success) + notify.should_receive(:note_wiki_email).twice subject.send(:notify_team, note) end it 'a merge request' do note.stub(:noteable_type).and_return('MergeRequest') - Notify.should_receive(:note_merge_request_email).twice.and_return(delivery_success) + notify.should_receive(:note_merge_request_email).twice subject.send(:notify_team, note) end @@ -100,16 +99,16 @@ describe NoteObserver do it 'a wall' do # Note: wall posts have #noteable_type of nil note.stub(:noteable_type).and_return(nil) - Notify.should_receive(:note_wall_email).twice.and_return(delivery_success) + notify.should_receive(:note_wall_email).twice subject.send(:notify_team, note) end end it 'does nothing for a new note on a snippet' do - note.stub(:noteable_type).and_return('Snippet') + note.stub(:noteable_type).and_return('Snippet') - subject.send(:notify_team, note).should be_nil + subject.send(:notify_team, note).should be_nil end end @@ -125,4 +124,8 @@ describe NoteObserver do subject.send(:team_without_note_author, note).should == team_without_author end end + + def notify + Notify + end end diff --git a/spec/observers/user_observer_spec.rb b/spec/observers/user_observer_spec.rb index befb980fdcb..bffa5fcfd69 100644 --- a/spec/observers/user_observer_spec.rb +++ b/spec/observers/user_observer_spec.rb @@ -10,24 +10,14 @@ describe UserObserver do end context 'when a new user is created' do - let(:user) { double(:user, id: 42, - password: 'P@ssword!', - name: 'John', - email: 'u@mail.local', - username: 'root', - create_namespace: true) } - let(:notification) { double :notification } - it 'sends an email' do - notification.should_receive(:deliver) - Notify.should_receive(:new_user_email).with(user.id, user.password).and_return(notification) - - subject.after_create(user) + Notify.should_receive(:new_user_email) + create(:user) end it 'trigger logger' do Gitlab::AppLogger.should_receive(:info) - subject.after_create(user) + create(:user) end end end diff --git a/spec/requests/admin/admin_users_spec.rb b/spec/requests/admin/admin_users_spec.rb index ca134c2d4f7..a66e85a34f9 100644 --- a/spec/requests/admin/admin_users_spec.rb +++ b/spec/requests/admin/admin_users_spec.rb @@ -41,7 +41,7 @@ describe "Admin::Users" do end it "should call send mail" do - Notify.should_receive(:new_user_email).and_return(stub(deliver: true)) + Notify.should_receive(:new_user_email) User.observers.enable :user_observer do click_button "Save" @@ -50,9 +50,7 @@ describe "Admin::Users" do it "should send valid email to user with email & password" do User.observers.enable :user_observer do - with_resque do - click_button "Save" - end + click_button "Save" user = User.last email = ActionMailer::Base.deliveries.last email.subject.should have_content("Account was created") diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index edabcc33625..dbac3c54901 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,5 +1,6 @@ require 'simplecov' unless ENV['CI'] + # This file is copied to spec/ when you run 'rails generate rspec:install' ENV["RAILS_ENV"] ||= 'test' require File.expand_path("../../config/environment", __FILE__) @@ -8,6 +9,7 @@ require 'capybara/rails' require 'capybara/rspec' require 'webmock/rspec' require 'email_spec' +require 'sidekiq/testing/inline' # Requires supporting ruby files with custom matchers and macros, etc, # in spec/support/ and its subdirectories. diff --git a/spec/workers/post_receive_spec.rb b/spec/workers/post_receive_spec.rb index 26b461c3825..f408c89afdd 100644 --- a/spec/workers/post_receive_spec.rb +++ b/spec/workers/post_receive_spec.rb @@ -4,7 +4,7 @@ describe PostReceive do context "as a resque worker" do it "reponds to #perform" do - PostReceive.should respond_to(:perform) + PostReceive.new.should respond_to(:perform) end end @@ -15,7 +15,7 @@ describe PostReceive do it "fetches the correct project" do Project.should_receive(:find_with_namespace).with(project.path_with_namespace).and_return(project) - PostReceive.perform(pwd(project), 'sha-old', 'sha-new', 'refs/heads/master', key_id) + PostReceive.new.perform(pwd(project), 'sha-old', 'sha-new', 'refs/heads/master', key_id) end it "does not run if the author is not in the project" do @@ -24,7 +24,7 @@ describe PostReceive do project.should_not_receive(:observe_push) project.should_not_receive(:execute_hooks) - PostReceive.perform(pwd(project), 'sha-old', 'sha-new', 'refs/heads/master', key_id).should be_false + PostReceive.new.perform(pwd(project), 'sha-old', 'sha-new', 'refs/heads/master', key_id).should be_false end it "asks the project to trigger all hooks" do @@ -34,7 +34,7 @@ describe PostReceive do project.should_receive(:update_merge_requests) project.should_receive(:observe_push) - PostReceive.perform(pwd(project), 'sha-old', 'sha-new', 'refs/heads/master', key_id) + PostReceive.new.perform(pwd(project), 'sha-old', 'sha-new', 'refs/heads/master', key_id) end end |