Backtrace is not shown on windows #57

Closed
voxik opened this Issue Jul 2, 2010 · 17 comments

Projects

None yet

3 participants

@voxik
Contributor
voxik commented Jul 2, 2010

This report is shown when the test case fails:

  1. ProjectsController GET index assigns all projects as @projects
    Failure/Error: Unable to find C to read failed line
    undefined method `get' for #RSpec::Core::ExampleGroup::Nested_1::Nested_1:0x47039e8

    ./spec/controllers/projects_controller_spec.rb:12:in`block (3 levels) in <top (required)>'

However, the backtrace should be the correct result I suppose. This seems to be very similar issues as http://github.com/rspec/rspec-core/issues/closed/#issue/52

@voxik
Contributor
voxik commented Jul 2, 2010

Sorry, this seems to be buplication of #56 and also #45. The problem should exist just on windows from my point of view and it is located in read_failed_line method from base_formatter.rb

@voxik
Contributor
voxik commented Jul 2, 2010

Actually, also the original_file = example.file_path.to_s.downcase already contains corrupted file path, so I guess the problem should be tracked also in metadata.rb in file_and_line_number method, which is suspicious to me.

@voxik
Contributor
voxik commented Jul 2, 2010

This might be solution:

  def file_and_line_number(metadata)
    entry = candidate_entries_from_caller(metadata).first
    if entry
      path = Pathname.new(entry).cleanpath.to_s
      entry = entry.gsub(/#{path + ':'}/, '')
    end
    entry && [path, entry.split(":")].flatten
  end

and

    def read_failed_line(exception, example)
      original_file = example.file_path.to_s.downcase
      matching_line = exception.backtrace.detect { |line| Pathname.new(line).cleanpath.to_s.downcase == original_file.downcase }
      return "Unable to find matching line from backtrace" if matching_line.nil?

      file_path = Pathname.new(matching_line).cleanpath.to_s
      line_number = matching_line.split(':')[-2]
      if File.exist?(file_path)
        open(file_path, 'r') { |f| f.readlines[line_number.to_i - 1] }
      else
        "Unable to find #{file_path} to read failed line"
      end
    end
@dchelimsky
Member

@voxik wanna make that a proper patch

@voxik
Contributor
voxik commented Jul 2, 2010

Unfortunately, I don't have more time right now. If you can wait few days ....

@dchelimsky
Member

@voxik - got your pull request for http://github.com/voxik/rspec-core/commit/0f0c24b44bee64abaf0fe93b6a37c03170ea219a. Please add specs and I'll be happy to merge it. They should show that everything works regardless of the format of the path (i.e. same examples for *nix format and Windows format).

Please just add a comment here with a link to the relevant commit(s) rather than a pull request (easier to keep track of the thread).

Thanks!

@voxik
Contributor
voxik commented Jul 5, 2010

Originally I wanted to write some dedicated spec, however I discovered that this two patches fixes 9 specs which were failing and that satisfied me. Nevertheless, I could try to do something more specific.

@dchelimsky
Member

They may have been failing on Windows, but I'm not running in Windows environment, nor do I have one accessible for my regular use :) So the specs need to operate above the OS. There was another path-related issue that cropped up recently - check out the specs for that fix for some guidance if you like: http://github.com/rspec/rspec-core/commit/97a3786a82dd0c7a42de6a30f42d0074ce3f477d

@voxik
Contributor
voxik commented Jul 5, 2010

So if you could merge these three commits:
http://github.com/voxik/rspec-core/commit/7181494c0d76cfa62ed475d277192011fd28e640

http://github.com/voxik/rspec-core/commit/c1f3f2458c07e42c13340e8e7911223d2264674b

http://github.com/voxik/rspec-core/commit/a0957cb45fb841a5b4932241ee699ee96be177dc

However please note that in the last patch I'm testing just methods I added. I don't like to implement specs of whole complex method.

@dchelimsky
Member

Additional specs for the methods you're adding/modifying is sufficient. Only problem is when I merge your changes I get 19 failures with Ruby 1.8.7 and 20 with Ruby 1.9.1 and 1.9.2 (see http://gist.github.com/465311).

I'm not going to have much time to figure out what's up until the weekend, so if you have the inclination, time and access to both *nix and windows envs, please do resubmit with specs that pass in both. If not, I'll look at it when I can.

@voxik
Contributor
voxik commented Jul 6, 2010

I see ... the problem is that I used Pathname.new which is platform dependent. I.e. it parses windows path just fine on windows, however it cannot parse the windows path on *nix. So although the specs are passing on Windows, they fail on *nix. That's a pity.

The biggest surprise for me is that Pathname.new("/path/file.html.erb_spec.rb:14").cleanpath.to_s apparently doesn't provide the same result on Windows and *nix

@voxik
Contributor
voxik commented Jul 8, 2010

The Pathname#cleanpath utilizes File.basename and this method's behavior differs by platform ... the funny thing is that "projects_controller_spec.rb:12:in `block (3 levels) in <top (required)>'" might be valid filename on *nix, even though it is not really sane one ;)

@voxik
Contributor
voxik commented Jul 20, 2010

One solution would be checking if file really exists on file system ...

@luislavena
Contributor

Hello guys,

Been late to chime in on this, but can't the following match solve the issue?

file_path, line_number = matching_line.match(/(.*):(\d+)/)[1..2]

That works in 1.8.7, 1.9.1, 1.9.2 and trunk on both Windows and Linux.

I would suggest keep the additional test for the drive, and only introduce the regex change.

Please let me know if that is enough or a formal patch is required.

Thank you.

@luislavena
Contributor

Hello,

Patch and results here:
http://gist.github.com/488393

There is a bogus Ruby 1.9 failure, but is completely unrelated to this issue.

I've applied this patch and report properly on Windows.

Cheers.

@luislavena
Contributor

Actually, the bogus Ruby 1.9 failure was my fault. Patch updated with better regexp to handle that case.

There are no failures now.

@timcharper timcharper pushed a commit to timcharper/rspec-core that referenced this issue Aug 19, 2011
@voxik @dchelimsky voxik + dchelimsky Added Windows specs [Issue #57] 1705d48
@timcharper timcharper pushed a commit to timcharper/rspec-core that referenced this issue Aug 19, 2011
@luislavena @dchelimsky luislavena + dchelimsky Fix backtrace regexps to work on Windows.
- Closes #57.
8ad9cfd
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment