[rubygems/rubygems] Modify `bundle doctor` to not report issue when files aren't writable:

- ### Problem

  Running `bundle doctor` warn about files that aren't writable.
  This makes the output of `bundle doctor` very verbose for something
  I believe isn't really an issue.

  ### Context

  Rubygems keeps the files original permission at the time the gem
  is packaged.
  Many gem maintainers have decided that the permissions of the files
  in their bundled would be 0444, this includes amongst others:
  minitest, selenium, brakeman...

  Any git gems that had a 0444 permissions at some point in its git
  history would also be reported (as bundle doctor look in the
  `cache/bundler/git/<gem>/object` path).

  While it completely make sense to report when files aren't readable,
  maybe it's worth questioning the usefulness of reporting files
  that can't be written and what problem this causes to the user
  (if any).

  ### Solution

  Removed the check for unwritable file.

  ### Side note

  I also tweaked the "No issues ..." message logic as it was doing
  the opposite (reporting an issue when there is none and vice versa).
  This wasn't caught in tests because as a stub on `Bundler.ui.info`
  was missing.

https://github.com/rubygems/rubygems/commit/9a426b9495

1 files changed, 22 insertions, 9 deletions

diff --git a/spec/bundler/commands/doctor_spec.rb b/spec/bundler/commands/doctor_spec.rb
index c040602bde..7c30d0e91b 100644
--- a/spec/bundler/commands/doctor_spec.rb
+++ b/spec/bundler/commands/doctor_spec.rb
@@ -14,7 +14,7 @@ RSpec.describe "bundle doctor" do
 
     @stdout = StringIO.new
 
-    [:error, :warn].each do |method|
+    [:error, :warn, :info].each do |method|
       allow(Bundler.ui).to receive(method).and_wrap_original do |m, message|
         m.call message
         @stdout.puts message
@@ -45,20 +45,20 @@ RSpec.describe "bundle doctor" do
       allow(File).to receive(:writable?).with(File.expand_path("..", Gem.default_dir))
     end
 
-    it "exits with no message if the installed gem has no C extensions" do
+    it "exits with a success message if the installed gem has no C extensions" do
       doctor = Bundler::CLI::Doctor.new({})
       allow(doctor).to receive(:lookup_with_fiddle).and_return(false)
       expect { doctor.run }.not_to raise_error
-      expect(@stdout.string).to be_empty
+      expect(@stdout.string).to include("No issues")
     end
 
-    it "exits with no message if the installed gem's C extension dylib breakage is fine" do
+    it "exits with a success message if the installed gem's C extension dylib breakage is fine" do
       doctor = Bundler::CLI::Doctor.new({})
       expect(doctor).to receive(:bundles_for_gem).exactly(2).times.and_return ["/path/to/myrack/myrack.bundle"]
       expect(doctor).to receive(:dylibs).exactly(2).times.and_return ["/usr/lib/libSystem.dylib"]
       allow(doctor).to receive(:lookup_with_fiddle).with("/usr/lib/libSystem.dylib").and_return(false)
       expect { doctor.run }.not_to raise_error
-      expect(@stdout.string).to be_empty
+      expect(@stdout.string).to include("No issues")
     end
 
     it "exits with a message if one of the linked libraries is missing" do
@@ -105,7 +105,7 @@ RSpec.describe "bundle doctor" do
       allow(File).to receive(:stat).with(@unwritable_file) { @stat }
     end
 
-    it "exits with an error if home contains files that are not readable/writable" do
+    it "exits with an error if home contains files that are not readable" do
       doctor = Bundler::CLI::Doctor.new({})
       allow(doctor).to receive(:lookup_with_fiddle).and_return(false)
       allow(@stat).to receive(:uid) { Process.uid }
@@ -113,11 +113,24 @@ RSpec.describe "bundle doctor" do
       allow(File).to receive(:readable?).with(@unwritable_file) { false }
       expect { doctor.run }.not_to raise_error
       expect(@stdout.string).to include(
-        "Files exist in the Bundler home that are not readable/writable by the current user. These files are:\n - #{@unwritable_file}"
+        "Files exist in the Bundler home that are not readable by the current user. These files are:\n - #{@unwritable_file}"
       )
       expect(@stdout.string).not_to include("No issues")
     end
 
+    it "exits without an error if home contains files that are not writable" do
+      doctor = Bundler::CLI::Doctor.new({})
+      allow(doctor).to receive(:lookup_with_fiddle).and_return(false)
+      allow(@stat).to receive(:uid) { Process.uid }
+      allow(File).to receive(:writable?).with(@unwritable_file) { false }
+      allow(File).to receive(:readable?).with(@unwritable_file) { true }
+      expect { doctor.run }.not_to raise_error
+      expect(@stdout.string).not_to include(
+        "Files exist in the Bundler home that are not readable by the current user. These files are:\n - #{@unwritable_file}"
+      )
+      expect(@stdout.string).to include("No issues")
+    end
+
     context "when home contains files that are not owned by the current process", :permissions do
       before(:each) do
         allow(@stat).to receive(:uid) { 0o0000 }
@@ -130,7 +143,7 @@ RSpec.describe "bundle doctor" do
         allow(File).to receive(:readable?).with(@unwritable_file) { false }
         expect { doctor.run }.not_to raise_error
         expect(@stdout.string).to include(
-          "Files exist in the Bundler home that are owned by another user, and are not readable/writable. These files are:\n - #{@unwritable_file}"
+          "Files exist in the Bundler home that are owned by another user, and are not readable. These files are:\n - #{@unwritable_file}"
         )
         expect(@stdout.string).not_to include("No issues")
       end
@@ -142,7 +155,7 @@ RSpec.describe "bundle doctor" do
         allow(File).to receive(:readable?).with(@unwritable_file) { true }
         expect { doctor.run }.not_to raise_error
         expect(@stdout.string).to include(
-          "Files exist in the Bundler home that are owned by another user, but are still readable/writable. These files are:\n - #{@unwritable_file}"
+          "Files exist in the Bundler home that are owned by another user, but are still readable. These files are:\n - #{@unwritable_file}"
         )
         expect(@stdout.string).not_to include("No issues")
       end