1st pass thorough specs on conservative resolver.

Ported over resolver specs from bundler-patch README (which don't match
the actual specs for no particular reason other than dev lolz) and
expanded on some of the cases after a few weird cases resolved ... a
little unexpectedly.

They're passing, but some discussion to be had on some of the cases
perhaps. See inline spec comments.

4 files changed, 89 insertions, 16 deletions

diff --git a/lib/bundler/resolver.rb b/lib/bundler/resolver.rb
index 8d3ea75760..5a7a441e7f 100644
--- a/lib/bundler/resolver.rb
+++ b/lib/bundler/resolver.rb
@@ -408,7 +408,7 @@ module Bundler
             (@strict ?
               filter_dep_specs(dep_specs, locked_spec) :
               sort_dep_specs(dep_specs, locked_spec)).tap do |specs|
-              if ENV['DEBUG_PATCH_RESOLVER'] # MODO: proper debug flag check and proper debug output
+              if ENV['DEBUG_PATCH_RESOLVER'] # MODO: proper debug flag name, check and proper debug output
                 STDERR.puts super_result
                 STDERR.puts "after search_for: #{debug_format_result(dep, specs).inspect}"
               end
@@ -419,7 +419,8 @@ module Bundler
       def debug_format_result(dep, res)
         a = [dep.to_s,
              res.map { |sg| [sg.version, sg.dependencies_for_activated_platforms.map { |dp| [dp.name, dp.requirement.to_s] }] }]
-        [a.first, a.last.map { |sg_data| [sg_data.first.version, sg_data.last.map { |aa| aa.join(' ') }] }, @level]
+        [a.first, a.last.map { |sg_data| [sg_data.first.version, sg_data.last.map { |aa| aa.join(' ') }] },
+         @level, @strict ? :strict : :not_strict, @minimal ? :minimal : :not_minimal]
       end
 
       def filter_dep_specs(specs, locked_spec)
@@ -435,7 +436,7 @@ module Bundler
             must_match = @level == :minor ? [0] : [0, 1]
 
             matches = must_match.map { |idx| gsv.segments[idx] == lsv.segments[idx] }
-            (matches.uniq == [true]) ? gsv.send(:>=, lsv) : false
+            (matches.uniq == [true]) ? (gsv >= lsv) : false
           else
             true
           end
diff --git a/spec/commands/update_spec.rb b/spec/commands/update_spec.rb
index 303f58baf5..ac7c3b4f07 100644
--- a/spec/commands/update_spec.rb
+++ b/spec/commands/update_spec.rb
@@ -443,7 +443,8 @@ describe "bundle update conservative" do
         gem 'foo'
       G
 
-      bundle "update --patch foo", {:env => {'DEBUG_PATCH_RESOLVER' => true}}
+      # bundle "update --patch foo", {:env => {'DEBUG_PATCH_RESOLVER' => true}}
+      bundle "update --patch foo"
 
       should_be_installed "foo 1.0.1"
     end
diff --git a/spec/resolver/basic_spec.rb b/spec/resolver/basic_spec.rb
index c35b0ea81a..a298564559 100644
--- a/spec/resolver/basic_spec.rb
+++ b/spec/resolver/basic_spec.rb
@@ -104,15 +104,78 @@ describe "Resolving" do
   end
 
   context 'conservative' do
-    context 'patch' do
-      it 'resolves single gem with no dependencies' do
-        @index = build_index do
-          gem "foo", %w(1.0.0 1.0.1 1.0.2 1.1.0 2.0.0)
-        end
-        dep 'foo'
-
-        should_consv_resolve_and_include :patch, locked('foo', '1.0.0'), %w(foo-1.0.2)
+    before :each do
+      @index = build_index do
+        gem('foo', '1.3.7') { dep 'bar', '~> 2.0' }
+        gem('foo', '1.3.8') { dep 'bar', '~> 2.0' }
+        gem('foo', '1.4.3') { dep 'bar', '~> 2.0' }
+        gem('foo', '1.4.4') { dep 'bar', '~> 2.0' }
+        gem('foo', '1.4.5') { dep 'bar', '~> 2.1' }
+        gem('foo', '1.5.0') { dep 'bar', '~> 2.1' }
+        gem('foo', '1.5.1') { dep 'bar', '~> 3.0' }
+        gem 'bar', %w(2.0.3 2.0.4 2.0.5 2.1.0 2.1.1 3.0.0)
       end
