Add defaults to go_module_versions factory

Improve go module factories to it doesn't
break when moving it to core

ee/app/models/packages/sem_ver.rb

 # frozen_string_literal: true
 
-class Packages::SemVer
-  attr_accessor :major, :minor, :patch, :prerelease, :build
-
-  def initialize(major = 0, minor = 0, patch = 0, prerelease = nil, build = nil, prefixed: false)
-    @major = major
-    @minor = minor
-    @patch = patch
-    @prerelease = prerelease
-    @build = build
-    @prefixed = prefixed
-  end
+module Packages
+  class SemVer
+    attr_accessor :major, :minor, :patch, :prerelease, :build
 
-  def prefixed?
-    @prefixed
-  end
+    def initialize(major = 0, minor = 0, patch = 0, prerelease = nil, build = nil, prefixed: false)
+      @major = major
+      @minor = minor
+      @patch = patch
+      @prerelease = prerelease
+      @build = build
+      @prefixed = prefixed
+    end
 
-  def ==(other)
-    self.class == other.class &&
-    self.major == other.major &&
-    self.minor == other.minor &&
-    self.patch == other.patch &&
-    self.prerelease == other.prerelease &&
-    self.build == other.build
-  end
+    def prefixed?
+      @prefixed
+    end
 
-  def to_s
-    s = "#{prefixed? ? 'v' : ''}#{major || 0}.#{minor || 0}.#{patch || 0}"
-    s += "-#{prerelease}" if prerelease
-    s += "+#{build}" if build
+    def with(**args)
+      self.class.new(
+        args.fetch(:major, major),
+        args.fetch(:minor, minor),
+        args.fetch(:patch, patch),
+        args.fetch(:prerelease, args.fetch(:pre, prerelease)),
+        args.fetch(:build, build),
+        prefixed: args.fetch(:prefixed, prefixed?)
+      )
+    end
 
-    s
-  end
+    def ==(other)
+      self.class == other.class &&
+        self.major == other.major &&
+        self.minor == other.minor &&
+        self.patch == other.patch &&
+        self.prerelease == other.prerelease &&
+        self.build == other.build
+    end
 
-  def self.match(str, prefixed: false)
-    return unless str&.start_with?('v') == prefixed
+    # rubocop: disable Metrics/CyclomaticComplexity,Metrics/PerceivedComplexity,Metrics/AbcSize
+    def <=>(other)
+      a, b = self, other
 
-    str = str[1..] if prefixed
+      raise ArgumentError.new('Not the same type') unless a.class == b.class
 
-    Gitlab::Regex.semver_regex.match(str)
-  end
+      return 0 if a == b
 
-  def self.match?(str, prefixed: false)
-    !match(str, prefixed: prefixed).nil?
-  end
+      return -1 if a.major < b.major
+      return +1 if a.major > b.major
+      return -1 if a.minor < b.minor
+      return +1 if a.minor > b.minor
+      return -1 if a.patch < b.patch
+      return +1 if a.patch > b.patch
+
+      if a.prerelease == b.prerelease
+        # "Build metadata MUST be ignored when determining version precedence."
+        # But that would lead to unstable ordering, so check it anyways.
+        return 0 if a.build == b.build
+        return -1 if !a.build.nil? &&  b.build.nil?
+        return +1 if  a.build.nil? && !b.build.nil?
+        return -1 if a.build < b.build
+
+        return +1 ## a.build > b.build
+      end
+
+      return -1 if !a.prerelease.nil? &&  b.prerelease.nil?
+      return +1 if  a.prerelease.nil? && !b.prerelease.nil?
+
+      # "Precedence for [...] patch versions MUST be determined by comparing each
+      # dot separated identifier from left to right."
+      a_parts = a.prerelease&.split('.') || []
+      b_parts = b.prerelease&.split('.') || []
+
+      (0...[a_parts.length, b_parts.length].min).each do |i|
+        a_part, b_part = a_parts[i], b_parts[i]
+        next if a_part == b_part
+
+        a_num = a_part.to_i if /^\d+$/.match?(a_part)
+        b_num = b_part.to_i if /^\d+$/.match?(b_part)
+
+        unless a_num.nil? || b_num.nil?
+          return -1 if a_num < b_num
+          return +1 if a_num > b_num
+
+          # '0' and '000' have the same precedence, but stable ordering is good.
+        end
+
+        # "Numeric identifiers always have lower precedence than non-numeric identifiers."
+        return -1 if !a_num.nil? &&  b_num.nil?
+        return +1 if  a_num.nil? && !b_num.nil?
+
+        return -1 if a_part < b_part
+        return +1 if a_part > b_part
+      end
+
+      return -1 if a_parts.length < b_parts.length
+      return +1 if a_parts.length > b_parts.length
+
+      return 0
+    end
+    # rubocop: enable Metrics/CyclomaticComplexity,Metrics/PerceivedComplexity,Metrics/AbcSize
+
+    def to_s
+      s = "#{prefixed? ? 'v' : ''}#{major || 0}.#{minor || 0}.#{patch || 0}"
+      s += "-#{prerelease}" if prerelease
+      s += "+#{build}" if build
+
+      s
+    end
+
+    def self.match(str, prefixed: false)
+      return unless str&.start_with?('v') == prefixed
+
+      str = str[1..] if prefixed
+
+      Gitlab::Regex.semver_regex.match(str)
+    end
+
+    def self.match?(str, prefixed: false)
+      !match(str, prefixed: prefixed).nil?
+    end
 
