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

Expose configuration options #834

Merged
4 changes: 4 additions & 0 deletions lib/rspec/core/backtrace_cleaner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ def full_backtrace=(true_or_false)
@exclusion_patterns = true_or_false ? [] : DEFAULT_EXCLUSION_PATTERNS.dup
end

def full_backtrace
@exclusion_patterns.empty?
end

Copy link
Member

Choose a reason for hiding this comment

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

Given that this is a boolean predicate, maybe it should be full_backtrace?.

private

def matches_an_exclusion_pattern?(line)
Expand Down
25 changes: 24 additions & 1 deletion lib/rspec/core/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,9 @@ def self.add_setting(name, opts={})
# Indicates files configured to be required
define_reader :requires

# Returns dirs that have been prepended to the load path by #lib=
define_reader :libs

# Default: `$stdout`.
# Also known as `output` and `out`
add_setting :output_stream, :alias_with => [:output, :out]
Expand Down Expand Up @@ -204,6 +207,7 @@ def initialize
@detail_color = :cyan
@profile_examples = false
@requires = []
@libs = []
end

# @private
Expand Down Expand Up @@ -462,6 +466,10 @@ def expect_with(*frameworks)
@expectation_frameworks.push(*modules)
end

def full_backtrace
@backtrace_cleaner.full_backtrace
end
Copy link
Member

Choose a reason for hiding this comment

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

Same here -- I think it would be more idiomatic as full_backtrace?.


def full_backtrace=(true_or_false)
Copy link
Member

Choose a reason for hiding this comment

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

Please put blank lines between method defs -- I find that it helps readability a lot.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done, sorry, I suck at that.

@backtrace_cleaner.full_backtrace = true_or_false
end
Expand Down Expand Up @@ -492,7 +500,10 @@ def color=(bool)
define_predicate_for :color_enabled, :color

def libs=(libs)
libs.map {|lib| $LOAD_PATH.unshift lib}
libs.map do |lib|
@libs.unshift lib
$LOAD_PATH.unshift lib
end
end
Copy link
Member

Choose a reason for hiding this comment

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

I'm a little concerned that this isn't symmetric with libs= -- this will return all load paths, not just the ones the user has added using libs=. I think that these reader/writer methods should be symmetric; otherwise it gets confusing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, I could argue that I did this because the paths can be mutated from anywhere prior too... However I will defer and make this capture only the path's we've configured.

Copy link
Member

Choose a reason for hiding this comment

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

Well, I could argue that I did this because the paths can be mutated from anywhere prior too.