+      dep 'foo'
+
+      @locked = locked(%w(foo 1.4.3), %w(bar 2.0.3))
+    end
+
+    it 'resolves all gems to latest patch' do
+      # strict is not set, so bar goes up a minor version due to dependency from foo 1.4.5
+      should_consv_resolve_and_include :patch, [], %w(foo-1.4.5 bar-2.1.1)
+    end
+
+    it 'resolves all gems to latest patch strict' do
+      # strict is set, so foo can only go up to 1.4.4 to avoid bar going up a minor version, and bar can go up to 2.0.5
+      should_consv_resolve_and_include [:patch, :strict], [], %w(foo-1.4.4 bar-2.0.5)
+    end
+
+    it 'resolves all gems to latest patch minimal' do
+      # minimal is set, so foo goes up the next available to 1.4.4 and bar goes up to next available 2.0.4
+      should_consv_resolve_and_include [:patch, :minimal], [], %w(foo-1.4.4 bar-2.0.4)
+    end
+
+    it 'resolves foo only to latest patch - same dependency case' do
+      @locked = locked(%w(foo 1.3.7), %w(bar 2.0.3))
+      # bar is locked, and the lock holds here because the dependency on bar doesn't change on the matching foo version.
+      should_consv_resolve_and_include :patch, ['foo'], %w(foo-1.3.8 bar-2.0.3)
+    end
+
+    it 'resolves foo only to latest patch - changing dependency case' do
+      # bar is locked, but locks don't apply to _changing_ dependencies and since the dependency of the
+      # selected foo gem changes, the latest matching of bar-2.1.1
+      # (this could be considered a bug, but possibly hard to solve for)
+      should_consv_resolve_and_include :patch, ['foo'], %w(foo-1.4.5 bar-2.1.1)
+    end
+
+    it 'resolves foo only to latest patch strict' do
+      # adding strict helps solve the possibly unexpected behavior of bar changing in the prior test case,
+      # because no versions will be returned for bar ~> 2.1, so the engine falls back to ~> 2.0 (turn on
+      # debugging to see this happen).
+      should_consv_resolve_and_include [:patch, :strict], ['foo'], %w(foo-1.4.4 bar-2.0.3)
+    end
+
+    it 'resolves bar only to latest patch' do
+      # bar is locked, so foo can only go up to 1.4.4
+      should_consv_resolve_and_include :patch, ['bar'], %w(foo-1.4.3 bar-2.0.5)
+    end
+
+    it 'resolves all gems to latest minor' do
+      # strict is not set, so bar goes up a major version due to dependency from foo 1.4.5
+      should_consv_resolve_and_include :minor, [], %w(foo-1.5.1 bar-3.0.0)
+    end
+
+    it 'resolves all gems to latest minor strict' do
+      # strict is set, so foo can only go up to 1.5.0 to avoid bar going up a major version
+      should_consv_resolve_and_include [:minor, :strict], [], %w(foo-1.5.0 bar-2.1.1)
+    end
+
+    it 'resolves all gems to latest minor minimal' do
+      # minimal is set, and it takes precedence over minor. not sure what is the PoLS in this case. Not sure
+      # if minimal is a great option in the first place. It exists to help a case where there are many, many
+      # versions and I'd rather go from 1.0.2 to 1.0.3 instead of 1.0.45. But, we could consider killing the
+      # minimal option altogether. If that's what you need, use the Gemfile dependency.
+      should_consv_resolve_and_include [:minor, :minimal], [], %w(foo-1.4.4 bar-2.0.4)
     end
   end
 end
diff --git a/spec/support/indexes.rb b/spec/support/indexes.rb
index cfd7f4b5fe..b048cd8fb3 100644
--- a/spec/support/indexes.rb
+++ b/spec/support/indexes.rb
@@ -49,12 +49,20 @@ module Spec
       build_spec(*args, &blk).first
     end
 
-    def locked(name, versions)
-      Bundler::SpecSet.new(build_spec(name, Array(versions)))
+    def locked(*args)
+      Bundler::SpecSet.new(args.map do |name, version|
+        gem(name, version)
+      end)
     end
 
-    def should_consv_resolve_and_include(level, locked, specs)
-      searcher = Bundler::Resolver::UpdateOptions.new(locked).tap { |s| s.level = level }
+    def should_consv_resolve_and_include(opts, unlock, specs)
+      # empty unlock means unlock all
+      opts = Array(opts)
+      searcher = Bundler::Resolver::UpdateOptions.new(@locked, unlock).tap do |s|
+        s.level = opts.first
+        s.strict = opts.include?(:strict)
+        s.minimal = opts.include?(:minimal)
+      end
       should_resolve_and_include specs, [{}, [], nil, searcher]
     end