-  def self.parse(str, prefixed: false)
-    m = match str, prefixed: prefixed
-    return unless m
+    def self.parse(str, prefixed: false)
+      m = match str, prefixed: prefixed
+      return unless m
 
-    new(m[1].to_i, m[2].to_i, m[3].to_i, m[4], m[5], prefixed: prefixed)
+      new(m[1].to_i, m[2].to_i, m[3].to_i, m[4], m[5], prefixed: prefixed)
+    end
   end
 end

ee/spec/factories/go_module_commits.rb

@@ -5,8 +5,23 @@ FactoryBot.define do
     skip_create
 
     transient do
-      project { raise ArgumentError.new("project is required") }
-      service { raise ArgumentError.new("this factory cannot be used without specifying a trait") }
+      files { { 'foo.txt' => 'content' } }
+      message { 'Message' }
+      project { create(:project, :repository) }
+
+      service do
+        Files::MultiService.new(
+          project,
+          project.owner,
+          commit_message: message,
+          start_branch: project.repository.root_ref || 'master',
+          branch_name: project.repository.root_ref || 'master',
+          actions: files.map do |path, content|
+            { action: :create, file_path: path, content: content }
+          end
+        )
+      end
+
       tag { nil }
       tag_message { nil }
 
@@ -27,47 +42,18 @@ FactoryBot.define do
       end
     end
 
-    initialize_with do
-      commit
-    end
-
     trait :files do
       transient do
         files { raise ArgumentError.new("files is required") }
         message { 'Add files' }
       end
-
-      service do
-        Files::MultiService.new(
-          project,
-          project.owner,
-          commit_message: message,
-          start_branch: project.repository.root_ref || 'master',
-          branch_name: project.repository.root_ref || 'master',
-          actions: files.map do |path, content|
-            { action: :create, file_path: path, content: content }
-          end
-        )
-      end
     end
 
     trait :package do
       transient do
         path { raise ArgumentError.new("path is required") }
         message { 'Add package' }
-      end
-
-      service do
-        Files::MultiService.new(
-          project,
-          project.owner,
-          commit_message: message,
-          start_branch: project.repository.root_ref || 'master',
-          branch_name: project.repository.root_ref || 'master',
-          actions: [
-            { action: :create, file_path: path + '/b.go', content: "package b\nfunc Bye() { println(\"Goodbye world!\") }\n" }
-          ]
-        )
+        files { { "#{path}/b.go" => "package b\nfunc Bye() { println(\"Goodbye world!\") }\n" } }
       end
     end
 
@@ -75,39 +61,22 @@ FactoryBot.define do
       transient do
         name { nil }
         message { 'Add module' }