That's another reason I think this should only return the load paths that have been added through rspec config: because if the user wants all load paths they can already get it via $LOAD_PATH. However, they don't have a way to get the load paths that were configured via rspec, and that, to me, is the value proposition of having this method at all. (If it's just going to return $LOAD_PATH, I'd just encourage users to use $LOAD_PATH directly.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Heh the @github notification email couldn't cope with the outdated diff there, oh well, I've changed this in my most recent push anyhow.


def requires=(paths)
Expand Down Expand Up @@ -523,15 +534,27 @@ def debug=(bool)
end
end

def debug
!!defined?(Debugger)
end
Copy link
Member

Choose a reason for hiding this comment

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

FWIW, I'm planning on removing the debug option in rspec 3...but it's fine to have this for now.

Copy link
Member Author

Choose a reason for hiding this comment

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

Noted

Copy link
Member

Choose a reason for hiding this comment

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

Maybe here, too? (debug?)


# Run examples defined on `line_numbers` in all files to run.
def line_numbers=(line_numbers)
filter_run :line_numbers => line_numbers.map{|l| l.to_i}
end

def line_numbers
filter.fetch(:line_numbers,[])
end
Copy link
Member

Choose a reason for hiding this comment

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

Again, this isn't symmetric -- line_numbers= accepts an array of line numbers, but this returns a boolean...that seems odd.

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 one makes the most sense to return directly, so done.


def full_description=(description)
filter_run :full_description => Regexp.union(*Array(description).map {|d| Regexp.new(d) })
end

def full_description
filter.fetch :full_description, false
end
Copy link
Member

Choose a reason for hiding this comment

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

This isn't symmetric either -- any reason not to have this return the assigned full description rather than just a boolean?

Copy link
Member Author

Choose a reason for hiding this comment

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

The thinking behind these non symmetrical readers was that they were filters, so they indicated wether the filters were on or not, (this thinking was caused by my reading of the specs, which are rather... oddly... structured when it comes to these 3 methods... )

Copy link
Member

Choose a reason for hiding this comment

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

Do you think nil makes more sense here than false? Usually false is paired with true, and other wise nil is used...


# @overload add_formatter(formatter)
#
# Adds a formatter to the formatters collection. `formatter` can be a
Expand Down
12 changes: 12 additions & 0 deletions spec/rspec/core/backtrace_cleaner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ module RSpec::Core
cleaner = BacktraceCleaner.new([], [])
expect(lines.all? {|line| cleaner.exclude? line}).to be_false
end

it 'is considered a full backtrace' do
expect(BacktraceCleaner.new([], []).full_backtrace).to be_true
end
end

context "with an exclusion pattern but no inclusion patterns" do
Expand All @@ -20,6 +24,10 @@ module RSpec::Core
cleaner = BacktraceCleaner.new([], [/remove/])
expect(cleaner.exclude? "apple").to be_false
end

it 'is considered a partial backtrace' do
expect(BacktraceCleaner.new([], [/remove/]).full_backtrace).to be_false
end
end

context "with an exclusion pattern and an inclusion pattern" do
Expand All @@ -37,6 +45,10 @@ module RSpec::Core
cleaner = BacktraceCleaner.new([/hi/], [/delete/])
expect(cleaner.exclude? "fish").to be_false
end

it 'is considered a partial backtrace' do
expect(BacktraceCleaner.new([], [/remove/]).full_backtrace).to be_false
end
end

context "with an exclusion pattern that matches the current working directory" do
Expand Down
65 changes: 61 additions & 4 deletions spec/rspec/core/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -493,16 +493,26 @@ def specify_consistent_ordering_of_files_to_run
end
end

context "with full_description" do
context "with full_description set" do
it "overrides filters" do
config.filter_run :focused => true
config.full_description = "foo"
expect(config.filter).not_to have_key(:focused)
end

it 'is possible to access the full description regular expression' do
config.full_description = "foo"
expect(config.full_description).to eq /foo/
end
end

context "with line number" do
context "without full_description having been set" do
it 'returns false from #full_description' do
expect(config.full_description).to eq false
end
end

context "with line number" do
it "assigns the file and line number as a location filter" do
config.files_or_directories_to_run = "path/to/a_spec.rb:37"
expect(config.filter).to eq({:locations => {File.expand_path("path/to/a_spec.rb") => [37]}})
Expand Down Expand Up @@ -537,6 +547,11 @@ def specify_consistent_ordering_of_files_to_run
expect(config.filter).to eq({:full_description => Regexp.union(/foo/, /bar/)})
end

it 'is possible to access the full description regular expression' do
config.full_description = "foo","bar"
expect(config.full_description).to eq Regexp.union(/foo/,/bar/)
end

describe "#default_path" do
it 'defaults to "spec"' do
expect(config.default_path).to eq('spec')
Expand Down Expand Up @@ -954,6 +969,17 @@ def metadata_hash(*args)
end
end

describe "line_numbers" do
it "returns the line numbers from the filter" do
config.line_numbers = ['42']
expect(config.line_numbers).to eq [42]
end

it "defaults to empty" do
expect(config.line_numbers).to eq []
end
end

describe "#full_backtrace=" do
context "given true" do
it "clears the backtrace exclusion patterns" do
Expand Down Expand Up @@ -987,6 +1013,18 @@ def metadata_hash(*args)
end
end

describe 'full_backtrace' do
it 'returns true when backtrace patterns is empty' do
config.backtrace_exclusion_patterns = []
expect(config.full_backtrace).to eq true
end

it 'returns false when backtrace patterns isnt empty' do
config.backtrace_exclusion_patterns = [:lib]
expect(config.full_backtrace).to eq false
end
end

describe "#backtrace_clean_patterns" do
it "is deprecated" do
RSpec.stub(:warn_deprecation)
Expand Down Expand Up @@ -1037,6 +1075,7 @@ def metadata_hash(*args)
else
@orig_debugger = nil
end
config.stub(:require)
Object.const_set("Debugger", debugger)
end

Expand All @@ -1053,17 +1092,26 @@ def metadata_hash(*args)
end

it "starts the debugger" do
config.stub(:require)
debugger.should_receive(:start)
config.debug = true
end

it 'sets the reader to true' do
config.debug = true
expect(config.debug).to eq true
end
end

describe "#debug=false" do
it "does not require 'ruby-debug'" do
config.should_not_receive(:require).with('ruby-debug')
config.debug = false
end

it 'sets the reader to false' do
config.debug = false
expect(config.debug).to eq false
end
end

describe "#output=" do
Expand All @@ -1083,6 +1131,15 @@ def metadata_hash(*args)
end
end

describe "libs" do
include_context "isolate load path mutation"

it 'records paths added to the load path' do
config.libs = ["a/dir"]
expect(config.libs).to eq ["a/dir"]
end
end

describe "#requires=" do
before { RSpec.should_receive :deprecate }

Expand All @@ -1095,7 +1152,7 @@ def metadata_hash(*args)
it "stores require paths" do
config.should_receive(:require).with("a/path")
config.requires = ["a/path"]
expect(config.requires).to eq(['a/path'])
expect(config.requires).to eq ['a/path']
end
end

Expand Down