From a3678e45ecf8e17527722889d5347325083ad560 Mon Sep 17 00:00:00 2001 From: "Mark J. Titorenko" Date: Thu, 20 Jun 2013 13:51:01 +0100 Subject: [PATCH] Fix BacktraceCleaner#noise for multiple silencers. The previous implementation of BacktraceSilencer#noise did not work correctly if more than one silencer was configured -- specifically, it would only return noise which was matched by all silencers. The new implementation is such that anything that has been matched by silencers is removed from the backtrace using Array#- (array difference), ie. we now return all elements within a backtrace that have been matched by any silencer (and are thus removed by #silence). Fixes #11030. --- activesupport/CHANGELOG.md | 7 +++++++ .../lib/active_support/backtrace_cleaner.rb | 6 +----- activesupport/test/clean_backtrace_test.rb | 21 +++++++++++++++++++ 3 files changed, 29 insertions(+), 5 deletions(-) diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index b69333851f..45f71daa08 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,10 @@ +* Fix return value from `BacktraceCleaner#noise` when the cleaner is configured + with multiple silencers. + + Fixes #11030 + + *Mark J. Titorenko* + * `HashWithIndifferentAccess#select` now returns a `HashWithIndifferentAccess` instance instead of a `Hash` instance. diff --git a/activesupport/lib/active_support/backtrace_cleaner.rb b/activesupport/lib/active_support/backtrace_cleaner.rb index 6f9dc679c2..c88ae3e661 100644 --- a/activesupport/lib/active_support/backtrace_cleaner.rb +++ b/activesupport/lib/active_support/backtrace_cleaner.rb @@ -97,11 +97,7 @@ module ActiveSupport end def noise(backtrace) - @silencers.each do |s| - backtrace = backtrace.select { |line| s.call(line) } - end - - backtrace + backtrace - silence(backtrace) end end end diff --git a/activesupport/test/clean_backtrace_test.rb b/activesupport/test/clean_backtrace_test.rb index b14950acb3..dd67a45cf6 100644 --- a/activesupport/test/clean_backtrace_test.rb +++ b/activesupport/test/clean_backtrace_test.rb @@ -36,6 +36,27 @@ class BacktraceCleanerSilencerTest < ActiveSupport::TestCase end end +class BacktraceCleanerMultipleSilencersTest < ActiveSupport::TestCase + def setup + @bc = ActiveSupport::BacktraceCleaner.new + @bc.add_silencer { |line| line =~ /mongrel/ } + @bc.add_silencer { |line| line =~ /yolo/ } + end + + test "backtrace should not contain lines that match the silencers" do + assert_equal \ + [ "/other/class.rb" ], + @bc.clean([ "/mongrel/class.rb", "/other/class.rb", "/mongrel/stuff.rb", "/other/yolo.rb" ]) + end + + test "backtrace should only contain lines that match the silencers" do + assert_equal \ + [ "/mongrel/class.rb", "/mongrel/stuff.rb", "/other/yolo.rb" ], + @bc.clean([ "/mongrel/class.rb", "/other/class.rb", "/mongrel/stuff.rb", "/other/yolo.rb" ], + :noise) + end +end + class BacktraceCleanerFilterAndSilencerTest < ActiveSupport::TestCase def setup @bc = ActiveSupport::BacktraceCleaner.new