+        host_prefix { "#{::Gitlab.config.gitlab.host}/#{project.path_with_namespace}" }
 
-        url do
-          v = "#{::Gitlab.config.gitlab.host}/#{project.path_with_namespace}"
-
-          if name
-            v + '/' + name
-          else
-            v
-          end
-        end
+        url { name ? "#{host_prefix}/#{name}" : host_prefix }
+        path { name.to_s + '/' }
 
-        path do
-          if name
-            name + '/'
-          else
-            ''
-          end
+        files do
+          {
+            "#{path}go.mod" => "module #{url}\n",
+            "#{path}a.go" => "package a\nfunc Hi() { println(\"Hello world!\") }\n"
+          }
         end
       end
+    end
 
-      service do
-        Files::MultiService.new(
-          project,
-          project.owner,
-          commit_message: message,
-          start_branch: project.repository.root_ref || 'master',
-          branch_name: project.repository.root_ref || 'master',
-          actions: [
-            { action: :create, file_path: path + 'go.mod', content: "module #{url}\n" },
-            { action: :create, file_path: path + 'a.go', content: "package a\nfunc Hi() { println(\"Hello world!\") }\n" }
-          ]
-        )
-      end
+    initialize_with do
+      commit
     end
   end
 end

ee/spec/factories/go_module_versions.rb

@@ -15,7 +15,7 @@ FactoryBot.define do
 
     mod { create :go_module }
     type { :commit }
-    commit { raise ArgumentError.new("commit is required") }
+    commit { mod.project.repository.head_commit }
     name { nil }
     semver { nil }
     ref { nil }
@@ -23,16 +23,49 @@ FactoryBot.define do
     params { OpenStruct.new(mod: mod, type: type, commit: commit, name: name, semver: semver, ref: ref) }
 
     trait :tagged do
-      name { raise ArgumentError.new("name is required") }
       ref { mod.project.repository.find_tag(name) }
       commit { ref.dereferenced_target }
+      name do
+        # This provides a sane default value, but in reality the caller should
+        # specify `name:`
+
+        # Find 'latest' semver tag (does not actually use semver precedence rules)
+        mod.project.repository.tags
+          .filter { |t| Packages::SemVer.match?(t.name, prefixed: true) }
+          .map    { |t| Packages::SemVer.parse(t.name, prefixed: true) }
+          .max    { |a, b| "#{a}" <=> "#{b}" }
+          .to_s
+      end
 
       params { OpenStruct.new(mod: mod, type: :ref, commit: commit, semver: name, ref: ref) }
     end
 
     trait :pseudo do
       transient do
-        prefix { raise ArgumentError.new("prefix is required") }
+        prefix do
+          # This provides a sane default value, but in reality the caller should
+          # specify `prefix:`
+
+          # This does not take into account that `commit` may be before the
+          # latest tag.
+
+          # Find 'latest' semver tag (does not actually use semver precedence rules)
+          v = mod.project.repository.tags
+            .filter { |t| Packages::SemVer.match?(t.name, prefixed: true) }
+            .map    { |t| Packages::SemVer.parse(t.name, prefixed: true) }
+            .max    { |a, b| "#{a}" <=> "#{b}" }
+
+          # Default if no semver tags exist
+          next 'v0.0.0' unless v
+
+          # Valid pseudo-versions are:
+          #   vX.0.0-yyyymmddhhmmss-sha1337beef0, when no earlier tagged commit exists for X
+          #   vX.Y.Z-pre.0.yyyymmddhhmmss-sha1337beef0, when most recent prior tag is vX.Y.Z-pre
+          #   vX.Y.(Z+1)-0.yyyymmddhhmmss-sha1337beef0, when most recent prior tag is vX.Y.Z
+
+          v = v.with(patch: v.patch + 1) unless v.prerelease
+          "#{v}.0"
+        end
       end
 
       type { :pseudo }

ee/spec/factories/go_modules.rb

@@ -5,7 +5,8 @@ FactoryBot.define do
     initialize_with { new(attributes[:project], attributes[:name], attributes[:path]) }
     skip_create
 
