[rubygems/rubygems] Rewrite GemVersionPromoter specs

Add tests for pre, move more of the setup into a helper method, and
restructure tests.

There seem to be five considerations for these tests (level, pre, strict,
locked, and whether the current version is a prerelease version, though
the last one overlaps with pre and didn't seem to behave how I expected
under test). Rather than write out the 16 (/32 if the last consideration
is real) combinations, I wrote most with independent tests for each
value. The existing combined tests were maintained (level vs strict)
because these seem the most interrelated.

rubygems/rubygems@74c23a91b2

spec/bundler/bundler/gem_version_promoter_spec.rb

@@ -1,175 +1,162 @@
 # frozen_string_literal: true
 
 RSpec.describe Bundler::GemVersionPromoter do
-  context "conservative resolver" do
-    def versions(result)
-      result.flatten.map(&:version).map(&:to_s)
-    end
-
+  let(:gvp) { described_class.new }
+
+  # Rightmost (highest array index) in result is most preferred.
+  # Leftmost (lowest array index) in result is least preferred.
+  # `build_candidates` has all versions of gem in index.
+  # `build_spec` is the version currently in the .lock file.
+  #
+  # In default (not strict) mode, all versions in the index will
+  # be returned, allowing Bundler the best chance to resolve all
+  # dependencies, but sometimes resulting in upgrades that some
+  # would not consider conservative.
+
+  describe "#sort_versions" do
     def build_candidates(versions)
       versions.map do |v|
         Bundler::Resolver::Candidate.new(v)
       end
     end
 
-    def build_spec_set(name, v)
-      Bundler::SpecSet.new(build_spec(name, v))
+    def build_package(name, version, locked = [])
+      Bundler::Resolver::Package.new(name, [], :locked_specs => Bundler::SpecSet.new(build_spec(name, version)), :unlock => locked)
     end
 
-    def build_package(name, platforms, locked_specs, unlock)
-      Bundler::Resolver::Package.new(name, platforms, :locked_specs => locked_specs, :unlock => unlock)
+    def sorted_versions(candidates:, current:, name: "foo", locked: [])
+      gvp.sort_versions(
+        build_package(name, current, locked),
+        build_candidates(candidates)
+      ).flatten.map(&:version).map(&:to_s)
     end
 
-    # Rightmost (highest array index) in result is most preferred.
-    # Leftmost (lowest array index) in result is least preferred.
-    # `build_candidates` has all versions of gem in index.
-    # `build_spec` is the version currently in the .lock file.
-    #
-    # In default (not strict) mode, all versions in the index will
-    # be returned, allowing Bundler the best chance to resolve all
-    # dependencies, but sometimes resulting in upgrades that some
-    # would not consider conservative.
-    context "filter specs (strict) level patch" do
-      let(:gvp) do
-        Bundler::GemVersionPromoter.new.tap do |gvp|
-          gvp.level = :patch
-          gvp.strict = true
-        end
-      end
+    it "numerically sorts versions" do
+      versions = sorted_versions(:candidates => %w[1.7.7 1.7.8 1.7.9 1.7.15 1.8.0], :current => "1.7.8")
+      expect(versions).to eq %w[1.7.7 1.7.8 1.7.9 1.7.15 1.8.0]
+    end
 
-      it "when keeping build_spec, keep current, next release" do
-        res = gvp.sort_versions(
-          build_package("foo", [], build_spec_set("foo", "1.7.8"), []),
-          build_candidates(%w[1.7.8 1.7.9 1.8.0])
-        )
-        expect(versions(res)).to eq %w[1.7.8 1.7.9]
+    context "with no options" do
+      it "defaults to level=:major, strict=false, pre=false" do
+        versions = sorted_versions(:candidates => %w[0.2.0 0.3.0 0.3.1 0.9.0 1.0.0 2.0.1 2.1.0], :current => "0.3.0")
+        expect(versions).to eq %w[0.2.0 0.3.0 0.3.1 0.9.0 1.0.0 2.0.1 2.1.0]
       end
+    end
 
-      it "when unlocking prefer next release first" do
-        res = gvp.sort_versions(
-          build_package("foo", [], build_spec_set("foo", "1.7.8"), []),
-          build_candidates(%w[1.7.8 1.7.9 1.8.0])
-        )
-        expect(versions(res)).to eq %w[1.7.8 1.7.9]
-      end
+    context "when strict" do
+      before { gvp.strict = true }
 
-      it "when unlocking keep current when already at latest release" do
-        res = gvp.sort_versions(
-          build_package("foo", [], build_spec_set("foo", "1.7.9"), []),
-          build_candidates(%w[1.7.9 1.8.0 2.0.0])
-        )
-        expect(versions(res)).to eq %w[1.7.9]
-      end
-    end
+      context "when level is major" do
+        before { gvp.level = :major }
 
