Merge pull request #2088 from rspec/find-failed-line-improvement

Improve logic that finds the failure line to print.

Author: myronmarston

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:
 

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<String>]
+      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 << f # the registered files are already expended absolute paths
+        end
+
         files_to_run.uniq.each do |f|
           file = File.expand_path(f)
           load file

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)

lib/rspec/core/metadata.rb

@@ -147,12 +147,13 @@ def populate_location_attributes
                                    end
 
           relative_file_path            = Metadata.relative_path(file_path)
+          absolute_file_path            = File.expand_path(relative_file_path)
           metadata[:file_path]          = relative_file_path
           metadata[:line_number]        = line_number.to_i
           metadata[:location]           = "#{relative_file_path}:#{line_number}"
-          metadata[:absolute_file_path] = File.expand_path(relative_file_path)
+          metadata[:absolute_file_path] = absolute_file_path
           metadata[:rerun_file_path]  ||= relative_file_path
-          metadata[:scoped_id]          = build_scoped_id_for(relative_file_path)
+          metadata[:scoped_id]          = build_scoped_id_for(absolute_file_path)
         end
 
         def file_path_and_line_number_from(backtrace)

lib/rspec/core/rake_task.rb

@@ -1,5 +1,6 @@
 require 'rake'
 require 'rake/tasklib'
+require 'rspec/support'
 require 'rspec/support/ruby_features'
 require 'rspec/core/shell_escape'
 

lib/rspec/core/world.rb

@@ -40,12 +40,17 @@ 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.
       def register(example_group)
         example_groups << example_group
-        @example_group_counts_by_spec_file[example_group.metadata[:file_path]] += 1
+        @example_group_counts_by_spec_file[example_group.metadata[:absolute_file_path]] += 1
         example_group
       end
 

spec/integration/failed_line_detection_spec.rb

@@ -0,0 +1,158 @@
+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
+    "
+
+    file = in_current_dir { "#{Dir.pwd}/failing_spec.rb" }
+    load file
+    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

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

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 `<main>': foo (RuntimeError)
@@ -159,7 +159,7 @@ def expected_summary_output_for_example_specs
         |     # ./spec/support/sandboxing.rb:7:in `block (2 levels) in <top (required)>'
         |
         |  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 `<main>': foo (RuntimeError)