rubygems / bundler

Manage your Ruby application's gem dependencies
https://bundler.io
MIT License
4.88k stars 1.99k forks source link

Fix running compact client updater spec in isolation #7592

Closed deivid-rodriguez closed 4 years ago

deivid-rodriguez commented 4 years ago

What was the end-user or developer problem that led to this PR?

The problem is the following error when running tests:

$ bin/rspec ./spec/bundler/compact_index_client/updater_spec.rb:47

Randomized with seed 40401
/home/deivid/.rbenv/versions/2.7.0/lib/ruby/2.7.0/tmpdir.rb:81: warning: method redefined; discarding old mktmpdir
/home/deivid/.rbenv/versions/2.7.0/lib/ruby/gems/2.7.0/gems/rspec-mocks-3.9.1/lib/rspec/mocks/method_double.rb:63: warning: previous definition of mktmpdir was here
F

Retried examples: 0

Failures:

  1) Bundler::CompactIndexClient::Updater when bundler doesn't have permissions on Dir.tmpdir Errno::EACCES is raised
     Failure/Error:
       expect do
         updater.update(local_path, remote_path)
       end.to raise_error(Bundler::PermissionError)

       expected Bundler::PermissionError, got #<RSpec::Mocks::MockExpectationError: #<Double :fetcher> received unexpected message :call with (#<Double :remote_path>, {"Accept-Encoding"=>"gzip"})> with backtrace:
         # ./spec/bundler/compact_index_client/updater_spec.rb:51:in `block (4 levels) in <top (required)>'
         # ./spec/bundler/compact_index_client/updater_spec.rb:50:in `block (3 levels) in <top (required)>'
         # ./spec/spec_helper.rb:109:in `block (3 levels) in <top (required)>'
         # ./spec/spec_helper.rb:109:in `block (2 levels) in <top (required)>'
         # ./spec/spec_helper.rb:76:in `block (2 levels) in <top (required)>'
         # ./spec/support/rubygems_ext.rb:98:in `load'
         # ./spec/support/rubygems_ext.rb:98:in `gem_load_and_activate'
         # ./spec/support/rubygems_ext.rb:45:in `gem_load'
     # ./spec/bundler/compact_index_client/updater_spec.rb:50:in `block (3 levels) in <top (required)>'
     # ./spec/spec_helper.rb:109:in `block (3 levels) in <top (required)>'
     # ./spec/spec_helper.rb:109:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:76:in `block (2 levels) in <top (required)>'
     # ./spec/support/rubygems_ext.rb:98:in `load'
     # ./spec/support/rubygems_ext.rb:98:in `gem_load_and_activate'
     # ./spec/support/rubygems_ext.rb:45:in `gem_load'

Finished in 0.24158 seconds (files took 0.21522 seconds to load)
1 example, 1 failure

Failed examples:

rspec ./spec/bundler/compact_index_client/updater_spec.rb:47 # Bundler::CompactIndexClient::Updater when bundler doesn't have permissions on Dir.tmpdir Errno::EACCES is raised

Randomized with seed 40401

What is your fix for the problem, implemented in this PR?

My diagnosis was that it's the Updater#initialize method that requires tmpdir making Dir.mktmpdir available. In the offending spec, first we stub Dir.mktmpdir, and then we initialize the updater, requiring tmpdir, and "undoing the stub". That means the test no longer does what it's supposed to.

So, my fix is to early instantiate the update, so that by the time we stub Dir.mktmpdir, tmpdir has already been required, so the stub is not reverted.

deivid-rodriguez commented 4 years ago

@bundlerbot merge

ghost commented 4 years ago

Build succeeded