-    context "filter specs (strict) level minor" do
-      let(:gvp) do
-        Bundler::GemVersionPromoter.new.tap do |gvp|
-          gvp.level = :minor
-          gvp.strict = true
+        it "keeps downgrades" do
+          versions = sorted_versions(:candidates => %w[0.2.0 0.3.0 0.3.1 0.9.0 1.0.0 2.0.1 2.1.0], :current => "0.3.0")
+          expect(versions).to eq %w[0.2.0 0.3.0 0.3.1 0.9.0 1.0.0 2.0.1 2.1.0]
         end
       end
 
-      it "when unlocking favor next releases, remove minor and major increases" do
-        res = gvp.sort_versions(
-          build_package("foo", [], build_spec_set("foo", "0.2.0"), []),
-          build_candidates(%w[0.2.0 0.3.0 0.3.1 0.9.0 1.0.0 2.0.0 2.0.1])
-        )
-        expect(versions(res)).to eq %w[0.2.0 0.3.0 0.3.1 0.9.0]
+      context "when level is minor" do
+        before { gvp.level = :minor }
+
+        it "removes downgrades and major upgrades" do
+          versions = sorted_versions(:candidates => %w[0.2.0 0.3.0 0.3.1 0.9.0 1.0.0 2.0.1 2.1.0], :current => "0.3.0")
+          expect(versions).to eq %w[0.3.0 0.3.1 0.9.0]
+        end
       end
 
-      it "when keep locked, keep current, then favor next release, remove minor and major increases" do
-        res = gvp.sort_versions(
-          build_package("foo", [], build_spec_set("foo", "0.2.0"), ["bar"]),
-          build_candidates(%w[0.2.0 0.3.0 0.3.1 0.9.0 1.0.0 2.0.0 2.0.1])
-        )
-        expect(versions(res)).to eq %w[0.3.0 0.3.1 0.9.0 0.2.0]
+      context "when level is patch" do
+        before { gvp.level = :patch }
+
+        it "removes downgrades and major and minor upgrades" do
+          versions = sorted_versions(:candidates => %w[0.2.0 0.3.0 0.3.1 0.9.0 1.0.0 2.0.1 2.1.0], :current => "0.3.0")
+          expect(versions).to eq %w[0.3.0 0.3.1]
+        end
       end
     end
 
-    context "sort specs (not strict) level patch" do
-      let(:gvp) do
-        Bundler::GemVersionPromoter.new.tap do |gvp|
-          gvp.level = :patch
-          gvp.strict = false
+    context "when not strict" do
+      before { gvp.strict = false }
+
+      context "when level is major" do
+        before { gvp.level = :major }
+
+        it "orders by version" do
+          versions = sorted_versions(:candidates => %w[0.2.0 0.3.0 0.3.1 0.9.0 1.0.0 2.0.1 2.1.0], :current => "0.3.0")
+          expect(versions).to eq %w[0.2.0 0.3.0 0.3.1 0.9.0 1.0.0 2.0.1 2.1.0]
         end
       end
 
-      it "when not unlocking, same order but make sure build_spec version is most preferred to stay put" do
-        res = gvp.sort_versions(
-          build_package("foo", [], build_spec_set("foo", "1.7.7"), ["bar"]),
-          build_candidates(%w[1.5.4 1.6.5 1.7.6 1.7.7 1.7.8 1.7.9 1.8.0 1.8.1 2.0.0 2.0.1])
-        )
-        expect(versions(res)).to eq %w[1.5.4 1.6.5 1.7.6 2.0.0 2.0.1 1.8.0 1.8.1 1.7.8 1.7.9 1.7.7]
-      end
+      context "when level is minor" do
+        before { gvp.level = :minor }
 
-      it "when unlocking favor next release, then current over minor increase" do
-        res = gvp.sort_versions(
-          build_package("foo", [], build_spec_set("foo", "1.7.8"), []),
-          build_candidates(%w[1.7.7 1.7.8 1.7.9 1.8.0])
-        )
-        expect(versions(res)).to eq %w[1.7.7 1.8.0 1.7.8 1.7.9]
+        it "favors downgrades, then upgrades by major descending, minor ascending, patch ascending" do
+          versions = sorted_versions(:candidates => %w[0.2.0 0.3.0 0.3.1 0.9.0 1.0.0 2.0.1 2.1.0], :current => "0.3.0")
+          expect(versions).to eq %w[0.2.0 2.0.1 2.1.0 1.0.0 0.3.0 0.3.1 0.9.0]
+        end
       end
 
