search

wincent / command-t

⌨️ Fast file navigation for Neovim and Vim
BSD 2-Clause "Simplified" License
2.74k stars 317 forks source link

Use rspec-mock for stubbing. #375

Closed voxik closed 3 years ago

voxik commented 3 years ago

Since RR gem was updated in Fedora from 1.1.2 to 1.2.1, the test suite fails with errors such as:

Failures:
  1) CommandT::Controller accept selection opens relative paths inside the working directory
     Failure/Error: stub(finder).path = anything
     NoMethodError:
       undefined method `stub' for #<RSpec::ExampleGroups::CommandTController::AcceptSelection "opens relative paths inside the working directory" (./spec/command-t/controller_spec.rb:23)>
     # ./spec/command-t/controller_spec.rb:62:in `stub_finder'
     # ./spec/command-t/controller_spec.rb:12:in `block (3 levels) in <top (required)>'

... snip ...

Finished in 0.04789 seconds (files took 0.14141 seconds to load)
121 examples, 26 failures, 1 pending

... snip ...

This seems to be due some backward incompatible changes in RR together with RR probably being incompatible with RSpec 3.x.

Therefore, I have used rr-to-rspec-converter with a several small tweaks and converted the test suite to use rspec-mocks. Looking at the result, I don't find the output too different or harder to read, therefore I propose to change the mock framework from RR to rspec-mocks. This allows to reduce the dependency chain, which is always good thing IMO.

wincent commented 3 years ago

Thanks @voxik. I have no special attachment to rr (I chose it eons ago because at the time it seemed more expressive, but RSpec mocks seem to have come a long way since then), so this seems like a fine change.

wincent commented 3 years ago

I'm going to grab that CI commit and try it separately. Thanks.

voxik commented 3 years ago

Ups, that was close. I was fighting with the test case you fixed along the way and pushed commit trying to enable GH actions to execute the test suite right at the time you have merged this, ufff :) Thx for merging. I'll try to open another PR trying to add the CI

wincent commented 3 years ago

Don't worry about it, I already grabbed it and merged it via e7817060e761f7e07. First time I have ever merged "the same PR" twice... 😂

You set it to run push, and pull_request, but it didn't seem to run when I pushed just now. Will see if it runs on next PR or push.