diff options
author | Bundlerbot <bot@bundler.io> | 2019-11-04 11:04:25 +0000 |
---|---|---|
committer | David RodrÃguez <deivid.rodriguez@riseup.net> | 2019-11-07 16:41:42 +0100 |
commit | a3260aa9d173388636a16e269da71bdb1464c9c9 (patch) | |
tree | 4e738d3e813f5ae2842211038c58e1386f16da63 /spec/runtime/with_unbundled_env_spec.rb | |
parent | 8adb425350935e8b7a575c30f6aa1bb34ae1e695 (diff) | |
download | bundler-a3260aa9d173388636a16e269da71bdb1464c9c9.tar.gz |
Merge #7401
7401: Stop silencing output by default r=indirect a=deivid-rodriguez
### What was the end-user problem that led to this PR?
The problem was that bundler defaults to a silent UI:
https://github.com/bundler/bundler/blob/e70643c1be3a4417bd537d7e63470265465e693e/lib/bundler.rb#L66-L68
In my opinion, this isn't a good behavior for a CLI tool, and forces us to override it in many many different places. It has also caused several issues, for example, https://github.com/bundler/bundler/issues/3710 where `bundle list` was printing nothing.
The [solution to that issues](https://github.com/bundler/bundler/pull/3707) led us to add yet more places where we override the default UI, and @indirect [predicting that having to unset the UI everytime we want it to not be silent](https://github.com/bundler/bundler/pull/3707#issuecomment-108646127) would cause many headaches.
Well, yeah... I've lost a lot of time trying to figure out why UI was silent sometimes, and normal another times, why some specs printed warnings and some didn't. In particular, see my series of "big fail PRs" fighting against bundler's UI: https://github.com/bundler/bundler/pull/7284, https://github.com/bundler/bundler/pull/7294, https://github.com/bundler/bundler/pull/7305, https://github.com/bundler/bundler/pull/7362.
Another series of issues/PRs probably related to this is issue https://github.com/bundler/bundler/issues/6900, where the output would use a different UI on different environments. We had a lot of trouble to reliably fix it (https://github.com/bundler/bundler/pull/6994, https://github.com/bundler/bundler/pull/7002, https://github.com/bundler/bundler/issues/7253).
I also run into these issues again when trying out the `RUBYGEMS_GEMDEPS` environment variable that enables `bundler/setup` from rubygems.
### What was your diagnosis of the problem?
My diagnosis was that we shouldn't silence UI by default.
### What is your fix for the problem, implemented in this PR?
My fix is to, instead of silencing and then overriding the default shell at many places, don't silence it by default and instead make it silent when needed. By doing this, I managed to get 100% of our specs green, so I'm pretty confident that the output is still the same (or if it's not, it's probably because some output/errors where being unintentionally swallowed).
Now specs should pass, but they print a bunch of output to the screen. You can see error messages, hard crashes, success messages... Some of them might be showing actual issues with either the code or tests, so I plan to go through each of them and review them. I can do that in this PR or separately, no strong opinion.
Co-authored-by: David RodrÃguez <deivid.rodriguez@riseup.net>
(cherry picked from commit 1a585f5cd760ae4be6930fd38d262677faa3694c)
Diffstat (limited to 'spec/runtime/with_unbundled_env_spec.rb')
-rw-r--r-- | spec/runtime/with_unbundled_env_spec.rb | 24 |
1 files changed, 9 insertions, 15 deletions
diff --git a/spec/runtime/with_unbundled_env_spec.rb b/spec/runtime/with_unbundled_env_spec.rb index 05f3c8e3ab..254f493042 100644 --- a/spec/runtime/with_unbundled_env_spec.rb +++ b/spec/runtime/with_unbundled_env_spec.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -require_relative "../support/streams" - RSpec.describe "Bundler.with_env helpers" do def bundle_exec_ruby!(code, options = {}) build_bundler_context options @@ -129,16 +127,16 @@ RSpec.describe "Bundler.with_env helpers" do describe "Bundler.with_clean_env", :bundler => 2 do it "should set ENV to unbundled_env in the block" do expected = Bundler.unbundled_env - actual = nil - capture(:stderr) do - actual = Bundler.with_clean_env { ENV.to_hash } + + actual = Bundler.ui.silence do + Bundler.with_clean_env { ENV.to_hash } end expect(actual).to eq(expected) end it "should restore the environment after execution" do - capture(:stderr) do + Bundler.ui.silence do Bundler.with_clean_env { ENV["FOO"] = "hello" } end @@ -181,9 +179,7 @@ RSpec.describe "Bundler.with_env helpers" do describe "Bundler.clean_system", :bundler => 2 do let(:code) do <<~RUBY - capture(:stderr) do - Bundler.clean_system(%([ "\$BUNDLE_FOO" = "bar" ] || exit 42)) - end + Bundler.ui.silence { Bundler.clean_system(%([ "\$BUNDLE_FOO" = "bar" ] || exit 42)) } exit $?.exitstatus RUBY @@ -191,7 +187,7 @@ RSpec.describe "Bundler.with_env helpers" do it "runs system inside with_clean_env" do lib = File.expand_path("../../lib", __dir__) - system({ "BUNDLE_FOO" => "bar" }, "ruby -I#{lib}:#{spec} -rsupport/streams -rbundler -e '#{code}'") + system({ "BUNDLE_FOO" => "bar" }, "ruby -I#{lib} -rbundler -e '#{code}'") expect($?.exitstatus).to eq(42) end end @@ -237,10 +233,8 @@ RSpec.describe "Bundler.with_env helpers" do describe "Bundler.clean_exec", :bundler => 2 do let(:code) do <<~RUBY - capture(:stderr) do - Process.fork do - exit Bundler.clean_exec(%(test "\$BUNDLE_FOO" = "bar")) - end + Process.fork do + exit Bundler.ui.silence { Bundler.clean_exec(%(test "\$BUNDLE_FOO" = "bar")) } end _, status = Process.wait2 @@ -253,7 +247,7 @@ RSpec.describe "Bundler.with_env helpers" do skip "Fork not implemented" if Gem.win_platform? lib = File.expand_path("../../lib", __dir__) - system({ "BUNDLE_FOO" => "bar" }, "ruby -I#{lib}:#{spec} -rsupport/streams -rbundler -e '#{code}'") + system({ "BUNDLE_FOO" => "bar" }, "ruby -I#{lib} -rbundler -e '#{code}'") expect($?.exitstatus).to eq(1) end end |