Refactoring of @lucasmazza fix w/ specs.try

The only existing spec coverage was essentially integration level and
there was no way either @lucasmazza or myself could find a way to
simulate the bug context.

I extracted some of the code out of outdated into Definition and
SpecSet and added unit specs to those extracted bits.

Closes: #5171
Approved by: <try>

5 files changed, 64 insertions, 25 deletions

diff --git a/lib/bundler/cli/outdated.rb b/lib/bundler/cli/outdated.rb
index a97cfeae4b..efbfe7f89b 100644
--- a/lib/bundler/cli/outdated.rb
+++ b/lib/bundler/cli/outdated.rb
@@ -32,7 +32,7 @@ module Bundler
       end
 
       Bundler::CLI::Common.configure_gem_version_promoter(Bundler.definition, options)
-      # the patch level options don't work without strict also being true
+      # the patch level options imply strict is also true. It wouldn't make sense otherwise.
       strict = options[:strict] || Bundler::CLI::Common.patch_level_options(options).any?
 
       definition_resolution = proc { options[:local] ? definition.resolve_with_cache! : definition.resolve_remotely! }
@@ -54,11 +54,11 @@ module Bundler
         dependency = current_dependencies[current_spec.name]
 
         if strict
-          active_spec = definition.specs.detect {|spec| spec.name == current_spec.name && spec.match_platform(current_spec.platform) }
+          active_spec = definition.find_resolved_spec(current_spec)
         else
-          active_specs = definition.index[current_spec.name].select {|spec| spec.match_platform(current_spec.platform) }.sort_by(&:version)
+          active_specs = definition.find_indexed_specs(current_spec)
           if !current_spec.version.prerelease? && !options[:pre] && active_specs.size > 1
-            active_spec = active_specs.delete_if {|b| b.respond_to?(:version) && b.version.prerelease? }
+            active_specs.delete_if {|b| b.respond_to?(:version) && b.version.prerelease? }
           end
           active_spec = active_specs.last
         end
diff --git a/lib/bundler/definition.rb b/lib/bundler/definition.rb
index b9ac233f29..6e3b655706 100644
--- a/lib/bundler/definition.rb
+++ b/lib/bundler/definition.rb
@@ -531,6 +531,14 @@ module Bundler
       add_platform(generic(current_platform))
     end
 
+    def find_resolved_spec(current_spec)
+      specs.find_by_name_and_platform(current_spec.name, current_spec.platform)
+    end
+
+    def find_indexed_specs(current_spec)
+      index[current_spec.name].select {|spec| spec.match_platform(current_spec.platform) }.sort_by(&:version)
+    end
+
     attr_reader :sources
     private :sources
 
diff --git a/lib/bundler/spec_set.rb b/lib/bundler/spec_set.rb
index fe31b17f0e..d3ffa0d5ca 100644
--- a/lib/bundler/spec_set.rb
+++ b/lib/bundler/spec_set.rb
@@ -114,6 +114,10 @@ module Bundler
       SpecSet.new(arr)
     end
 
+    def find_by_name_and_platform(name, platform)
+      @specs.detect {|spec| spec.name == name && spec.match_platform(platform) }
+    end
+
   private
 
     def sorted
diff --git a/spec/bundler/definition_spec.rb b/spec/bundler/definition_spec.rb
index 470b4437f5..1bd6575d8b 100644
--- a/spec/bundler/definition_spec.rb
+++ b/spec/bundler/definition_spec.rb
@@ -225,26 +225,53 @@ describe Bundler::Definition do
           expect(locked.include?("shared_dep")).to be_truthy
         end
       end
+    end
+  end
+
+  describe "find_resolved_spec" do
+    it "with no platform set in SpecSet" do
+      ss = Bundler::SpecSet.new([build_stub_spec("a", "1.0"), build_stub_spec("b", "1.0")])
+      dfn = Bundler::Definition.new(nil, [], mock_source_list, true)
+      dfn.instance_variable_set("@specs", ss)
+      found = dfn.find_resolved_spec(build_spec("a", "0.9", "ruby").first)
+      expect(found.name).to eq "a"
+      expect(found.version.to_s).to eq "1.0"
+    end
+  end
 
-      def mock_source_list
-        Class.new do
-          def all_sources
-            []
-          end
+  describe "find_indexed_specs" do
+    it "with no platform set in indexed specs" do
+      index = Bundler::Index.new
+      %w(1.0.0 1.0.1 1.1.0).each {|v| index << build_stub_spec("foo", v) }
 
-          def path_sources
-            []
-          end
+      dfn = Bundler::Definition.new(nil, [], mock_source_list, true)
+      dfn.instance_variable_set("@index", index)
+      found = dfn.find_indexed_specs(build_spec("foo", "0.9", "ruby").first)
+      expect(found.length).to eq 3
+    end
+  end
 
-          def rubygems_remotes
-            []
-          end
+  def build_stub_spec(name, version)
+    Bundler::StubSpecification.new(name, version, nil, nil)
+  end
 
-          def replace_sources!(arg)
-            nil
-          end
-        end.new
+  def mock_source_list
+    Class.new do
+      def all_sources
+        []
       end
-    end
+
+      def path_sources
+        []
+      end
+
+      def rubygems_remotes
+        []
+      end
+
+      def replace_sources!(arg)
+        nil
+      end
+    end.new
   end
 end
diff --git a/spec/commands/outdated_spec.rb b/spec/commands/outdated_spec.rb
index b3d9a97b82..a3e46a4e7f 100644
--- a/spec/commands/outdated_spec.rb
+++ b/spec/commands/outdated_spec.rb
@@ -607,17 +607,17 @@ describe "bundle outdated" do
       it "shows all gems when patching and filtering to patch" do
         bundle "outdated --patch --filter-patch"
 
-        expect(out).to include("patch (newest")
-        expect(out).to include("minor (newest")
-        expect(out).to include("major (newest")
+        expect(out).to include("patch (newest 1.0.1")
+        expect(out).to include("minor (newest 1.0.1")
+        expect(out).to include("major (newest 1.0.1")
       end
 
       it "shows minor and major when updating to minor and filtering to patch and minor" do
         bundle "outdated --minor --filter-minor"
 
         expect(out).not_to include("patch (newest")
-        expect(out).to include("minor (newest")
-        expect(out).to include("major (newest")
+        expect(out).to include("minor (newest 1.1.0")
+        expect(out).to include("major (newest 1.1.0")
       end
 
       it "shows minor when updating to major and filtering to minor with parseable" do