From 8c60eb54e744ccc0170231436f56d1f0fef7a99e Mon Sep 17 00:00:00 2001 From: Stephen Crosby Date: Tue, 16 Jun 2015 23:51:58 -0700 Subject: [PATCH] fix specs related to mongoid5 upgrade --- app/controllers/problems_controller.rb | 5 +++-- app/decorators/backtrace_decorator.rb | 5 +++++ app/decorators/backtrace_line_decorator.rb | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ app/decorators/notice_decorator.rb | 4 ++++ app/decorators/problem_decorator.rb | 4 ++++ app/helpers/backtrace_line_helper.rb | 44 ++++++++++++++++++++------------------------ app/mailers/mailer.rb | 12 ++++++------ app/models/backtrace.rb | 26 +++++++++----------------- app/models/backtrace_line.rb | 42 ------------------------------------------ app/models/backtrace_line_normalizer.rb | 32 -------------------------------- app/models/error_report.rb | 9 +++------ app/models/notice.rb | 4 ---- app/models/problem.rb | 4 ++-- app/views/issue_trackers/issue.md.erb | 2 +- app/views/issue_trackers/issue.txt.erb | 2 +- app/views/mailer/comment_notification.text.erb | 4 ++-- app/views/mailer/err_notification.html.haml | 11 ++++++----- app/views/mailer/err_notification.text.erb | 5 +++-- app/views/notices/_backtrace_line.html.haml | 2 +- spec/acceptance/app_regenerate_api_key_spec.rb | 8 ++++---- spec/controllers/problems_controller_spec.rb | 6 ++---- spec/fabricators/err_fabricator.rb | 15 +++++++-------- spec/fabricators/problem_fabricator.rb | 3 +-- spec/mailers/mailer_spec.rb | 60 ++++++++++++++++++++++++++++++++++++++++-------------------- spec/models/backtrace_line_spec.rb | 10 ---------- spec/models/backtrace_spec.rb | 54 +++++++++++++++--------------------------------------- spec/models/error_report_spec.rb | 14 ++++++-------- spec/models/fingerprint/md5_spec.rb | 16 +++++++--------- spec/models/fingerprint/sha1_spec.rb | 10 ++++------ spec/spec_helper.rb | 2 +- spec/views/issue_trackers/issue.md.erb_spec.rb | 2 +- spec/views/issue_trackers/issue.txt.erb_spec.rb | 3 ++- 32 files changed, 236 insertions(+), 260 deletions(-) create mode 100644 app/decorators/backtrace_decorator.rb create mode 100644 app/decorators/backtrace_line_decorator.rb create mode 100644 app/decorators/notice_decorator.rb create mode 100644 app/decorators/problem_decorator.rb delete mode 100644 app/models/backtrace_line.rb delete mode 100644 app/models/backtrace_line_normalizer.rb delete mode 100644 spec/models/backtrace_line_spec.rb diff --git a/app/controllers/problems_controller.rb b/app/controllers/problems_controller.rb index 1b9a375..12c68a4 100644 --- a/app/controllers/problems_controller.rb +++ b/app/controllers/problems_controller.rb @@ -22,7 +22,7 @@ class ProblemsController < ApplicationController } expose(:problem) { - app.problems.find(params[:id]) + ProblemDecorator.new app.problems.find(params[:id]) } expose(:all_errs) { @@ -50,7 +50,8 @@ class ProblemsController < ApplicationController def index; end def show - @notices = problem.notices.reverse_ordered.page(params[:notice]).per(1) + @notices = problem.object.notices.reverse_ordered + .page(params[:notice]).per(1) @notice = @notices.first @comment = Comment.new end diff --git a/app/decorators/backtrace_decorator.rb b/app/decorators/backtrace_decorator.rb new file mode 100644 index 0000000..9c2392d --- /dev/null +++ b/app/decorators/backtrace_decorator.rb @@ -0,0 +1,5 @@ +class BacktraceDecorator < Draper::Decorator + def lines + @lines ||= object.lines.map { |line| BacktraceLineDecorator.new line } + end +end diff --git a/app/decorators/backtrace_line_decorator.rb b/app/decorators/backtrace_line_decorator.rb new file mode 100644 index 0000000..2b62da7 --- /dev/null +++ b/app/decorators/backtrace_line_decorator.rb @@ -0,0 +1,76 @@ +class BacktraceLineDecorator < Draper::Decorator + EMPTY_STRING = ''.freeze + + def in_app? + object[:file].match Backtrace::IN_APP_PATH + end + + def number + object[:number] + end + + def file + object[:file] + end + + def method + object[:method] + end + + def file_relative + file.to_s.sub(Backtrace::IN_APP_PATH, EMPTY_STRING) + end + + def file_name + File.basename file + end + + def to_s + column = object.try(:[], :column) + "#{file_relative}:#{number}#{column.present? ? ":#{column}" : ''}" + end + + def link_to_source_file(app, &block) + text = h.capture_haml(&block) + link_to_in_app_source_file(app, text) || text + end + + private + def link_to_in_app_source_file(app, text) + return unless in_app? + if file_name =~ /\.js$/ + link_to_hosted_javascript(app, text) + else + link_to_repo_source_file(app, text) || + link_to_issue_tracker_file(app, text) + end + end + + def link_to_repo_source_file(app, text) + link_to_github(app, text) || link_to_bitbucket(app, text) + end + + def link_to_hosted_javascript(app, text) + if app.asset_host? + h.link_to(text, "#{app.asset_host}/#{file_relative}", :target => '_blank') + end + end + + def link_to_github(app, text = nil) + return unless app.github_repo? + href = "%s#L%s" % [app.github_url_to_file(decorated_path + file_name), number] + h.link_to(text || file_name, href, :target => '_blank') + end + + def link_to_bitbucket(app, text = nil) + return unless app.bitbucket_repo? + href = "%s#cl-%s" % [app.bitbucket_url_to_file(decorated_path + file_name), number] + h.link_to(text || file_name, href, :target => '_blank') + end + + def link_to_issue_tracker_file(app, text = nil) + return unless app.issue_tracker && app.issue_tracker.respond_to?(:url_to_file) + href = app.issue_tracker.url_to_file(file_relative, number) + h.link_to(text || file_name, href, :target => '_blank') + end +end diff --git a/app/decorators/notice_decorator.rb b/app/decorators/notice_decorator.rb new file mode 100644 index 0000000..bffe374 --- /dev/null +++ b/app/decorators/notice_decorator.rb @@ -0,0 +1,4 @@ +class NoticeDecorator < Draper::Decorator + decorates_association :backtrace + delegate_all +end diff --git a/app/decorators/problem_decorator.rb b/app/decorators/problem_decorator.rb new file mode 100644 index 0000000..e2efb95 --- /dev/null +++ b/app/decorators/problem_decorator.rb @@ -0,0 +1,4 @@ +class ProblemDecorator < Draper::Decorator + decorates_association :notices + delegate_all +end diff --git a/app/helpers/backtrace_line_helper.rb b/app/helpers/backtrace_line_helper.rb index 6c1d7fd..c65a7cd 100644 --- a/app/helpers/backtrace_line_helper.rb +++ b/app/helpers/backtrace_line_helper.rb @@ -1,49 +1,45 @@ module BacktraceLineHelper - def link_to_source_file(line, &block) + def link_to_source_file(line, app, &block) text = capture_haml(&block) - link_to_in_app_source_file(line, text) || link_to_external_source_file(text) + link_to_in_app_source_file(line, app, text) || text end private - def link_to_in_app_source_file(line, text) + def link_to_in_app_source_file(line, app, text) return unless line.in_app? if line.file_name =~ /\.js$/ - link_to_hosted_javascript(line, text) + link_to_hosted_javascript(line, app, text) else - link_to_repo_source_file(line, text) || - link_to_issue_tracker_file(line, text) + link_to_repo_source_file(line, app, text) || + link_to_issue_tracker_file(line, app, text) end end - def link_to_repo_source_file(line, text) - link_to_github(line, text) || link_to_bitbucket(line, text) + def link_to_repo_source_file(line, app, text) + link_to_github(line, app, text) || link_to_bitbucket(line, app, text) end - def link_to_hosted_javascript(line, text) - if line.app.asset_host? - link_to(text, "#{line.app.asset_host}/#{line.file_relative}", :target => '_blank') + def link_to_hosted_javascript(line, app, text) + if app.asset_host? + link_to(text, "#{app.asset_host}/#{line.file_relative}", :target => '_blank') end end - def link_to_external_source_file(text) - text - end - - def link_to_github(line, text = nil) - return unless line.app.github_repo? - href = "%s#L%s" % [line.app.github_url_to_file(line.decorated_path + line.file_name), line.number] + def link_to_github(line, app, text = nil) + return unless app.github_repo? + href = "%s#L%s" % [app.github_url_to_file(line.decorated_path + line.file_name), line.number] link_to(text || line.file_name, href, :target => '_blank') end - def link_to_bitbucket(line, text = nil) - return unless line.app.bitbucket_repo? - href = "%s#cl-%s" % [line.app.bitbucket_url_to_file(line.decorated_path + line.file_name), line.number] + def link_to_bitbucket(line, app, text = nil) + return unless app.bitbucket_repo? + href = "%s#cl-%s" % [app.bitbucket_url_to_file(line.decorated_path + line.file_name), line.number] link_to(text || line.file_name, href, :target => '_blank') end - def link_to_issue_tracker_file(line, text = nil) - return unless line.app.issue_tracker && line.app.issue_tracker.respond_to?(:url_to_file) - href = line.app.issue_tracker.url_to_file(line.file_relative, line.number) + def link_to_issue_tracker_file(line, app, text = nil) + return unless app.issue_tracker && app.issue_tracker.respond_to?(:url_to_file) + href = app.issue_tracker.url_to_file(line.file_relative, line.number) link_to(text || line.file_name, href, :target => '_blank') end diff --git a/app/mailers/mailer.rb b/app/mailers/mailer.rb index eead55c..0f3ec4d 100644 --- a/app/mailers/mailer.rb +++ b/app/mailers/mailer.rb @@ -13,11 +13,11 @@ class Mailer < ActionMailer::Base 'Precedence' => 'bulk', 'Auto-Submitted' => 'auto-generated' - def err_notification(notice) - @notice = notice - @app = notice.app + def err_notification(error_report) + @notice = NoticeDecorator.new error_report.notice + @app = AppDecorator.new error_report.app - count = @notice.similar_count + count = error_report.problem.notices_count count = count > 1 ? "(#{count}) " : "" errbit_headers 'App' => @app.name, @@ -30,7 +30,7 @@ class Mailer < ActionMailer::Base def deploy_notification(deploy) @deploy = deploy - @app = deploy.app + @app = AppDecorator.new deploy.app errbit_headers 'App' => @app.name, 'Environment' => @deploy.environment, @@ -44,7 +44,7 @@ class Mailer < ActionMailer::Base def comment_notification(comment) @comment = comment @user = comment.user - @problem = comment.err + @problem = ProblemDecorator.new comment.err @notice = @problem.notices.first @app = @problem.app diff --git a/app/models/backtrace.rb b/app/models/backtrace.rb index 113358c..f0596e0 100644 --- a/app/models/backtrace.rb +++ b/app/models/backtrace.rb @@ -2,31 +2,23 @@ class Backtrace include Mongoid::Document include Mongoid::Timestamps - field :fingerprint - index :fingerprint => 1 - - has_many :notices - has_one :notice + IN_APP_PATH = %r{^\[PROJECT_ROOT\](?!(\/vendor))/?} - embeds_many :lines, :class_name => "BacktraceLine" - - after_initialize :generate_fingerprint + field :fingerprint + field :lines - delegate :app, :to => :notice + index :fingerprint => 1 def self.find_or_create(lines) fingerprint = generate_fingerprint(lines) - where(fingerprint: generate_fingerprint(lines)). - find_one_and_update( - { '$setOnInsert' => { fingerprint: fingerprint, lines: lines } }, - { return_document: :after, upsert: true }) + where(fingerprint: fingerprint).find_one_and_update( + { '$setOnInsert' => { fingerprint: fingerprint, lines: lines } }, + { return_document: :after, upsert: true }) end - def raw=(raw) - raw.compact.each do |raw_line| - lines << BacktraceLine.new(BacktraceLineNormalizer.new(raw_line).call) - end + def self.generate_fingerprint(lines) + Digest::SHA1.hexdigest(lines.map(&:to_s).join) end private diff --git a/app/models/backtrace_line.rb b/app/models/backtrace_line.rb deleted file mode 100644 index 34df8a2..0000000 --- a/app/models/backtrace_line.rb +++ /dev/null @@ -1,42 +0,0 @@ -class BacktraceLine - include Mongoid::Document - IN_APP_PATH = %r{^\[PROJECT_ROOT\](?!(\/vendor))/?} - GEMS_PATH = %r{\[GEM_ROOT\]\/gems\/([^\/]+)} - - field :number, :type => Integer - field :column, :type => Integer - field :file - field :method - - embedded_in :backtrace - - scope :in_app, ->{ where(:file => IN_APP_PATH) } - - delegate :app, :to => :backtrace - - def to_s - "#{file_relative}:#{number}" << (column.present? ? ":#{column}" : "") - end - - def in_app? - !!(file =~ IN_APP_PATH) - end - - def path - File.dirname(file).gsub(/^\.$/, '') + "/" - end - - def file_relative - file.to_s.sub(IN_APP_PATH, '') - end - - def file_name - File.basename file - end - - def decorated_path - path.sub(BacktraceLine::IN_APP_PATH, ''). - sub(BacktraceLine::GEMS_PATH, "\\1") - end - -end diff --git a/app/models/backtrace_line_normalizer.rb b/app/models/backtrace_line_normalizer.rb deleted file mode 100644 index cf99551..0000000 --- a/app/models/backtrace_line_normalizer.rb +++ /dev/null @@ -1,32 +0,0 @@ -class BacktraceLineNormalizer - def initialize(raw_line) - @raw_line = raw_line || {} - end - - def call - @raw_line.merge 'file' => normalized_file, 'method' => normalized_method - end - - private - def normalized_file - if @raw_line['file'].blank? - "[unknown source]" - else - file = @raw_line['file'].to_s - # Detect lines from gem - file.gsub!(/\[PROJECT_ROOT\]\/.*\/ruby\/[0-9.]+\/gems/, '[GEM_ROOT]/gems') - # Strip any query strings - file.gsub!(/\?[^\?]*$/, '') - @raw_line['file'] = file - end - end - - def normalized_method - if @raw_line['method'].blank? - "[unknown method]" - else - @raw_line['method'].to_s.gsub(/[0-9_]{10,}+/, "__FRAGMENT__") - end - end - -end diff --git a/app/models/error_report.rb b/app/models/error_report.rb index 9cd99a1..fd7a7ba 100644 --- a/app/models/error_report.rb +++ b/app/models/error_report.rb @@ -79,23 +79,20 @@ class ErrorReport # Update problem cache with information about this notice def cache_attributes_on_problem @problem = Problem.cache_notice(@error.problem_id, @notice) - - # cache_notice returns the old problem, so the count is one higher - @similar_count = @problem.notices_count + 1 end # Send email notification if needed def email_notification return false unless app.emailable? - return false unless app.email_at_notices.include?(@similar_count) - Mailer.err_notification(@notice).deliver + return false unless app.email_at_notices.include?(@problem.notices_count) + Mailer.err_notification(self).deliver rescue => e HoptoadNotifier.notify(e) end def should_notify? app.notification_service.notify_at_notices.include?(0) || - app.notification_service.notify_at_notices.include?(@similar_count) + app.notification_service.notify_at_notices.include?(@problem.notices_count) end # Launch all notification define on the app associate to this notice diff --git a/app/models/notice.rb b/app/models/notice.rb index d901331..8104861 100644 --- a/app/models/notice.rb +++ b/app/models/notice.rb @@ -103,10 +103,6 @@ class Notice request['session'] || {} end - def in_app_backtrace_lines - backtrace_lines.in_app - end - ## # TODO: Move on decorator maybe # diff --git a/app/models/problem.rb b/app/models/problem.rb index 26abaef..991db04 100644 --- a/app/models/problem.rb +++ b/app/models/problem.rb @@ -76,7 +76,7 @@ class Problem host_digest = Digest::MD5.hexdigest(notice.host) user_agent_digest = Digest::MD5.hexdigest(notice.user_agent_string) - Problem.where('_id' => id).find_one_and_update( + Problem.where('_id' => id).find_one_and_update({ '$set' => { 'environment' => notice.environment_name, 'error_class' => notice.error_class, @@ -95,7 +95,7 @@ class Problem "hosts.#{host_digest}.count" => 1, "user_agents.#{user_agent_digest}.count" => 1, } - ) + }, return_document: :after) end def uncache_notice(notice) diff --git a/app/views/issue_trackers/issue.md.erb b/app/views/issue_trackers/issue.md.erb index 56353a9..6b39c61 100644 --- a/app/views/issue_trackers/issue.md.erb +++ b/app/views/issue_trackers/issue.md.erb @@ -27,7 +27,7 @@ ## Backtrace ## ~~~ -<% notice.backtrace_lines.each do |line| %><%= line.number %>: <%= line.file_relative %> -> **<%= line.method %>** +<% notice.backtrace.lines.each do |line| %><%= line.number %>: <%= line.file_relative %> -> **<%= line.method %>** <% end %> ~~~ diff --git a/app/views/issue_trackers/issue.txt.erb b/app/views/issue_trackers/issue.txt.erb index ff285cb..ca28aa7 100644 --- a/app/views/issue_trackers/issue.txt.erb +++ b/app/views/issue_trackers/issue.txt.erb @@ -16,6 +16,6 @@ Env: <%= pretty_hash notice.env_vars %> Backtrace --------- -<% notice.backtrace_lines.each do |line| %><%= sprintf('%5d: %s **%s', line.number, line.file_relative, line.method) %> +<% notice.backtrace.lines.each do |line| %><%= sprintf('%5d: %s **%s', line.number, line.file_relative, line.method) %> <% end %> <% end %> diff --git a/app/views/mailer/comment_notification.text.erb b/app/views/mailer/comment_notification.text.erb index bf43174..9347fda 100644 --- a/app/views/mailer/comment_notification.text.erb +++ b/app/views/mailer/comment_notification.text.erb @@ -21,8 +21,8 @@ WHERE: <%= @notice.where %> -<% @notice.in_app_backtrace_lines.each do |line| %> - <%= line %> +<% @notice.backtrace.lines.each do |line| %> + <% next unless line.in_app? %><%= line %> <% end %> diff --git a/app/views/mailer/err_notification.html.haml b/app/views/mailer/err_notification.html.haml index 074ad22..ee61597 100644 --- a/app/views/mailer/err_notification.html.haml +++ b/app/views/mailer/err_notification.html.haml @@ -27,9 +27,10 @@ %p.heading WHERE: %p.monospace = @notice.where - - @notice.in_app_backtrace_lines.each do |line| + - @notice.backtrace.lines.each do |line| + - next unless line.in_app? %p.backtrace - = link_to_source_file(line) do + = line.link_to_source_file(@app) do = line.to_s %br - if @notice.app_version.present? @@ -59,11 +60,11 @@ %td(style="text-align: right; padding-right: 10px; color: #6a6a6a;")= key.to_s.titleize + ":" %td= auto_link(value.to_s) %br - - if @notice.backtrace_lines.any? + - if @notice.backtrace.lines.any? %br %p.heading FULL BACKTRACE: - - @notice.backtrace_lines.each do |line| + - @notice.backtrace.lines.each do |line| %p.backtrace - = link_to_source_file(line) do + = link_to_source_file(line, @app) do = line.to_s %br diff --git a/app/views/mailer/err_notification.text.erb b/app/views/mailer/err_notification.text.erb index 90aae85..5b12e56 100644 --- a/app/views/mailer/err_notification.text.erb +++ b/app/views/mailer/err_notification.text.erb @@ -14,7 +14,8 @@ WHERE: <%= @notice.where %> -<% @notice.in_app_backtrace_lines.each do |line| %> +<% @notice.backtrace.lines.each do |line| %> + <% next unless line.in_app? %> <%= line %> <% end %> @@ -51,7 +52,7 @@ USER: BACKTRACE: -<% @notice.backtrace_lines.each do |line| %> +<% @notice.backtrace.lines.each do |line| %> <%= line %> <% end %> diff --git a/app/views/notices/_backtrace_line.html.haml b/app/views/notices/_backtrace_line.html.haml index 62402aa..966bb1c 100644 --- a/app/views/notices/_backtrace_line.html.haml +++ b/app/views/notices/_backtrace_line.html.haml @@ -1,6 +1,6 @@ %tr{:class => defined?(row_class) && row_class} %td.line{:class => line.in_app? && 'in-app' } - = link_to_source_file(line) do + = line.link_to_source_file(app) do %span.path>= raw line.decorated_path %span.file>= line.file_name - if line.number.present? diff --git a/spec/acceptance/app_regenerate_api_key_spec.rb b/spec/acceptance/app_regenerate_api_key_spec.rb index 69604d9..a3077df 100644 --- a/spec/acceptance/app_regenerate_api_key_spec.rb +++ b/spec/acceptance/app_regenerate_api_key_spec.rb @@ -52,8 +52,8 @@ feature "Create an application" do fill_in 'app_name', :with => 'My new app' click_on I18n.t('apps.new.add_app') page.has_content?(I18n.t('controllers.apps.flash.create.success')) - expect(App.where(:name => 'My new app').count).to eql 1 - expect(App.where(:name => 'My new app 2').count).to eql 0 + expect(App.where(:name => 'My new app').count).to eq 1 + expect(App.where(:name => 'My new app 2').count).to eq 0 click_on I18n.t('shared.navigation.apps') @@ -62,8 +62,8 @@ feature "Create an application" do fill_in 'app_name', :with => 'My new app 2' click_on I18n.t('apps.edit.update') page.has_content?(I18n.t('controllers.apps.flash.update.success')) - expect(App.where(:name => 'My new app').count).to eql 0 - expect(App.where(:name => 'My new app 2').count).to eql 1 + expect(App.where(:name => 'My new app').count).to eq 0 + expect(App.where(:name => 'My new app 2').count).to eq 1 end diff --git a/spec/controllers/problems_controller_spec.rb b/spec/controllers/problems_controller_spec.rb index 1f14023..c1419ef 100644 --- a/spec/controllers/problems_controller_spec.rb +++ b/spec/controllers/problems_controller_spec.rb @@ -145,8 +145,6 @@ describe ProblemsController, type: 'controller' do end describe "GET /apps/:app_id/problems/:id" do - #render_views - context 'when logged in as an admin' do before do sign_in admin @@ -250,8 +248,8 @@ describe ProblemsController, type: 'controller' do before { sign_in admin } context "when app has a issue tracker" do - let(:notice) { Fabricate :notice } - let(:problem) { notice.problem } + let(:notice) { NoticeDecorator.new(Fabricate :notice) } + let(:problem) { ProblemDecorator.new(notice.problem) } let(:issue_tracker) do Fabricate(:issue_tracker).tap do |t| t.instance_variable_set(:@tracker, ErrbitPlugin::MockIssueTracker.new(t.options)) diff --git a/spec/fabricators/err_fabricator.rb b/spec/fabricators/err_fabricator.rb index ef85786..91e4b71 100644 --- a/spec/fabricators/err_fabricator.rb +++ b/spec/fabricators/err_fabricator.rb @@ -18,12 +18,11 @@ Fabricator :notice do end Fabricator :backtrace do - lines(:count => 99) { Fabricate.build(:backtrace_line) } -end - -Fabricator :backtrace_line do - number { rand(999) } - file { "/path/to/file/#{SecureRandom.hex(4)}.rb" } - method(:method) { ActiveSupport.methods.shuffle.first } + lines(:count => 99) do + { + number: rand(999), + file: "/path/to/file/#{SecureRandom.hex(4)}.rb", + method: ActiveSupport.methods.shuffle.first + } + end end - diff --git a/spec/fabricators/problem_fabricator.rb b/spec/fabricators/problem_fabricator.rb index 537c37a..b230ef7 100644 --- a/spec/fabricators/problem_fabricator.rb +++ b/spec/fabricators/problem_fabricator.rb @@ -23,8 +23,7 @@ end Fabricator(:problem_resolved, :from => :problem) do after_create do |pr| - Fabricate(:notice, - :err => Fabricate(:err, :problem => pr)) + Fabricate(:notice, :err => Fabricate(:err, :problem => pr)) pr.resolve! end end diff --git a/spec/mailers/mailer_spec.rb b/spec/mailers/mailer_spec.rb index 320bf65..c5b7bb0 100644 --- a/spec/mailers/mailer_spec.rb +++ b/spec/mailers/mailer_spec.rb @@ -1,26 +1,27 @@ shared_examples "a notification email" do it "should have X-Mailer header" do - expect(@email).to have_header('X-Mailer', 'Errbit') + expect(email).to have_header('X-Mailer', 'Errbit') end it "should have X-Errbit-Host header" do - expect(@email).to have_header('X-Errbit-Host', Errbit::Config.host) + expect(email).to have_header('X-Errbit-Host', Errbit::Config.host) end it "should have Precedence header" do - expect(@email).to have_header('Precedence', 'bulk') + expect(email).to have_header('Precedence', 'bulk') end it "should have Auto-Submitted header" do - expect(@email).to have_header('Auto-Submitted', 'auto-generated') + expect(email).to have_header('Auto-Submitted', 'auto-generated') end it "should have X-Auto-Response-Suppress header" do # http://msdn.microsoft.com/en-us/library/ee219609(v=EXCHG.80).aspx - expect(@email).to have_header('X-Auto-Response-Suppress', 'OOF, AutoReply') + expect(email).to have_header('X-Auto-Response-Suppress', 'OOF, AutoReply') end it "should send the email" do + email expect(ActionMailer::Base.deliveries.size).to eq 1 end end @@ -30,44 +31,63 @@ describe Mailer do include EmailSpec::Helpers include EmailSpec::Matchers - let(:notice) { Fabricate(:notice, :message => "class < ActionController::Base") } - let!(:user) { Fabricate(:admin) } + let(:notice) do + n = Fabricate(:notice, message: "class < ActionController::Base") + n.backtrace.lines.last[:file] = '[PROJECT_ROOT]/path/to/file.js' + # notice.backtrace.update_attributes(lines: lines) + n + end - before do - ActionMailer::Base.deliveries = [] - notice.backtrace.lines.last.update_attributes(:file => "[PROJECT_ROOT]/path/to/file.js") - notice.app.update_attributes( + let(:app) do + a = notice.app + a.update_attributes( :asset_host => "http://example.com", :notify_all_users => true ) - notice.problem.update_attributes :notices_count => 3 - - @email = Mailer.err_notification(notice).deliver + a + end + let(:problem) do + p = notice.problem + p.notices_count = 3 + p + end + let!(:user) { Fabricate(:admin) } + let(:error_report) do + instance_double( + 'ErrorReport', + notice: notice, + app: app, + problem: problem + ) + end + let(:email) do + Mailer.err_notification(error_report).deliver end - it_should_behave_like "a notification email" + before { email } + it_should_behave_like "a notification email" it "should html-escape the notice's message for the html part" do - expect(@email).to have_body_text("class < ActionController::Base") + expect(email).to have_body_text("class < ActionController::Base") end it "should have inline css" do - expect(@email).to have_body_text('

path/to/file.js') + expect(email).to have_body_text('path/to/file.js') end it "should have the error count in the subject" do - expect(@email.subject).to match( /^\(3\) / ) + expect(email.subject).to match( /^\(3\) / ) end context 'with a very long message' do let(:notice) { Fabricate(:notice, :message => 6.times.collect{|a| "0123456789" }.join('')) } it "should truncate the long message" do - expect(@email.subject).to match( / \d{47}\.{3}$/ ) + expect(email.subject).to match( / \d{47}\.{3}$/ ) end end end diff --git a/spec/models/backtrace_line_spec.rb b/spec/models/backtrace_line_spec.rb deleted file mode 100644 index 197b5cf..0000000 --- a/spec/models/backtrace_line_spec.rb +++ /dev/null @@ -1,10 +0,0 @@ -describe BacktraceLine, type: 'model' do - subject { described_class.new(raw_line) } - - describe "root at the start of decorated filename" do - let(:raw_line) { { 'number' => rand(999), 'file' => '[PROJECT_ROOT]/app/controllers/pages_controller.rb', 'method' => ActiveSupport.methods.shuffle.first.to_s } } - it "should leave leading root symbol in filepath" do - expect(subject.decorated_path).to eq 'app/controllers/' - end - end -end diff --git a/spec/models/backtrace_spec.rb b/spec/models/backtrace_spec.rb index 1d93853..5140aec 100644 --- a/spec/models/backtrace_spec.rb +++ b/spec/models/backtrace_spec.rb @@ -1,49 +1,25 @@ describe Backtrace, type: 'model' do - subject { described_class.new } - - its(:fingerprint) { should be_present } - - describe "#similar" do - context "no similar backtrace" do - its(:similar) { should be_nil } + describe '.find_or_create' do + let(:lines) do + [ + { 'number' => '123', 'file' => '/some/path/to.rb', 'method' => 'abc' }, + { 'number' => '345', 'file' => '/path/to.rb', 'method' => 'dowhat' } + ] end + let(:fingerprint) { Backtrace.generate_fingerprint(lines) } - context "similar backtrace exist" do - let!(:similar_backtrace) { - b = Fabricate(:backtrace) - b.fingerprint = fingerprint - b.save! - b - } - let(:fingerprint) { "fingerprint" } - - before { allow(subject).to receive(:fingerprint).and_return(fingerprint) } - - its(:similar) { should == similar_backtrace } - end - end - - describe "find_or_create" do - subject { described_class.find_or_create(attributes) } - let(:attributes) { double :attributes } - let(:backtrace) { double :backtrace } - - before { allow(described_class).to receive(:new).and_return(backtrace) } - - context "no similar backtrace" do - before { allow(backtrace).to receive(:similar).and_return(nil) } - it "create new backtrace" do - expect(described_class).to receive(:create).with(attributes) + it 'create new backtrace' do + backtrace = described_class.find_or_create(lines) - described_class.find_or_create(attributes) - end + expect(backtrace.lines).to eq(lines) + expect(backtrace.fingerprint).to eq(fingerprint) end - context "similar backtrace exist" do - let(:similar_backtrace) { double :similar_backtrace } - before { allow(backtrace).to receive(:similar).and_return(similar_backtrace) } + it 'creates one backtrace for two identical ones' do + described_class.find_or_create(lines) + described_class.find_or_create(lines) - it { should == similar_backtrace } + expect(Backtrace.where(fingerprint: fingerprint).count).to eq(1) end end end diff --git a/spec/models/error_report_spec.rb b/spec/models/error_report_spec.rb index 3316111..98cf420 100644 --- a/spec/models/error_report_spec.rb +++ b/spec/models/error_report_spec.rb @@ -21,9 +21,7 @@ describe ErrorReport do Rails.root.join('spec','fixtures','hoptoad_test_notice.xml').read } - let(:error_report) { - ErrorReport.new(xml) - } + let(:error_report) { ErrorReport.new(xml) } let!(:app) { Fabricate( @@ -210,14 +208,13 @@ describe ErrorReport do end it 'find the correct err for the notice' do - err = Fabricate(:err, :problem => Fabricate(:problem, :resolved => true)) - - allow(error_report).to receive(:fingerprint).and_return(err.fingerprint) + error_report.generate_notice! + error_report.problem.resolve! expect { - error_report.generate_notice! + ErrorReport.new(xml).generate_notice! }.to change { - error_report.error.resolved? + error_report.problem.reload.resolved? }.from(true).to(false) end @@ -227,6 +224,7 @@ describe ErrorReport do app.watchers.build(:email => 'foo@example.com') app.save end + it 'send email' do notice = error_report.generate_notice! email = ActionMailer::Base.deliveries.last diff --git a/spec/models/fingerprint/md5_spec.rb b/spec/models/fingerprint/md5_spec.rb index 3209de5..283e56c 100644 --- a/spec/models/fingerprint/md5_spec.rb +++ b/spec/models/fingerprint/md5_spec.rb @@ -1,7 +1,7 @@ describe Fingerprint::MD5, type: 'model' do context 'being created' do let(:backtrace) do - Backtrace.create(:raw => [ + Backtrace.find_or_create([ { "number"=>"17", "file"=>"[GEM_ROOT]/gems/activesupport/lib/active_support/callbacks.rb", @@ -14,10 +14,9 @@ describe Fingerprint::MD5, type: 'model' do context "with same backtrace" do let(:backtrace_2) do - backtrace - backtrace.lines.last.method = '_run__FRAGMENT__process_action__FRAGMENT__callbacks' - backtrace.save - backtrace + new_lines = backtrace.lines.dup + new_lines.last[:method] = '_run__FRAGMENT__process_action__FRAGMENT__callbacks' + Backtrace.find_or_create(new_lines) end it "normalizes the fingerprint of generated methods" do @@ -27,10 +26,9 @@ describe Fingerprint::MD5, type: 'model' do context "with same backtrace where FRAGMENT has not been extracted" do let(:backtrace_2) do - backtrace - backtrace.lines.last.method = '_run__998857585768765__process_action__1231231312321313__callbacks' - backtrace.save - backtrace + new_lines = backtrace.lines.dup + new_lines.last[:method] = '_run__998857585768765__process_action__1231231312321313__callbacks' + Backtrace.find_or_create(new_lines) end it "normalizes the fingerprint of generated methods" do diff --git a/spec/models/fingerprint/sha1_spec.rb b/spec/models/fingerprint/sha1_spec.rb index fb06fb5..417914c 100644 --- a/spec/models/fingerprint/sha1_spec.rb +++ b/spec/models/fingerprint/sha1_spec.rb @@ -1,7 +1,7 @@ describe Fingerprint::Sha1, type: 'model' do context '#generate' do let(:backtrace) { - Backtrace.create(:raw => [ + Backtrace.find_or_create([ {"number"=>"425", "file"=>"[GEM_ROOT]/gems/activesupport-3.0.0.rc/lib/active_support/callbacks.rb", "method"=>"_run__2115867319__process_action__262109504__callbacks"}, {"number"=>"404", "file"=>"[GEM_ROOT]/gems/activesupport-3.0.0.rc/lib/active_support/callbacks.rb", "method"=>"send"}, {"number"=>"404", "file"=>"[GEM_ROOT]/gems/activesupport-3.0.0.rc/lib/active_support/callbacks.rb", "method"=>"_run_process_action_callbacks"} @@ -19,11 +19,9 @@ describe Fingerprint::Sha1, type: 'model' do context "with different backtrace with only last line change" do let(:backtrace_2) { - backtrace - backtrace.lines.last.number = 401 - backtrace.send(:generate_fingerprint) - backtrace.save - backtrace + new_lines = backtrace.lines.dup + new_lines.last[:number] = 401 + Backtrace.find_or_create backtrace.lines } it 'should not same fingerprint' do expect( diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 520cfbf..364f60e 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,7 +1,7 @@ # This file is copied to ~/spec when you run 'ruby script/generate rspec' # from the project root directory. ENV["RAILS_ENV"] = 'test' -ENV["ERRBIT_LOG_LEVEL"] = 'error' +ENV["ERRBIT_LOG_LEVEL"] = 'info' if ENV['COVERAGE'] require 'coveralls' diff --git a/spec/views/issue_trackers/issue.md.erb_spec.rb b/spec/views/issue_trackers/issue.md.erb_spec.rb index edc3e19..e344f93 100644 --- a/spec/views/issue_trackers/issue.md.erb_spec.rb +++ b/spec/views/issue_trackers/issue.md.erb_spec.rb @@ -6,7 +6,7 @@ describe "issue_trackers/issue.md.erb", type: 'view' do } before do - allow(view).to receive(:problem).and_return(problem) + allow(view).to receive(:problem).and_return(ProblemDecorator.new(problem)) end it "has the problem url" do diff --git a/spec/views/issue_trackers/issue.txt.erb_spec.rb b/spec/views/issue_trackers/issue.txt.erb_spec.rb index 43676ce..fcd30e1 100644 --- a/spec/views/issue_trackers/issue.txt.erb_spec.rb +++ b/spec/views/issue_trackers/issue.txt.erb_spec.rb @@ -6,7 +6,8 @@ describe "issue_trackers/issue.txt.erb", type: 'view' do } before do - allow(view).to receive(:problem).and_return(problem) + allow(view).to receive(:problem).and_return( + ProblemDecorator.new(problem)) end it "has the problem url" do -- libgit2 0.21.2