From b31bd8b8531a7af294a056ce7b1e9c132fa9bcad Mon Sep 17 00:00:00 2001 From: Michael Siegfried Date: Thu, 23 Mar 2023 21:46:45 -0700 Subject: [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. https://github.com/rubygems/rubygems/commit/74c23a91b2 --- spec/bundler/bundler/gem_version_promoter_spec.rb | 235 ++++++++++------------ 1 file changed, 111 insertions(+), 124 deletions(-) (limited to 'spec/bundler') diff --git a/spec/bundler/bundler/gem_version_promoter_spec.rb b/spec/bundler/bundler/gem_version_promoter_spec.rb index b5c0f69795..8058412f32 100644 --- a/spec/bundler/bundler/gem_version_promoter_spec.rb +++ b/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 -- cgit v1.2.1