-    project
+    project { create :project, :repository }
+
     path { '' }
     name { "#{Settings.build_gitlab_go_url}/#{project.full_path}#{path.empty? ? '' : '/'}#{path}" }
   end

ee/spec/factories/sem_vers.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :semver, class: 'Packages::SemVer' do
+    initialize_with { new(attributes[:major], attributes[:minor], attributes[:patch], attributes[:prerelease], attributes[:build], prefixed: attributes[:prefixed]) }
+    skip_create
+
+    major { 1 }
+    minor { 0 }
+    patch { 0 }
+    prerelease { nil }
+    build { nil }
+    prefixed { false }
+  end
+end

ee/spec/models/packages/sem_ver_spec.rb

 # frozen_string_literal: true
 
-require 'spec_helper'
+require 'fast_spec_helper'
 
 RSpec.describe Packages::SemVer, type: :model do
   shared_examples '#parse with a valid semver' do |str, major, minor, patch, prerelease, build|
+    subject(:semver) { described_class.new(major, minor, patch, prerelease, build) }
+
     context "with #{str}" do
-      it "returns #{described_class.new(major, minor, patch, prerelease, build, prefixed: true)} with prefix" do
-        expected = described_class.new(major, minor, patch, prerelease, build, prefixed: true)
-        expect(described_class.parse('v' + str, prefixed: true)).to eq(expected)
+      subject(:expected) { semver.with(prefixed: prefixed) }
+
+      context 'prefixed' do
+        let(:prefixed) { true }
+
+        specify do
+          expect(described_class.parse('v' + str, prefixed: true)).to eq(expected)
+        end
       end
 
-      it "returns #{described_class.new(major, minor, patch, prerelease, build)} without prefix" do
-        expected = described_class.new(major, minor, patch, prerelease, build)
-        expect(described_class.parse(str)).to eq(expected)
+      context 'without prefix' do
+        let(:prefixed) { false }
+
+        specify do
+          expect(described_class.parse(str)).to eq(expected)
+        end
       end
     end
   end
@@ -29,9 +39,17 @@ RSpec.describe Packages::SemVer, type: :model do
     end
   end
 
+  shared_examples 'sorted' do
+    it 'orders correctly' do
+      (1..10).each do |_|
+        expect(expected_list.shuffle.sort.map(&:to_s)).to eq(expected_list.map(&:to_s))
+      end
+    end
+  end
+
   describe '#parse' do
     it_behaves_like '#parse with a valid semver', '1.0.0', 1, 0, 0, nil, nil
-    it_behaves_like '#parse with a valid semver', '1.0.0-pre', 1, 0, 0, 'pre', nil
+    it_behaves_like '#parse with a valid semver', '1.0.0-pre', 1, 0, 0, 'pre'
     it_behaves_like '#parse with a valid semver', '1.0.0+build', 1, 0, 0, nil, 'build'
     it_behaves_like '#parse with a valid semver', '1.0.0-pre+build', 1, 0, 0, 'pre', 'build'
     it_behaves_like '#parse with an invalid semver', '01.0.0'
@@ -39,4 +57,17 @@ RSpec.describe Packages::SemVer, type: :model do
     it_behaves_like '#parse with an invalid semver', '0.0.01'
     it_behaves_like '#parse with an invalid semver', '1.0.0asdf'
   end
+
+  describe '#<=>' do
+    let(:v1) { described_class.new(1, 0, 0) }
+    let(:v2) { described_class.new(2, 0, 0) }
+
+    it_behaves_like 'sorted' do
+      let(:expected_list) { [v1.with(pre: 'beta'), v1, v1.with(minor: 1), v2.with(pre: 'alpha'), v2, v2.with(patch: 1), v2.with(minor: 1)] }
+    end
+
+    it_behaves_like 'sorted' do
+      let(:expected_list) { [v1.with(pre: 'alpha'), v1.with(pre: 'alpha.1'), v1.with(pre: 'alpha.beta'), v1.with(pre: 'beta'), v1.with(pre: 'beta.2'), v1.with(pre: 'beta.11'), v1.with(pre: 'rc.1'), v1] }
+    end
+  end
 end