Skip to content
This repository has been archived by the owner on Nov 30, 2024. It is now read-only.

Fix caller filter to ignore rubygems #64

Merged
merged 2 commits into from
May 13, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions lib/rspec/support/caller_filter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,14 @@ class CallerFilter

LIB_REGEX = %r{/lib/rspec/(#{(RSPEC_LIBS + ADDITIONAL_TOP_LEVEL_FILES).join('|')})(\.rb|/)}

# rubygems/core_ext/kernel_require.rb isn't actually part of rspec (obviously) but we want
# it ignored when we are looking for the first meaningful line of the backtrace outside
# of RSpec. It can show up in the backtrace as the immediate first caller
# when `CallerFilter.first_non_rspec_line` is called from the top level of a required
# file, but it depends on if rubygems is loaded or not. We don't want to have to deal
# with this complexity in our `RSpec.deprecate` calls, so we ignore it here.
IGNORE_REGEX = Regexp.union(LIB_REGEX, "rubygems/core_ext/kernel_require.rb")

if RUBY_VERSION >= '2.0.0'
def self.first_non_rspec_line
# `caller` is an expensive method that scales linearly with the size of
Expand All @@ -37,7 +45,7 @@ def self.first_non_rspec_line
stack = caller(i, increment)
raise "No non-lib lines in stack" unless stack

line = stack.find { |l| l !~ LIB_REGEX }
line = stack.find { |l| l !~ IGNORE_REGEX }

i += increment
increment *= 2 # The choice of two here is arbitrary.
Expand All @@ -49,7 +57,7 @@ def self.first_non_rspec_line
# Earlier rubies do not support the two argument form of `caller`. This
# fallback is logically the same, but slower.
def self.first_non_rspec_line
caller.find { |line| line !~ LIB_REGEX }
caller.find { |line| line !~ IGNORE_REGEX }
end
end
end
Expand Down
34 changes: 27 additions & 7 deletions spec/rspec/support/caller_filter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,12 @@ def ruby_files_in_lib(lib)

describe "the filtering regex" do
def unmatched_from(files)
files.reject { |file| file.match(CallerFilter::LIB_REGEX) }
files.reject { |file| file.match(CallerFilter::IGNORE_REGEX) }
end

%w[ core mocks expectations support ].each do |lib|
it "matches all ruby files in rspec-#{lib}" do
files = ruby_files_in_lib(lib)

# We don't care about this file -- it only has a single require statement
# and won't show up in any backtraces.
files.reject! { |file| file.end_with?('lib/rspec-expectations.rb') }

files = ruby_files_in_lib(lib)
expect(unmatched_from files).to eq([])
end
end
Expand All @@ -38,6 +33,31 @@ def unmatched_from(files)

expect(unmatched_from files).to eq(files)
end

def in_rspec_support_lib(name)
root = File.expand_path("../../../../lib/rspec/support", __FILE__)
dir = "#{root}/#{name}"
FileUtils.mkdir(dir)
yield dir
ensure
FileUtils.rm_rf(dir)
end

it 'does not match rubygems lines from `require` statements' do
require 'rubygems' # ensure rubygems is laoded

in_rspec_support_lib("test_dir") do |dir|
File.open("#{dir}/file.rb", "w") do |file|
file.write("$_caller_filter = RSpec::CallerFilter.first_non_rspec_line")
end

$_caller_filter = nil

expect {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cough do/end ;)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the one place I prefer curlies for multiple lines...because "do" and "end" are english words I think it's odd to have them in the middle of an expect { }.to matcher expression. The curlies, as punctuation, don't have that problem.

Funny to see that are preferences as a team are all over the map on this :).

require "rspec/support/test_dir/file"
}.to change { $_caller_filter }.to(include "#{__FILE__}:#{__LINE__ - 1}")
end
end
end
end
end
Expand Down