From 95edf32e38184747089c9c2344fcb8c195db6aa9 Mon Sep 17 00:00:00 2001 From: Scott Albertson Date: Fri, 18 Apr 2014 08:18:54 -0700 Subject: [PATCH] Use Resque instead of DelayedJob https://trello.com/c/adsxS8Sv/197-use-resque * Create JobQueue to abstract queue choice --- Gemfile | 5 +- Gemfile.lock | 40 +++++++++-- Procfile | 2 +- .../repos_controller.js.coffee.erb | 10 +-- .../javascripts/services/user.js.coffee | 3 + app/controllers/builds_controller.rb | 33 +++------ app/controllers/repo_syncs_controller.rb | 19 ++---- app/controllers/users_controller.rb | 7 ++ app/jobs/build_job.rb | 20 ------ app/jobs/buildable.rb | 11 +++ app/jobs/large_build_job.rb | 8 +++ app/jobs/monitorable.rb | 13 ---- app/jobs/repo_synchronization_job.rb | 16 ++++- app/jobs/retryable.rb | 9 +++ app/jobs/small_build_job.rb | 8 +++ app/models/payload.rb | 26 ++++--- app/serializers/user_serializer.rb | 3 + config.ru | 1 - config/environments/test.rb | 1 + .../initializers/active_model_serializer.rb | 2 + config/initializers/delayed_job_config.rb | 2 - config/initializers/redis.rb | 2 + config/initializers/resque.rb | 16 +++++ config/routes.rb | 5 +- ...419160756_add_refreshing_repos_to_users.rb | 5 ++ db/schema.rb | 9 +-- lib/job_queue.rb | 7 ++ lib/tasks/resque.rake | 3 + spec/controllers/builds_controller_spec.rb | 28 +++++--- .../controllers/repo_syncs_controller_spec.rb | 17 +++++ spec/controllers/users_controller_spec.rb | 18 +++++ spec/features/repo_list_spec.rb | 4 +- spec/jobs/build_job_spec.rb | 67 ------------------- spec/jobs/buildable_spec.rb | 36 ++++++++++ spec/jobs/large_build_job_spec.rb | 14 ++++ spec/jobs/monitorable_spec.rb | 22 ------ spec/jobs/repo_synchronization_job_spec.rb | 54 +++++++++++---- spec/jobs/retryable_spec.rb | 0 spec/jobs/small_build_job_spec.rb | 14 ++++ spec/lib/job_queue_spec.rb | 15 +++++ spec/models/payload_spec.rb | 9 +++ spec/spec_helper.rb | 2 +- 42 files changed, 365 insertions(+), 221 deletions(-) create mode 100644 app/assets/javascripts/services/user.js.coffee create mode 100644 app/controllers/users_controller.rb delete mode 100644 app/jobs/build_job.rb create mode 100644 app/jobs/buildable.rb create mode 100644 app/jobs/large_build_job.rb delete mode 100644 app/jobs/monitorable.rb create mode 100644 app/jobs/retryable.rb create mode 100644 app/jobs/small_build_job.rb create mode 100644 app/serializers/user_serializer.rb create mode 100644 config/initializers/active_model_serializer.rb delete mode 100644 config/initializers/delayed_job_config.rb create mode 100644 config/initializers/redis.rb create mode 100644 config/initializers/resque.rb create mode 100644 db/migrate/20140419160756_add_refreshing_repos_to_users.rb create mode 100644 lib/job_queue.rb create mode 100644 lib/tasks/resque.rake create mode 100644 spec/controllers/repo_syncs_controller_spec.rb create mode 100644 spec/controllers/users_controller_spec.rb delete mode 100644 spec/jobs/build_job_spec.rb create mode 100644 spec/jobs/buildable_spec.rb create mode 100644 spec/jobs/large_build_job_spec.rb delete mode 100644 spec/jobs/monitorable_spec.rb create mode 100644 spec/jobs/retryable_spec.rb create mode 100644 spec/jobs/small_build_job_spec.rb create mode 100644 spec/lib/job_queue_spec.rb diff --git a/Gemfile b/Gemfile index fc4630961..a1e7d3728 100644 --- a/Gemfile +++ b/Gemfile @@ -2,11 +2,11 @@ source 'https://rubygems.org' ruby '2.0.0' +gem 'active_model_serializers' gem 'angularjs-rails' gem 'bourbon' gem 'coffee-rails' gem 'dalli' -gem 'delayed_job_active_record' gem 'faraday-http-cache' gem 'font-awesome-rails' gem 'haml-rails' @@ -19,6 +19,9 @@ gem 'octokit' gem 'omniauth-github' gem 'pg' gem 'rails', '4.0.4' +gem 'resque', '~> 1.22.0' +gem 'resque-retry' +gem 'resque-sentry' gem 'rubocop' gem 'sass-rails', '~> 4.0.2' gem 'sentry-raven' diff --git a/Gemfile.lock b/Gemfile.lock index 57db6dd70..e5aaca487 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -10,6 +10,8 @@ GEM erubis (~> 2.7.0) rack (~> 1.5.2) rack-test (~> 0.6.2) + active_model_serializers (0.8.1) + activemodel (>= 3.0) activemodel (4.0.4) activesupport (= 4.0.4) builder (~> 3.1.0) @@ -61,11 +63,6 @@ GEM debugger-ruby_core_source (~> 1.3.1) debugger-linecache (1.2.0) debugger-ruby_core_source (1.3.1) - delayed_job (4.0.0) - activesupport (>= 3.0, < 4.1) - delayed_job_active_record (4.0.0) - activerecord (>= 3.0, < 4.1) - delayed_job (>= 3.0, < 4.1) diff-lcs (1.2.4) dotenv (0.9.0) erubis (2.7.0) @@ -144,6 +141,8 @@ GEM polyglot (0.3.4) powerpack (0.0.9) rack (1.5.2) + rack-protection (1.5.3) + rack rack-test (0.6.2) rack (>= 1.0) rails (4.0.4) @@ -167,6 +166,24 @@ GEM rainbow (2.0.0) raindrops (0.12.0) rake (10.1.1) + redis (3.0.7) + redis-namespace (1.4.1) + redis (~> 3.0.4) + resque (1.22.0) + multi_json (~> 1.0) + redis-namespace (~> 1.0) + sinatra (>= 0.9.2) + vegas (~> 0.1.2) + resque-retry (1.0.0) + resque (>= 1.10.0) + resque-scheduler (>= 1.9.9) + resque-scheduler (2.2.0) + redis (>= 3.0.0) + resque (>= 1.20.0, < 1.25) + rufus-scheduler (~> 2.0) + resque-sentry (1.2.0) + resque (>= 1.18.0) + sentry-raven (>= 0.4.6) rspec-core (2.14.7) rspec-expectations (2.14.3) diff-lcs (>= 1.1.3, < 2.0) @@ -185,6 +202,8 @@ GEM rainbow (>= 1.99.1, < 3.0) ruby-progressbar (~> 1.4) ruby-progressbar (1.4.2) + rufus-scheduler (2.0.24) + tzinfo (>= 0.3.22) safe_yaml (0.9.7) sass (3.2.17) sass-rails (4.0.2) @@ -201,6 +220,10 @@ GEM uuidtools shoulda-matchers (2.4.0) activesupport (>= 3.0.0) + sinatra (1.4.5) + rack (~> 1.4) + rack-protection (~> 1.4) + tilt (~> 1.3, >= 1.3.4) slop (3.5.0) sprockets (2.11.0) hike (~> 1.2) @@ -227,6 +250,8 @@ GEM rack raindrops (~> 0.7) uuidtools (2.1.4) + vegas (0.1.11) + rack (>= 1.0.0) webmock (1.15.2) addressable (>= 2.2.7) crack (>= 0.3.2) @@ -237,6 +262,7 @@ PLATFORMS ruby DEPENDENCIES + active_model_serializers angularjs-rails bourbon capybara (~> 2.1.0) @@ -245,7 +271,6 @@ DEPENDENCIES dalli database_cleaner debugger - delayed_job_active_record factory_girl_rails faraday-http-cache font-awesome-rails @@ -262,6 +287,9 @@ DEPENDENCIES pg rails (= 4.0.4) rails_12factor + resque (~> 1.22.0) + resque-retry + resque-sentry rspec-rails (>= 2.14) rubocop sass-rails (~> 4.0.2) diff --git a/Procfile b/Procfile index 793489725..b4f7a0b65 100644 --- a/Procfile +++ b/Procfile @@ -1,2 +1,2 @@ web: bundle exec unicorn -p $PORT -c ./config/unicorn.rb -worker: bundle exec rake jobs:work +resque: env TERM_CHILD=1 RESQUE_TERM_TIMEOUT=8 bundle exec rake resque:work diff --git a/app/assets/javascripts/controllers/repos_controller.js.coffee.erb b/app/assets/javascripts/controllers/repos_controller.js.coffee.erb index 7eec4b988..ce164c9ab 100644 --- a/app/assets/javascripts/controllers/repos_controller.js.coffee.erb +++ b/app/assets/javascripts/controllers/repos_controller.js.coffee.erb @@ -1,4 +1,4 @@ -App.controller 'ReposController', ['$scope', '$timeout', 'Repo', 'Sync', ($scope, $timeout, Repo, Sync) -> +App.controller 'ReposController', ['$scope', '$timeout', 'Repo', 'Sync', 'User', ($scope, $timeout, Repo, Sync, User) -> loadRepos = -> $scope.syncingRepos = false @@ -15,8 +15,8 @@ App.controller 'ReposController', ['$scope', '$timeout', 'Repo', 'Sync', ($scope $scope.sync() pollSyncStatus = -> - successfulSync = (results) -> - if results.length > 0 + successfulSync = (user) -> + if user.refreshing_repos pollSyncStatus() else loadRepos() @@ -25,8 +25,8 @@ App.controller 'ReposController', ['$scope', '$timeout', 'Repo', 'Sync', ($scope pollSyncStatus() getSyncs = -> - syncs = Sync.query() - syncs.$promise.then(successfulSync, failedSync) + user = User.get() + user.$promise.then(successfulSync, failedSync) $timeout getSyncs, 3000 diff --git a/app/assets/javascripts/services/user.js.coffee b/app/assets/javascripts/services/user.js.coffee new file mode 100644 index 000000000..31ec68f6d --- /dev/null +++ b/app/assets/javascripts/services/user.js.coffee @@ -0,0 +1,3 @@ +App.factory 'User', ['$resource', ($resource) -> + $resource '/user' +] diff --git a/app/controllers/builds_controller.rb b/app/controllers/builds_controller.rb index cd724da85..cd729c6c3 100644 --- a/app/controllers/builds_controller.rb +++ b/app/controllers/builds_controller.rb @@ -3,11 +3,8 @@ class BuildsController < ApplicationController skip_before_filter :verify_authenticity_token, only: [:create] skip_before_filter :authenticate, only: [:create] - HIGH_PRIORITY = 1 - LOW_PRIORITY = 2 - def create - Delayed::Job.enqueue(build_job, priority: priority) + JobQueue.push(build_job_class, payload.data) head :ok end @@ -17,33 +14,21 @@ def force_https? false end - def build_job - BuildJob.new(build_runner) - end - - def build_runner - BuildRunner.new(payload) - end - - def payload - Payload.new(event_data) - end - - def event_data - JSON.parse(params[:payload] || request.raw_post) - end - def ignore_confirmation_pings - if event_data.key?('zen') + if payload.ping? head :ok end end - def priority + def build_job_class if payload.changed_files < ENV['CHANGED_FILES_THRESHOLD'].to_i - HIGH_PRIORITY + SmallBuildJob else - LOW_PRIORITY + LargeBuildJob end end + + def payload + @payload ||= Payload.new(params[:payload] || request.raw_post) + end end diff --git a/app/controllers/repo_syncs_controller.rb b/app/controllers/repo_syncs_controller.rb index b3e930dd9..deaa5be63 100644 --- a/app/controllers/repo_syncs_controller.rb +++ b/app/controllers/repo_syncs_controller.rb @@ -1,21 +1,12 @@ class RepoSyncsController < ApplicationController respond_to :json - def index - syncs = Delayed::Job.uncached do - Delayed::Job.where(<<-SQL) -handler like '%RepoSynchronizationJob%' -and handler like '%user_id: #{current_user.id}%' -and failed_at IS NULL - SQL - end - - respond_with syncs - end - def create - sync_job = RepoSynchronizationJob.new(current_user.id, session[:github_token]) - Delayed::Job.enqueue(sync_job) + JobQueue.push( + RepoSynchronizationJob, + current_user.id, + session[:github_token] + ) head 201 end end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb new file mode 100644 index 000000000..aef853fd8 --- /dev/null +++ b/app/controllers/users_controller.rb @@ -0,0 +1,7 @@ +class UsersController < ApplicationController + respond_to :json + + def show + respond_with current_user + end +end diff --git a/app/jobs/build_job.rb b/app/jobs/build_job.rb deleted file mode 100644 index dfd70a34e..000000000 --- a/app/jobs/build_job.rb +++ /dev/null @@ -1,20 +0,0 @@ -class BuildJob < Struct.new(:build_runner) - include Monitorable - - def perform - build_runner.run - end - - def error(job, exception) - super - if do_not_retry_exceptions.include?(exception.class) - job.fail! - end - end - - private - - def do_not_retry_exceptions - [Octokit::NotFound, Octokit::Unauthorized] - end -end diff --git a/app/jobs/buildable.rb b/app/jobs/buildable.rb new file mode 100644 index 000000000..2f527d9f3 --- /dev/null +++ b/app/jobs/buildable.rb @@ -0,0 +1,11 @@ +require 'resque' + +module Buildable + def perform(payload_data) + payload = Payload.new(payload_data) + build_runner = BuildRunner.new(payload) + build_runner.run + rescue Resque::TermException + Resque.enqueue(self, payload_data) + end +end diff --git a/app/jobs/large_build_job.rb b/app/jobs/large_build_job.rb new file mode 100644 index 000000000..7ae9f24ae --- /dev/null +++ b/app/jobs/large_build_job.rb @@ -0,0 +1,8 @@ +require 'octokit' + +class LargeBuildJob + extend Retryable + extend Buildable + + @queue = :low +end diff --git a/app/jobs/monitorable.rb b/app/jobs/monitorable.rb deleted file mode 100644 index e1d3ad428..000000000 --- a/app/jobs/monitorable.rb +++ /dev/null @@ -1,13 +0,0 @@ -require 'sentry-raven' - -module Monitorable - def error(job, exception) - monitor.capture_exception(exception, extra: { job_id: job.id }) - end - - private - - def monitor - Raven - end -end diff --git a/app/jobs/repo_synchronization_job.rb b/app/jobs/repo_synchronization_job.rb index 069d7ec65..4798d888e 100644 --- a/app/jobs/repo_synchronization_job.rb +++ b/app/jobs/repo_synchronization_job.rb @@ -1,9 +1,19 @@ -class RepoSynchronizationJob < Struct.new(:user_id, :github_token) - include Monitorable +class RepoSynchronizationJob + extend Retryable - def perform + @queue = :high + + def self.before_enqueue(user_id, github_token) + user = User.find(user_id) + user.update_attribute(:refreshing_repos, true) + end + + def self.perform(user_id, github_token) user = User.find(user_id) synchronization = RepoSynchronization.new(user, github_token) synchronization.start + user.update_attribute(:refreshing_repos, false) + rescue Resque::TermException + Resque.enqueue(self, user_id, github_token) end end diff --git a/app/jobs/retryable.rb b/app/jobs/retryable.rb new file mode 100644 index 000000000..d966faa23 --- /dev/null +++ b/app/jobs/retryable.rb @@ -0,0 +1,9 @@ +require 'resque-retry' + +module Retryable + extend Resque::Plugins::Retry + + @retry_limit = 10 + @retry_delay = 120 + @fatal_exceptions = [Octokit::NotFound, Octokit::Unauthorized] +end diff --git a/app/jobs/small_build_job.rb b/app/jobs/small_build_job.rb new file mode 100644 index 000000000..21e14b4f8 --- /dev/null +++ b/app/jobs/small_build_job.rb @@ -0,0 +1,8 @@ +require 'octokit' + +class SmallBuildJob + extend Retryable + extend Buildable + + @queue = :medium +end diff --git a/app/models/payload.rb b/app/models/payload.rb index 36f479376..0c5c3d982 100644 --- a/app/models/payload.rb +++ b/app/models/payload.rb @@ -1,35 +1,41 @@ require 'json' class Payload - def initialize(payload_data) - if payload_data.is_a? String - @payload_data = JSON.parse(payload_data) + attr_reader :data + + def initialize(data) + if data.is_a? String + @data = JSON.parse(data) else - @payload_data = payload_data + @data = data end end def head_sha - @payload_data['pull_request']['head']['sha'] + data['pull_request']['head']['sha'] end def github_repo_id - @payload_data['repository']['id'] + data['repository']['id'] end def full_repo_name - @payload_data['repository']['full_name'] + data['repository']['full_name'] end def number - @payload_data['number'] + data['number'] end def action - @payload_data['action'] + data['action'] end def changed_files - @payload_data['pull_request']['changed_files'] + data['pull_request']['changed_files'] + end + + def ping? + data['zen'] end end diff --git a/app/serializers/user_serializer.rb b/app/serializers/user_serializer.rb new file mode 100644 index 000000000..888f362d3 --- /dev/null +++ b/app/serializers/user_serializer.rb @@ -0,0 +1,3 @@ +class UserSerializer < ActiveModel::Serializer + attributes :id, :github_username, :refreshing_repos +end diff --git a/config.ru b/config.ru index 878b0e97a..3a51d0d35 100644 --- a/config.ru +++ b/config.ru @@ -1,4 +1,3 @@ # This file is used by Rack-based servers to start the application. - require ::File.expand_path('../config/environment', __FILE__) run Houndapp::Application diff --git a/config/environments/test.rb b/config/environments/test.rb index ce38487e5..f2b120784 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -4,6 +4,7 @@ ENV['HOUND_GITHUB_TOKEN'] = 'houndgithubtoken' ENV['ENABLE_HTTPS'] = 'no' ENV['CHANGED_FILES_THRESHOLD'] = '300' +ENV['REDISTOGO_URL'] = 'http://localhost:6379' Houndapp::Application.configure do # Settings specified here will take precedence over those in config/application.rb diff --git a/config/initializers/active_model_serializer.rb b/config/initializers/active_model_serializer.rb new file mode 100644 index 000000000..25cebeca4 --- /dev/null +++ b/config/initializers/active_model_serializer.rb @@ -0,0 +1,2 @@ +ActiveModel::Serializer.root = false +ActiveModel::ArraySerializer.root = false diff --git a/config/initializers/delayed_job_config.rb b/config/initializers/delayed_job_config.rb deleted file mode 100644 index 320ad831d..000000000 --- a/config/initializers/delayed_job_config.rb +++ /dev/null @@ -1,2 +0,0 @@ -Delayed::Worker.max_attempts = 8 -Delayed::Worker.destroy_failed_jobs = false diff --git a/config/initializers/redis.rb b/config/initializers/redis.rb new file mode 100644 index 000000000..370abaeb7 --- /dev/null +++ b/config/initializers/redis.rb @@ -0,0 +1,2 @@ +REDIS = Redis.new(url: ENV['REDISTOGO_URL']) +Resque.redis = REDIS diff --git a/config/initializers/resque.rb b/config/initializers/resque.rb new file mode 100644 index 000000000..25e2375ca --- /dev/null +++ b/config/initializers/resque.rb @@ -0,0 +1,16 @@ +require 'resque/server' +require 'resque-retry' +require 'resque-sentry' +require 'resque/failure/redis' + +Resque::Server.use(Rack::Auth::Basic) do |user, password| + password == ENV['RESQUE_ADMIN_PASSWORD'] +end + +Resque::Failure::Sentry.logger = 'resque' + +Resque::Failure::MultipleWithRetrySuppression.classes = [ + Resque::Failure::Redis, + Resque::Failure::Sentry +] +Resque::Failure.backend = Resque::Failure::MultipleWithRetrySuppression diff --git a/config/routes.rb b/config/routes.rb index 91e0646d5..89b9bde94 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,5 +1,5 @@ Houndapp::Application.routes.draw do - root to: 'home#index' + mount Resque::Server, at: '/queue' get '/auth/github/callback', to: 'sessions#create' get '/sign_in', to: 'sessions#new' @@ -12,4 +12,7 @@ resource :deactivation, only: [:create] end resources :repo_syncs, only: [:index, :create] + resource :user, only: [:show] + + root to: 'home#index' end diff --git a/db/migrate/20140419160756_add_refreshing_repos_to_users.rb b/db/migrate/20140419160756_add_refreshing_repos_to_users.rb new file mode 100644 index 000000000..c96dba186 --- /dev/null +++ b/db/migrate/20140419160756_add_refreshing_repos_to_users.rb @@ -0,0 +1,5 @@ +class AddRefreshingReposToUsers < ActiveRecord::Migration + def change + add_column :users, :refreshing_repos, :boolean, default: false + end +end diff --git a/db/schema.rb b/db/schema.rb index a4a51dade..0a18240c1 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -66,10 +66,11 @@ add_index "repos", ["github_id"], name: "index_repos_on_github_id", unique: true, using: :btree create_table "users", force: true do |t| - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.string "github_username", null: false - t.string "remember_token", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.string "github_username", null: false + t.string "remember_token", null: false + t.boolean "refreshing_repos", default: false end add_index "users", ["remember_token"], name: "index_users_on_remember_token", using: :btree diff --git a/lib/job_queue.rb b/lib/job_queue.rb new file mode 100644 index 000000000..789f23066 --- /dev/null +++ b/lib/job_queue.rb @@ -0,0 +1,7 @@ +require 'resque' + +class JobQueue + def self.push(job_class, *args) + Resque.enqueue(job_class, *args) + end +end diff --git a/lib/tasks/resque.rake b/lib/tasks/resque.rake new file mode 100644 index 000000000..4c093c8a5 --- /dev/null +++ b/lib/tasks/resque.rake @@ -0,0 +1,3 @@ +require "resque/tasks" + +task "resque:setup" => :environment diff --git a/spec/controllers/builds_controller_spec.rb b/spec/controllers/builds_controller_spec.rb index 5af38e1d3..4009d4310 100644 --- a/spec/controllers/builds_controller_spec.rb +++ b/spec/controllers/builds_controller_spec.rb @@ -12,6 +12,8 @@ context 'when https is enabled' do context 'and http is used' do it 'does not redirect' do + JobQueue.stub(:push) + with_https_enabled do payload_data = File.read( 'spec/support/fixtures/pull_request_opened_event.json' @@ -25,28 +27,34 @@ end context 'when number of changed files is below the threshold' do - it 'enqueues job with high priority' do - Delayed::Job.stub(:enqueue) + it 'enqueues small build job' do + JobQueue.stub(:push) payload_data = File.read( 'spec/support/fixtures/pull_request_opened_event.json' ) - post(:create, payload: payload_data) - expect(Delayed::Job).to have_received(:enqueue). - with(anything, priority: BuildsController::HIGH_PRIORITY) + post :create, payload: payload_data + + expect(JobQueue).to have_received(:push).with( + SmallBuildJob, + JSON.parse(payload_data) + ) end end context 'when number of changed files is at the threshold or above' do - it 'enqueues job with low priority' do - Delayed::Job.stub(:enqueue) + it 'enqueues large build job' do + JobQueue.stub(:push) payload_data = File.read( 'spec/support/fixtures/pull_request_event_with_many_files.json' ) - post(:create, payload: payload_data) - expect(Delayed::Job).to have_received(:enqueue). - with(anything, priority: BuildsController::LOW_PRIORITY) + post :create, payload: payload_data + + expect(JobQueue).to have_received(:push).with( + LargeBuildJob, + JSON.parse(payload_data) + ) end end end diff --git a/spec/controllers/repo_syncs_controller_spec.rb b/spec/controllers/repo_syncs_controller_spec.rb new file mode 100644 index 000000000..4c5b8f44d --- /dev/null +++ b/spec/controllers/repo_syncs_controller_spec.rb @@ -0,0 +1,17 @@ +require 'spec_helper' + +describe RepoSyncsController, '#create' do + it 'enqueues repo sync job' do + user = create(:user) + stub_sign_in(user) + JobQueue.stub(:push) + + post :create + + expect(JobQueue).to have_received(:push).with( + RepoSynchronizationJob, + user.id, + AuthenticationHelper::GITHUB_TOKEN + ) + end +end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb new file mode 100644 index 000000000..d35886b2b --- /dev/null +++ b/spec/controllers/users_controller_spec.rb @@ -0,0 +1,18 @@ +require 'spec_helper' + +describe UsersController do + describe '#show' do + it 'returns current user in json' do + user = create(:user) + stub_sign_in(user) + + get :show, format: :json + + expect(response.body).to eq user_json(user) + end + end + + def user_json(user) + user.attributes.slice('id', 'github_username', 'refreshing_repos').to_json + end +end diff --git a/spec/features/repo_list_spec.rb b/spec/features/repo_list_spec.rb index b9bd98f9a..e991281bb 100644 --- a/spec/features/repo_list_spec.rb +++ b/spec/features/repo_list_spec.rb @@ -107,9 +107,9 @@ private def with_job_delay - Delayed::Worker.delay_jobs = true + Resque.inline = false yield ensure - Delayed::Worker.delay_jobs = false + Resque.inline = true end end diff --git a/spec/jobs/build_job_spec.rb b/spec/jobs/build_job_spec.rb deleted file mode 100644 index 553583ec5..000000000 --- a/spec/jobs/build_job_spec.rb +++ /dev/null @@ -1,67 +0,0 @@ -require 'fast_spec_helper' -require 'app/jobs/monitorable' -require 'app/jobs/build_job' -require 'octokit' - -describe BuildJob do - it 'is monitored' do - build_job = BuildJob.new(double) - - expect(build_job).to be_a Monitorable - end -end - -describe BuildJob, '#perform' do - it 'runs the build' do - build_runner = double(:build_runner, run: true) - build_job = BuildJob.new(build_runner) - - build_job.perform - - expect(build_runner).to have_received(:run) - end -end - -describe BuildJob, '#error' do - context 'with Octokit::NotFound exception' do - it 'sets job to failed' do - build_runner = double(:build_runner, run: true) - build_job = BuildJob.new(build_runner) - job = double(fail!: true, id: 1) - - build_job.error(job, Octokit::NotFound.new) - - expect(job).to have_received(:fail!) - end - end - - context 'with Octokit::Unauthorized exception' do - it 'sets job to failed' do - build_runner = double(:build_runner, run: true) - build_job = BuildJob.new(build_runner) - job = double(fail!: true, id: 1) - - build_job.error(job, Octokit::Unauthorized.new) - - expect(job).to have_received(:fail!) - end - end - - context 'with another exception' do - it 'does not update failed_at' do - Raven.stub(:capture_exception) - build_runner = double(:build_runner, run: true) - job = double(fail!: true, id: 1) - exception = double(:exception) - build_job = BuildJob.new(build_runner) - - build_job.error(job, exception) - - expect(Raven).to have_received(:capture_exception).with( - exception, - extra: { job_id: job.id } - ) - expect(job).not_to have_received(:fail!) - end - end -end diff --git a/spec/jobs/buildable_spec.rb b/spec/jobs/buildable_spec.rb new file mode 100644 index 000000000..bc341eba2 --- /dev/null +++ b/spec/jobs/buildable_spec.rb @@ -0,0 +1,36 @@ +require 'fast_spec_helper' +require 'app/jobs/buildable' +require 'app/models/payload' +require 'app/services/build_runner' + +describe Buildable do + class TestJob + extend Buildable + end + + describe '.perform' do + it 'runs build runner' do + payload_data = double(:payload_data) + payload = double(:payload) + Payload.stub(new: payload) + build_runner = double(:build_runner, run: nil) + BuildRunner.stub(new: build_runner) + + TestJob.perform(payload_data) + + expect(Payload).to have_received(:new).with(payload_data) + expect(BuildRunner).to have_received(:new).with(payload) + expect(build_runner).to have_received(:run) + end + + it 'retries when Resque::TermException is raised' do + Payload.stub(:new).and_raise(Resque::TermException.new(1)) + Resque.stub(:enqueue) + payload_data = double + + TestJob.perform(payload_data) + + expect(Resque).to have_received(:enqueue).with(TestJob, payload_data) + end + end +end diff --git a/spec/jobs/large_build_job_spec.rb b/spec/jobs/large_build_job_spec.rb new file mode 100644 index 000000000..546292d98 --- /dev/null +++ b/spec/jobs/large_build_job_spec.rb @@ -0,0 +1,14 @@ +require 'fast_spec_helper' +require 'app/jobs/retryable' +require 'app/jobs/buildable' +require 'app/jobs/large_build_job' + +describe LargeBuildJob do + it 'is retryable' do + expect(LargeBuildJob).to be_a(Retryable) + end + + it 'is buildable' do + expect(LargeBuildJob).to be_a(Buildable) + end +end diff --git a/spec/jobs/monitorable_spec.rb b/spec/jobs/monitorable_spec.rb deleted file mode 100644 index 3d44e7285..000000000 --- a/spec/jobs/monitorable_spec.rb +++ /dev/null @@ -1,22 +0,0 @@ -require 'fast_spec_helper' -require 'app/jobs/monitorable' - -describe Monitorable do - class FakeJob - include Monitorable - end - - it 'captures exception with context using Raven' do - Raven.stub(:capture_exception) - job = FakeJob.new - exception = double(:exception) - job_param = double(id: 123) - - job.error(job_param, exception) - - expect(Raven).to have_received(:capture_exception).with( - exception, - extra: { job_id: job_param.id } - ) - end -end diff --git a/spec/jobs/repo_synchronization_job_spec.rb b/spec/jobs/repo_synchronization_job_spec.rb index c92a8f50e..e4c5d935d 100644 --- a/spec/jobs/repo_synchronization_job_spec.rb +++ b/spec/jobs/repo_synchronization_job_spec.rb @@ -1,24 +1,50 @@ require 'spec_helper' describe RepoSynchronizationJob do - it 'is monitored' do - build_job = RepoSynchronizationJob.new(double) + it 'is retryable' do + expect(RepoSynchronizationJob).to be_a(Retryable) + end + + describe '.before_enqueue' do + it 'sets refreshing_repos to true' do + user = create(:user) - expect(build_job).to be_a Monitorable + RepoSynchronizationJob.before_enqueue(user.id, 'token') + + expect(user.reload).to be_refreshing_repos + end end -end -describe RepoSynchronizationJob, '#perform' do - it 'syncs repos for a given user' do - user = create(:user) - github_token = 'githubtoken' - sync_job = RepoSynchronizationJob.new(user.id, github_token) - sync = double(start: nil) - RepoSynchronization.stub(new: sync) + describe '.perform' do + it 'syncs repos and sets refreshing_repos to false' do + user = create(:user, refreshing_repos: true) + github_token = 'token' + synchronization = double(:repo_synchronization, start: nil) + RepoSynchronization.stub(new: synchronization) + + RepoSynchronizationJob.perform(user.id, github_token) + + expect(RepoSynchronization).to have_received(:new).with( + user, + github_token + ) + expect(synchronization).to have_received(:start) + expect(user.reload).not_to be_refreshing_repos + end + + it 'retries when Resque::TermException is raised' do + User.stub(:find).and_raise(Resque::TermException.new(1)) + Resque.stub(:enqueue) + user_id = 'userid' + github_token = 'token' - sync_job.perform + RepoSynchronizationJob.perform(user_id, github_token) - expect(RepoSynchronization).to have_received(:new).with(user, github_token) - expect(sync).to have_received(:start) + expect(Resque).to have_received(:enqueue).with( + RepoSynchronizationJob, + user_id, + github_token + ) + end end end diff --git a/spec/jobs/retryable_spec.rb b/spec/jobs/retryable_spec.rb new file mode 100644 index 000000000..e69de29bb diff --git a/spec/jobs/small_build_job_spec.rb b/spec/jobs/small_build_job_spec.rb new file mode 100644 index 000000000..ecb81bf37 --- /dev/null +++ b/spec/jobs/small_build_job_spec.rb @@ -0,0 +1,14 @@ +require 'fast_spec_helper' +require 'app/jobs/retryable' +require 'app/jobs/buildable' +require 'app/jobs/small_build_job' + +describe SmallBuildJob do + it 'is retryable' do + expect(SmallBuildJob).to be_a(Retryable) + end + + it 'is buildable' do + expect(SmallBuildJob).to be_a(Buildable) + end +end diff --git a/spec/lib/job_queue_spec.rb b/spec/lib/job_queue_spec.rb new file mode 100644 index 000000000..ff60c19ac --- /dev/null +++ b/spec/lib/job_queue_spec.rb @@ -0,0 +1,15 @@ +require 'fast_spec_helper' +require 'lib/job_queue' + +describe JobQueue do + describe '.push' do + it 'enqueues a Resque job' do + job_class = double(:job_class) + Resque.stub(:enqueue) + + JobQueue.push(job_class, 1, 2, 3) + + expect(Resque).to have_received(:enqueue).with(job_class, 1, 2, 3) + end + end +end diff --git a/spec/models/payload_spec.rb b/spec/models/payload_spec.rb index 0ff3b579a..55a9d896b 100644 --- a/spec/models/payload_spec.rb +++ b/spec/models/payload_spec.rb @@ -12,4 +12,13 @@ expect(payload.changed_files).to eq 1 end end + + describe '#data' do + it 'returns data' do + data = {one: 1} + payload = Payload.new(data) + + expect(payload.data).to eq data + end + end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 932ff34d8..eb8a64fc0 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -12,7 +12,7 @@ config.include OauthHelper config.include FactoryGirl::Syntax::Methods DatabaseCleaner.strategy = :deletion - Delayed::Worker.delay_jobs = false + Resque.inline = true config.before do DatabaseCleaner.clean