From aadd339c9ebdbffcfad52b0a94c435b787ffdd66 Mon Sep 17 00:00:00 2001 From: Myron Marston Date: Mon, 12 Oct 2015 23:56:54 -0700 Subject: [PATCH] Improve logic that finds the failure line to print. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Introduce `project_source_dirs` config setting. - Look for the first backtrace line in one of the `project_source_dirs` rather than the first line from your spec file. This helps in situations where you define a helper method in a support file that has a failing expectation, and call it from your spec. Previously it would have shown the helper method call site rather than the expectation in the helper method itself. - If no backtrace line can be found in a `project_source_dirs`, pick the first backtrace line. While we don’t generally want to show lines from gems, it’s better than showing no line at all. Fixes #1991. --- Changelog.md | 10 ++ lib/rspec/core/configuration.rb | 36 +++- .../core/formatters/exception_presenter.rb | 9 +- lib/rspec/core/world.rb | 5 + .../integration/failed_line_detection_spec.rb | 157 ++++++++++++++++++ spec/rspec/core/configuration_spec.rb | 6 + spec/support/formatter_support.rb | 6 +- 7 files changed, 222 insertions(+), 7 deletions(-) create mode 100644 spec/integration/failed_line_detection_spec.rb diff --git a/Changelog.md b/Changelog.md index a921efb102..f8e8ed3fbc 100644 --- a/Changelog.md +++ b/Changelog.md @@ -26,6 +26,16 @@ Enhancements: output when a `cause` is available. (Adam Magan) * Stop rescuing `NoMemoryError`, `SignalExcepetion`, `Interrupt` and `SystemExit`. It is dangerous to interfere with these. (Myron Marston, #2063) +* Add `config.project_source_dirs` setting which RSpec uses to determine + if a backtrace line comes from your project source or from some + external library. It defaults to `spec`, `lib` and `app` but can be + configured differently. (Myron Marston, #2088) +* Improve failure line detection so that it looks for the failure line + in any project source directory instead of just in the spec file. + In addition, if no backtrace lines can be found from a project source + file, we fall back to displaying the source of the first backtrace + line. This should virtually eliminate the "Unable to find matching + line from backtrace" messages. (Myron Marston, #2088) Bug Fixes: diff --git a/lib/rspec/core/configuration.rb b/lib/rspec/core/configuration.rb index d488363017..03bebaef82 100644 --- a/lib/rspec/core/configuration.rb +++ b/lib/rspec/core/configuration.rb @@ -100,7 +100,11 @@ def self.add_read_only_setting(name, opts={}) # # @note Other scripts invoking `rspec` indirectly will ignore this # setting. - add_setting :default_path + add_read_only_setting :default_path + def default_path=(path) + project_source_dirs << path + @default_path = path + end # @macro add_setting # Run examples over DRb (default: `false`). RSpec doesn't supply the DRb @@ -241,6 +245,16 @@ def exclude_pattern=(value) update_pattern_attr :exclude_pattern, value end + # @macro add_setting + # Specifies which directories contain the source code for your project. + # When a failure occurs, RSpec looks through the backtrace to find a + # a line of source to print. It first looks for a line coming from + # one of the project source directories so that, for example, it prints + # the expectation or assertion call rather than the source code from + # the expectation or assertion framework. + # @return [Array] + add_setting :project_source_dirs + # @macro add_setting # Report the times for the slowest examples (default: `false`). # Use this to specify the number of examples to include in the profile. @@ -353,6 +367,7 @@ def initialize @backtrace_formatter = BacktraceFormatter.new @default_path = 'spec' + @project_source_dirs = %w[ spec lib app ] @deprecation_stream = $stderr @output_stream = $stdout @reporter = nil @@ -1274,6 +1289,15 @@ def requires=(paths) @requires += paths end + # @private + def in_project_source_dir_regex + regexes = project_source_dirs.map do |dir| + /\A#{Regexp.escape(File.expand_path(dir))}\// + end + + Regexp.union(regexes) + end + # @private if RUBY_VERSION.to_f >= 1.9 # @private @@ -1315,6 +1339,16 @@ def configure_expectation_framework # @private def load_spec_files + # Note which spec files world is already aware of. + # This is generally only needed for when the user runs + # `ruby path/to/spec.rb` (and loads `rspec/autorun`) -- + # in that case, the spec file was loaded by `ruby` and + # isn't loaded by us here so we only know about it because + # of an example group being registered in it. + RSpec.world.registered_example_group_files.each do |f| + loaded_spec_files << File.expand_path(f) + end + files_to_run.uniq.each do |f| file = File.expand_path(f) load file diff --git a/lib/rspec/core/formatters/exception_presenter.rb b/lib/rspec/core/formatters/exception_presenter.rb index 5635c6cbca..eea15f2d04 100644 --- a/lib/rspec/core/formatters/exception_presenter.rb +++ b/lib/rspec/core/formatters/exception_presenter.rb @@ -165,11 +165,14 @@ def read_failed_line end def find_failed_line - example_path = example.metadata[:absolute_file_path].downcase + line_regex = RSpec.configuration.in_project_source_dir_regex + loaded_spec_files = RSpec.configuration.loaded_spec_files + exception_backtrace.find do |line| next unless (line_path = line[/(.+?):(\d+)(|:\d+)/, 1]) - File.expand_path(line_path).downcase == example_path - end + path = File.expand_path(line_path) + loaded_spec_files.include?(path) || path =~ line_regex + end || exception_backtrace.first end def formatted_message_and_backtrace(colorizer, indentation) diff --git a/lib/rspec/core/world.rb b/lib/rspec/core/world.rb index 45fe203830..b2b7e2bbb6 100644 --- a/lib/rspec/core/world.rb +++ b/lib/rspec/core/world.rb @@ -40,6 +40,11 @@ def filter_manager @configuration.filter_manager end + # @private + def registered_example_group_files + @example_group_counts_by_spec_file.keys + end + # @api private # # Register an example group. diff --git a/spec/integration/failed_line_detection_spec.rb b/spec/integration/failed_line_detection_spec.rb new file mode 100644 index 0000000000..169fa1f44a --- /dev/null +++ b/spec/integration/failed_line_detection_spec.rb @@ -0,0 +1,157 @@ +require 'support/aruba_support' + +RSpec.describe 'Failed line detection' do + include_context "aruba support" + before { clean_current_dir } + + it "finds the source of a failure in a spec file that is defined at the current directory instead of in the normal `spec` subdir" do + write_file "the_spec.rb", " + RSpec.describe do + it 'fails via expect' do + expect(1).to eq(2) + end + end + " + + run_command "the_spec.rb" + expect(last_cmd_stdout).to include("expect(1).to eq(2)") + end + + it "finds the source of a failure in a spec file loaded by running `ruby file` rather than loaded directly by RSpec" do + write_file "passing_spec.rb", " + RSpec.describe do + example { } + end + " + + write_file "failing_spec.rb", " + RSpec.describe do + it 'fails via expect' do + expect(1).to eq(2) + end + end + " + + in_current_dir { load "failing_spec.rb" } + run_command "passing_spec.rb" + + expect(last_cmd_stdout).to include("expect(1).to eq(2)") + end + + it "finds the direct source of failure in any lib, app or spec file, and allows the user to configure what is considered a project source dir" do + write_file "lib/lib_mod.rb", " + module LibMod + def self.trigger_failure + raise 'LibMod failure' + end + end + " + + write_file "app/app_mod.rb", " + module AppMod + def self.trigger_failure + raise 'AppMod failure' + end + end + " + + write_file "spec/support/spec_support.rb", " + module SpecSupport + def self.trigger_failure + raise 'SpecSupport failure' + end + end + " + + write_file "spec/default_config_spec.rb", " + require './lib/lib_mod' + require './spec/support/spec_support' + require './app/app_mod' + + RSpec.describe do + example('1') { LibMod.trigger_failure } + example('2') { AppMod.trigger_failure } + example('3') { SpecSupport.trigger_failure } + end + " + + run_command "./spec/default_config_spec.rb" + + expect(last_cmd_stdout).to include("raise 'LibMod failure'"). + and include("raise 'AppMod failure'"). + and include("raise 'SpecSupport failure'"). + and exclude("AppMod.trigger_failure") + + write_file "spec/change_config_spec.rb", " + require './app/app_mod' + + RSpec.configure do |c| + c.project_source_dirs = %w[ lib spec ] + end + + RSpec.describe do + example('1') { AppMod.trigger_failure } + end + " + + run_command "./spec/change_config_spec.rb" + + expect(last_cmd_stdout).to include("AppMod.trigger_failure"). + and exclude("raise 'AppMod failure'") + end + + it "finds the callsite of a method provided by a gem that fails (rather than the line in the gem)" do + write_file "vendor/gems/assertions/lib/assertions.rb", " + module Assertions + AssertionFailed = Class.new(StandardError) + + def assert(value, msg) + raise(AssertionFailed, msg) unless value + end + end + " + + write_file "spec/unit/the_spec.rb", " + require './vendor/gems/assertions/lib/assertions' + + RSpec.describe do + include Assertions + + it 'fails via assert' do + assert false, 'failed assertion' + end + + it 'fails via expect' do + expect(1).to eq(2) + end + end + " + + run_command "" + + expect(last_cmd_stdout).to include("assert false, 'failed assertion'"). + and include("expect(1).to eq(2)"). + and exclude("raise(AssertionFailed, msg)") + end + + it "falls back to finding a line in a gem when there are no backtrace lines in the app, lib or spec directories" do + write_file "vendor/gems/before_failure/lib/before_failure.rb", " + RSpec.configure do |c| + c.before { raise 'before failure!' } + end + " + + write_file "spec/unit/the_spec.rb", " + require './vendor/gems/before_failure/lib/before_failure' + + RSpec.describe do + example('1') { } + end + " + + run_command "" + + expect(last_cmd_stdout).to include("c.before { raise 'before failure!' }"). + and exclude("Unable to find matching line from backtrace") + end +end diff --git a/spec/rspec/core/configuration_spec.rb b/spec/rspec/core/configuration_spec.rb index b3f18b700c..12de32d95b 100644 --- a/spec/rspec/core/configuration_spec.rb +++ b/spec/rspec/core/configuration_spec.rb @@ -856,6 +856,12 @@ def specify_consistent_ordering_of_files_to_run it 'defaults to "spec"' do expect(config.default_path).to eq('spec') end + + it 'adds to the `project_source_dirs`' do + expect { + config.default_path = 'test' + }.to change { config.project_source_dirs.include?('test') }.from(false).to(true) + end end describe "#include" do diff --git a/spec/support/formatter_support.rb b/spec/support/formatter_support.rb index 88a11fafed..b81ccd0b2d 100644 --- a/spec/support/formatter_support.rb +++ b/spec/support/formatter_support.rb @@ -79,13 +79,13 @@ def expected_summary_output_for_example_specs | # ./spec/support/sandboxing.rb:7 | | 3) a failing spec with odd backtraces fails with a backtrace that has no file - | Failure/Error: Unable to find matching line from backtrace + | Failure/Error: Unable to find (erb) to read failed line | RuntimeError: | foo | # (erb):1 | | 4) a failing spec with odd backtraces fails with a backtrace containing an erb file - | Failure/Error: Unable to find matching line from backtrace + | Failure/Error: Unable to find /foo.html.erb to read failed line | Exception: | Exception | # /foo.html.erb:1:in `
': foo (RuntimeError) @@ -159,7 +159,7 @@ def expected_summary_output_for_example_specs | # ./spec/support/sandboxing.rb:7:in `block (2 levels) in ' | | 4) a failing spec with odd backtraces fails with a backtrace containing an erb file - | Failure/Error: Unable to find matching line from backtrace + | Failure/Error: Unable to find /foo.html.erb to read failed line | Exception: | Exception | # /foo.html.erb:1:in `
': foo (RuntimeError)