-      it "when unlocking do proper integer comparison, not string" do
-        res = gvp.sort_versions(
-          build_package("foo", [], build_spec_set("foo", "1.7.8"), []),
-          build_candidates(%w[1.7.7 1.7.8 1.7.9 1.7.15 1.8.0])
-        )
-        expect(versions(res)).to eq %w[1.7.7 1.8.0 1.7.8 1.7.9 1.7.15]
-      end
+      context "when level is patch" do
+        before { gvp.level = :patch }
 
-      it "leave current when unlocking but already at latest release" do
-        res = gvp.sort_versions(
-          build_package("foo", [], build_spec_set("foo", "1.7.9"), []),
-          build_candidates(%w[1.7.9 1.8.0 2.0.0])
-        )
-        expect(versions(res)).to eq %w[2.0.0 1.8.0 1.7.9]
+        it "favors downgrades, then upgrades by major descending, minor descending, patch ascending" do
+          versions = sorted_versions(:candidates => %w[0.2.0 0.3.0 0.3.1 0.9.0 1.0.0 2.0.1 2.1.0], :current => "0.3.0")
+          expect(versions).to eq %w[0.2.0 2.1.0 2.0.1 1.0.0 0.9.0 0.3.0 0.3.1]
+        end
       end
     end
 
-    context "sort specs (not strict) level minor" do
-      let(:gvp) do
-        Bundler::GemVersionPromoter.new.tap do |gvp|
-          gvp.level = :minor
-          gvp.strict = false
-        end
+    context "when pre" do
+      before { gvp.pre = true }
+
+      it "sorts regardless of prerelease status" do
+        versions = sorted_versions(:candidates => %w[1.7.7.pre 1.8.0 1.8.1.pre 1.8.1 2.0.0.pre 2.0.0], :current => "1.8.0")
+        expect(versions).to eq %w[1.7.7.pre 1.8.0 1.8.1.pre 1.8.1 2.0.0.pre 2.0.0]
       end
+    end
+
+    context "when not pre" do
+      before { gvp.pre = false }
 
-      it "when unlocking favor next release, then minor increase over current" do
-        res = gvp.sort_versions(
-          build_package("foo", [], build_spec_set("foo", "0.2.0"), []),
-          build_candidates(%w[0.2.0 0.3.0 0.3.1 0.9.0 1.0.0 2.0.0 2.0.1])
-        )
-        expect(versions(res)).to eq %w[2.0.0 2.0.1 1.0.0 0.2.0 0.3.0 0.3.1 0.9.0]
+      it "deprioritizes prerelease gems" do
+        versions = sorted_versions(:candidates => %w[1.7.7.pre 1.8.0 1.8.1.pre 1.8.1 2.0.0.pre 2.0.0], :current => "1.8.0")
+        expect(versions).to eq %w[1.7.7.pre 1.8.1.pre 2.0.0.pre 1.8.0 1.8.1 2.0.0]
       end
     end
 
-    context "level error handling" do
-      subject { Bundler::GemVersionPromoter.new }
+    context "when locking and not major" do
+      before { gvp.level = :minor }
 
-      it "should raise if not major, minor or patch is passed" do
-        expect { subject.level = :minjor }.to raise_error ArgumentError
+      it "keeps the current version last" do
+        versions = sorted_versions(:candidates => %w[0.2.0 0.3.0 0.3.1 0.9.0 1.0.0 2.1.0 2.0.1], :current => "0.3.0", :locked => ["bar"])
+        expect(versions.last).to eq("0.3.0")
       end
+    end
+  end
 
-      it "should raise if invalid classes passed" do
-        [123, nil].each do |value|
-          expect { subject.level = value }.to raise_error ArgumentError
-        end
+  describe "#level=" do
+    subject { described_class.new }
+
+    it "should raise if not major, minor or patch is passed" do
+      expect { subject.level = :minjor }.to raise_error ArgumentError
+    end
+
+    it "should raise if invalid classes passed" do
+      [123, nil].each do |value|
+        expect { subject.level = value }.to raise_error ArgumentError
       end
+    end
 
-      it "should accept major, minor patch symbols" do
-        [:major, :minor, :patch].each do |value|
-          subject.level = value
-          expect(subject.level).to eq value
-        end
+    it "should accept major, minor patch symbols" do
+      [:major, :minor, :patch].each do |value|
+        subject.level = value
+        expect(subject.level).to eq value
       end
+    end
 
-      it "should accept major, minor patch strings" do
-        %w[major minor patch].each do |value|
-          subject.level = value
-          expect(subject.level).to eq value.to_sym
-        end
+    it "should accept major, minor patch strings" do
+      %w[major minor patch].each do |value|
+        subject.level = value
+        expect(subject.level).to eq value.to_sym
       end
     end
   end