Auto merge of #5509 - brchristian:amber_regressions, r=segiddins

use yellow color when version appears to be moving backwards

This is very much intended as a work in progress (no tests!) and proof-of-concept .
Closes #5506.

In cases where the _dependencies_ change, it is not always possible to tell if they are going forwards or backwards -- for instance a change from `> 2.0` to `< 2.8` is neither obviously a forward or a backward change.

Hence the behavior here is to always default to the green color unless there are two specific version numbers and the new one is strictly less than the old one, in which case we use the yellow color.

I’d love any code review and any suggestions for the right place to add some tests for this. I think the tests should end up being quite simple -- we can check for `\e[32m` (green) versus `e[33m` (yellow) in the strings sent to the shell, and that’s easy enough -- but I’m simply not familiar enough with the test suite to know the appropriate place and syntax to do that. Suggestions welcome!

2 files changed, 43 insertions, 1 deletions

diff --git a/lib/bundler/source.rb b/lib/bundler/source.rb
index 917e9106e0..cf56ed1cc1 100644
--- a/lib/bundler/source.rb
+++ b/lib/bundler/source.rb
@@ -20,7 +20,7 @@ module Bundler
         locked_spec = Bundler.locked_gems.specs.find {|s| s.name == spec.name }
         locked_spec_version = locked_spec.version if locked_spec
         if locked_spec_version && spec.version != locked_spec_version
-          message += Bundler.ui.add_color(" (was #{locked_spec_version})", :green)
+          message += Bundler.ui.add_color(" (was #{locked_spec_version})", version_color(spec.version, locked_spec_version))
         end
       end
 
@@ -38,5 +38,21 @@ module Bundler
     def inspect
       "#<#{self.class}:0x#{object_id} #{self}>"
     end
+
+  private
+
+    def version_color(spec_version, locked_spec_version)
+      if Gem::Version.correct?(spec_version) && Gem::Version.correct?(locked_spec_version)
+        # display yellow if there appears to be a regression
+        earlier_version?(spec_version, locked_spec_version) ? :yellow : :green
+      else
+        # default to green if the versions cannot be directly compared
+        :green
+      end
+    end
+
+    def earlier_version?(spec_version, locked_spec_version)
+      Gem::Version.new(spec_version) < Gem::Version.new(locked_spec_version)
+    end
   end
 end
diff --git a/spec/bundler/source_spec.rb b/spec/bundler/source_spec.rb
index d35a6916a8..08d1698fcd 100644
--- a/spec/bundler/source_spec.rb
+++ b/spec/bundler/source_spec.rb
@@ -71,6 +71,32 @@ RSpec.describe Bundler::Source do
             end
           end
         end
+
+        context "with a more recent version" do
+          let(:spec) { double(:spec, :name => "nokogiri", :version => "1.6.1", :platform => rb) }
+          let(:locked_gem) { double(:locked_gem, :name => "nokogiri", :version => "1.7.0") }
+
+          context "with color" do
+            before { Bundler.ui = Bundler::UI::Shell.new }
+
+            it "should return a string with the locked spec version in yellow" do
+              expect(subject.version_message(spec)).to eq("nokogiri 1.6.1\e[33m (was 1.7.0)\e[0m")
+            end
+          end
+        end
+
+        context "with an older version" do
+          let(:spec) { double(:spec, :name => "nokogiri", :version => "1.7.1", :platform => rb) }
+          let(:locked_gem) { double(:locked_gem, :name => "nokogiri", :version => "1.7.0") }
+
+          context "with color" do
+            before { Bundler.ui = Bundler::UI::Shell.new }
+
+            it "should return a string with the locked spec version in green" do
+              expect(subject.version_message(spec)).to eq("nokogiri 1.7.1\e[32m (was 1.7.0)\e[0m")
+            end
+          end
+        end
       end
 
       context "that do not contain the relevant gem spec" do