From 7cee6139c07db7ba4fd6febf74c24dbfaee8e59d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Wed, 17 Jul 2019 15:33:52 +0200 Subject: Find build by sha from ref Adds ability to find builds by sha when only specifying a ref. --- app/controllers/projects/artifacts_controller.rb | 5 ++++- app/models/ci/pipeline.rb | 7 ++++++- app/models/project.rb | 13 ++++++++++++- .../projects/artifacts/user_downloads_artifacts_spec.rb | 4 ++-- 4 files changed, 24 insertions(+), 5 deletions(-) diff --git a/app/controllers/projects/artifacts_controller.rb b/app/controllers/projects/artifacts_controller.rb index 2ef18d900f2..da8a371acaa 100644 --- a/app/controllers/projects/artifacts_controller.rb +++ b/app/controllers/projects/artifacts_controller.rb @@ -92,7 +92,10 @@ class Projects::ArtifactsController < Projects::ApplicationController def build_from_ref return unless @ref_name - project.latest_successful_build_for(params[:job], @ref_name) + commit = project.commit(@ref_name) + return unless commit + + project.latest_successful_build_for_sha(params[:job], commit.id) end def artifacts_file diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 2262282e647..f70ff7e3192 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -230,9 +230,10 @@ module Ci # ref - The name (or names) of the branch(es)/tag(s) to limit the list of # pipelines to. # limit - This limits a backlog search, default to 100. - def self.newest_first(ref: nil, limit: 100) + def self.newest_first(ref: nil, sha: nil, limit: 100) relation = order(id: :desc) relation = relation.where(ref: ref) if ref + relation = relation.where(sha: sha) if sha if limit ids = relation.limit(limit).select(:id) @@ -252,6 +253,10 @@ module Ci newest_first(ref: ref).success.take end + def self.latest_successful_for_sha(sha) + newest_first(sha: sha).success.take + end + def self.latest_successful_for_refs(refs) relation = newest_first(ref: refs).success diff --git a/app/models/project.rb b/app/models/project.rb index 8030c645e2e..9616e8c9748 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -719,14 +719,25 @@ class Project < ApplicationRecord repository.commits_by(oids: oids) end - # ref can't be HEAD, can only be branch/tag name or SHA + # ref can't be HEAD, can only be branch/tag name def latest_successful_build_for(job_name, ref = default_branch) + return unless ref + latest_pipeline = ci_pipelines.latest_successful_for(ref) return unless latest_pipeline latest_pipeline.builds.latest.with_artifacts_archive.find_by(name: job_name) end + def latest_successful_build_for_sha(job_name, sha = commit(default_branch).id) + return unless sha + + latest_pipeline = ci_pipelines.latest_successful_for_sha(sha) + return unless latest_pipeline + + latest_pipeline.builds.latest.with_artifacts_archive.find_by(name: job_name) + end + def latest_successful_build_for!(job_name, ref = default_branch) latest_successful_build_for(job_name, ref) || raise(ActiveRecord::RecordNotFound.new("Couldn't find job #{job_name}")) end diff --git a/spec/features/projects/artifacts/user_downloads_artifacts_spec.rb b/spec/features/projects/artifacts/user_downloads_artifacts_spec.rb index 5cb015e80be..69296ef00fd 100644 --- a/spec/features/projects/artifacts/user_downloads_artifacts_spec.rb +++ b/spec/features/projects/artifacts/user_downloads_artifacts_spec.rb @@ -1,8 +1,8 @@ require "spec_helper" describe "User downloads artifacts" do - set(:project) { create(:project, :public) } - set(:pipeline) { create(:ci_empty_pipeline, status: :success, project: project) } + set(:project) { create(:project, :repository, :public) } + set(:pipeline) { create(:ci_empty_pipeline, status: :success, sha: project.commit.id, project: project) } set(:job) { create(:ci_build, :artifacts, :success, pipeline: pipeline) } shared_examples "downloading" do -- cgit v1.2.1 From 1eab06571496c1b7950dc081435fa21058b614c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Tue, 23 Jul 2019 21:21:05 +0200 Subject: Add specs for latest_successful methods for SHAs --- spec/models/ci/pipeline_spec.rb | 13 ++++++++++ spec/models/project_spec.rb | 55 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index e24bbc39761..e3f699d9ad3 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -1812,6 +1812,19 @@ describe Ci::Pipeline, :mailer do end end + describe '.latest_successful_for_sha' do + include_context 'with some outdated pipelines' + + let!(:latest_successful_pipeline) do + create_pipeline(:success, 'ref', 'awesomesha', project) + end + + it 'returns the latest successful pipeline' do + expect(described_class.latest_successful_for_sha('awesomesha')) + .to eq(latest_successful_pipeline) + end + end + describe '.latest_successful_for_refs' do include_context 'with some outdated pipelines' diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 9a083eee05e..e88c6a60fec 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2074,6 +2074,61 @@ describe Project do end end + describe '#latest_successful_build_for_sha' do + let(:project) { create(:project, :repository) } + let(:pipeline) { create_pipeline(project) } + + context 'with many builds' do + it 'gives the latest builds from latest pipeline' do + pipeline1 = create_pipeline(project) + pipeline2 = create_pipeline(project) + create_build(pipeline1, 'test') + create_build(pipeline1, 'test2') + build1_p2 = create_build(pipeline2, 'test') + create_build(pipeline2, 'test2') + + expect(project.latest_successful_build_for_sha(build1_p2.name)) + .to eq(build1_p2) + end + end + + context 'with succeeded pipeline' do + let!(:build) { create_build } + + context 'standalone pipeline' do + it 'returns builds for ref for default_branch' do + expect(project.latest_successful_build_for_sha(build.name)) + .to eq(build) + end + + it 'returns empty relation if the build cannot be found' do + expect(project.latest_successful_build_for_sha('TAIL')) + .to be_nil + end + end + + context 'with some pending pipeline' do + before do + create_build(create_pipeline(project, 'pending')) + end + + it 'gives the latest build from latest pipeline' do + expect(project.latest_successful_build_for_sha(build.name)) + .to eq(build) + end + end + end + + context 'with pending pipeline' do + it 'returns empty relation' do + pipeline.update(status: 'pending') + pending_build = create_build(pipeline) + + expect(project.latest_successful_build_for_sha(pending_build.name)).to be_nil + end + end + end + describe '#latest_successful_build_for!' do let(:project) { create(:project, :repository) } let(:pipeline) { create_pipeline(project) } -- cgit v1.2.1 From 38ab1ae2f200e2071ea7329e106beb1b9232f44c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Tue, 23 Jul 2019 21:29:12 +0200 Subject: Rename latest_successful to be more explicit * Reword Project#latest_successful_build_for to Project#latest_successful_build_for_ref * Reword Ci::Pipeline#latest_successful_for to Ci::Pipeline#latest_successful_build_for_ref --- .../projects/build_artifacts_controller.rb | 2 +- app/models/ci/pipeline.rb | 3 ++- app/models/project.rb | 14 +++++------ lib/api/job_artifacts.rb | 4 ++-- lib/gitlab/badge/coverage/report.rb | 2 +- spec/models/ci/pipeline_spec.rb | 4 ++-- spec/models/project_spec.rb | 28 +++++++++++----------- 7 files changed, 29 insertions(+), 28 deletions(-) diff --git a/app/controllers/projects/build_artifacts_controller.rb b/app/controllers/projects/build_artifacts_controller.rb index 4274c356227..99f4524eec5 100644 --- a/app/controllers/projects/build_artifacts_controller.rb +++ b/app/controllers/projects/build_artifacts_controller.rb @@ -51,6 +51,6 @@ class Projects::BuildArtifactsController < Projects::ApplicationController def job_from_ref return unless @ref_name - project.latest_successful_build_for(params[:job], @ref_name) + project.latest_successful_build_for_ref(params[:job], @ref_name) end end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index f70ff7e3192..08fd1f81ffc 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -229,6 +229,7 @@ module Ci # # ref - The name (or names) of the branch(es)/tag(s) to limit the list of # pipelines to. + # sha - The commit SHA (or mutliple SHAs) to limit the list of pipelines to. # limit - This limits a backlog search, default to 100. def self.newest_first(ref: nil, sha: nil, limit: 100) relation = order(id: :desc) @@ -249,7 +250,7 @@ module Ci newest_first(ref: ref).pluck(:status).first end - def self.latest_successful_for(ref) + def self.latest_successful_for_ref(ref) newest_first(ref: ref).success.take end diff --git a/app/models/project.rb b/app/models/project.rb index 9616e8c9748..dd0922a2fbf 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -720,16 +720,16 @@ class Project < ApplicationRecord end # ref can't be HEAD, can only be branch/tag name - def latest_successful_build_for(job_name, ref = default_branch) + def latest_successful_build_for_ref(job_name, ref = default_branch) return unless ref - latest_pipeline = ci_pipelines.latest_successful_for(ref) + latest_pipeline = ci_pipelines.latest_successful_for_ref(ref) return unless latest_pipeline latest_pipeline.builds.latest.with_artifacts_archive.find_by(name: job_name) end - def latest_successful_build_for_sha(job_name, sha = commit(default_branch).id) + def latest_successful_build_for_sha(job_name, sha) return unless sha latest_pipeline = ci_pipelines.latest_successful_for_sha(sha) @@ -738,8 +738,8 @@ class Project < ApplicationRecord latest_pipeline.builds.latest.with_artifacts_archive.find_by(name: job_name) end - def latest_successful_build_for!(job_name, ref = default_branch) - latest_successful_build_for(job_name, ref) || raise(ActiveRecord::RecordNotFound.new("Couldn't find job #{job_name}")) + def latest_successful_build_for_ref!(job_name, ref = default_branch) + latest_successful_build_for_ref(job_name, ref) || raise(ActiveRecord::RecordNotFound.new("Couldn't find job #{job_name}")) end def merge_base_commit(first_commit_id, second_commit_id) @@ -1514,12 +1514,12 @@ class Project < ApplicationRecord end @latest_successful_pipeline_for_default_branch = - ci_pipelines.latest_successful_for(default_branch) + ci_pipelines.latest_successful_for_ref(default_branch) end def latest_successful_pipeline_for(ref = nil) if ref && ref != default_branch - ci_pipelines.latest_successful_for(ref) + ci_pipelines.latest_successful_for_ref(ref) else latest_successful_pipeline_for_default_branch end diff --git a/lib/api/job_artifacts.rb b/lib/api/job_artifacts.rb index e7fed55170e..b35aa952f81 100644 --- a/lib/api/job_artifacts.rb +++ b/lib/api/job_artifacts.rb @@ -27,7 +27,7 @@ module API requirements: { ref_name: /.+/ } do authorize_download_artifacts! - latest_build = user_project.latest_successful_build_for!(params[:job], params[:ref_name]) + latest_build = user_project.latest_successful_build_for_ref!(params[:job], params[:ref_name]) present_carrierwave_file!(latest_build.artifacts_file) end @@ -45,7 +45,7 @@ module API requirements: { ref_name: /.+/ } do authorize_download_artifacts! - build = user_project.latest_successful_build_for!(params[:job], params[:ref_name]) + build = user_project.latest_successful_build_for_ref!(params[:job], params[:ref_name]) path = Gitlab::Ci::Build::Artifacts::Path .new(params[:artifact_path]) diff --git a/lib/gitlab/badge/coverage/report.rb b/lib/gitlab/badge/coverage/report.rb index 7f7cc62c8ef..15cccc6f287 100644 --- a/lib/gitlab/badge/coverage/report.rb +++ b/lib/gitlab/badge/coverage/report.rb @@ -14,7 +14,7 @@ module Gitlab @ref = ref @job = job - @pipeline = @project.ci_pipelines.latest_successful_for(@ref) + @pipeline = @project.ci_pipelines.latest_successful_for_ref(@ref) end def entity diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index e3f699d9ad3..1fb83fbb088 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -1799,7 +1799,7 @@ describe Ci::Pipeline, :mailer do end end - describe '.latest_successful_for' do + describe '.latest_successful_for_ref' do include_context 'with some outdated pipelines' let!(:latest_successful_pipeline) do @@ -1807,7 +1807,7 @@ describe Ci::Pipeline, :mailer do end it 'returns the latest successful pipeline' do - expect(described_class.latest_successful_for('ref')) + expect(described_class.latest_successful_for_ref('ref')) .to eq(latest_successful_pipeline) end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index e88c6a60fec..fc778174314 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2019,7 +2019,7 @@ describe Project do end end - describe '#latest_successful_build_for' do + describe '#latest_successful_build_for_ref' do let(:project) { create(:project, :repository) } let(:pipeline) { create_pipeline(project) } @@ -2032,7 +2032,7 @@ describe Project do build1_p2 = create_build(pipeline2, 'test') create_build(pipeline2, 'test2') - expect(project.latest_successful_build_for(build1_p2.name)) + expect(project.latest_successful_build_for_ref(build1_p2.name)) .to eq(build1_p2) end end @@ -2042,12 +2042,12 @@ describe Project do context 'standalone pipeline' do it 'returns builds for ref for default_branch' do - expect(project.latest_successful_build_for(build.name)) + expect(project.latest_successful_build_for_ref(build.name)) .to eq(build) end it 'returns empty relation if the build cannot be found' do - expect(project.latest_successful_build_for('TAIL')) + expect(project.latest_successful_build_for_ref('TAIL')) .to be_nil end end @@ -2058,7 +2058,7 @@ describe Project do end it 'gives the latest build from latest pipeline' do - expect(project.latest_successful_build_for(build.name)) + expect(project.latest_successful_build_for_ref(build.name)) .to eq(build) end end @@ -2069,7 +2069,7 @@ describe Project do pipeline.update(status: 'pending') pending_build = create_build(pipeline) - expect(project.latest_successful_build_for(pending_build.name)).to be_nil + expect(project.latest_successful_build_for_ref(pending_build.name)).to be_nil end end end @@ -2129,7 +2129,7 @@ describe Project do end end - describe '#latest_successful_build_for!' do + describe '#latest_successful_build_for_ref!' do let(:project) { create(:project, :repository) } let(:pipeline) { create_pipeline(project) } @@ -2142,7 +2142,7 @@ describe Project do build1_p2 = create_build(pipeline2, 'test') create_build(pipeline2, 'test2') - expect(project.latest_successful_build_for(build1_p2.name)) + expect(project.latest_successful_build_for_ref!(build1_p2.name)) .to eq(build1_p2) end end @@ -2152,12 +2152,12 @@ describe Project do context 'standalone pipeline' do it 'returns builds for ref for default_branch' do - expect(project.latest_successful_build_for!(build.name)) + expect(project.latest_successful_build_for_ref!(build.name)) .to eq(build) end it 'returns exception if the build cannot be found' do - expect { project.latest_successful_build_for!(build.name, 'TAIL') } + expect { project.latest_successful_build_for_ref!(build.name, 'TAIL') } .to raise_error(ActiveRecord::RecordNotFound) end end @@ -2168,7 +2168,7 @@ describe Project do end it 'gives the latest build from latest pipeline' do - expect(project.latest_successful_build_for!(build.name)) + expect(project.latest_successful_build_for_ref!(build.name)) .to eq(build) end end @@ -2179,7 +2179,7 @@ describe Project do pipeline.update(status: 'pending') pending_build = create_build(pipeline) - expect { project.latest_successful_build_for!(pending_build.name) } + expect { project.latest_successful_build_for_ref!(pending_build.name) } .to raise_error(ActiveRecord::RecordNotFound) end end @@ -4091,7 +4091,7 @@ describe Project do context 'with a ref that is not the default branch' do it 'returns the latest successful pipeline for the given ref' do - expect(project.ci_pipelines).to receive(:latest_successful_for).with('foo') + expect(project.ci_pipelines).to receive(:latest_successful_for_ref).with('foo') project.latest_successful_pipeline_for('foo') end @@ -4119,7 +4119,7 @@ describe Project do it 'memoizes and returns the latest successful pipeline for the default branch' do pipeline = double(:pipeline) - expect(project.ci_pipelines).to receive(:latest_successful_for) + expect(project.ci_pipelines).to receive(:latest_successful_for_ref) .with(project.default_branch) .and_return(pipeline) .once -- cgit v1.2.1 From ae58f82e62d60edf0a1de2790d1798f5b5f16b28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Tue, 23 Jul 2019 22:13:56 +0200 Subject: Extract common spec elements to shared_examples --- spec/models/project_spec.rb | 100 ++------------------- ...project_latest_successful_build_for_examples.rb | 63 +++++++++++++ 2 files changed, 71 insertions(+), 92 deletions(-) create mode 100644 spec/support/shared_examples/project_latest_successful_build_for_examples.rb diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index fc778174314..a44afe9285f 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2023,54 +2023,16 @@ describe Project do let(:project) { create(:project, :repository) } let(:pipeline) { create_pipeline(project) } - context 'with many builds' do - it 'gives the latest builds from latest pipeline' do - pipeline1 = create_pipeline(project) - pipeline2 = create_pipeline(project) - create_build(pipeline1, 'test') - create_build(pipeline1, 'test2') - build1_p2 = create_build(pipeline2, 'test') - create_build(pipeline2, 'test2') - - expect(project.latest_successful_build_for_ref(build1_p2.name)) - .to eq(build1_p2) - end - end + it_behaves_like 'latest successful build for sha or ref' - context 'with succeeded pipeline' do - let!(:build) { create_build } + subject { project.latest_successful_build_for_ref(build_name) } - context 'standalone pipeline' do - it 'returns builds for ref for default_branch' do - expect(project.latest_successful_build_for_ref(build.name)) - .to eq(build) - end + context 'with a specified ref' do + let(:build) { create_build } - it 'returns empty relation if the build cannot be found' do - expect(project.latest_successful_build_for_ref('TAIL')) - .to be_nil - end - end + subject { project.latest_successful_build_for_ref(build.name, project.default_branch) } - context 'with some pending pipeline' do - before do - create_build(create_pipeline(project, 'pending')) - end - - it 'gives the latest build from latest pipeline' do - expect(project.latest_successful_build_for_ref(build.name)) - .to eq(build) - end - end - end - - context 'with pending pipeline' do - it 'returns empty relation' do - pipeline.update(status: 'pending') - pending_build = create_build(pipeline) - - expect(project.latest_successful_build_for_ref(pending_build.name)).to be_nil - end + it { is_expected.to eq(build) } end end @@ -2078,55 +2040,9 @@ describe Project do let(:project) { create(:project, :repository) } let(:pipeline) { create_pipeline(project) } - context 'with many builds' do - it 'gives the latest builds from latest pipeline' do - pipeline1 = create_pipeline(project) - pipeline2 = create_pipeline(project) - create_build(pipeline1, 'test') - create_build(pipeline1, 'test2') - build1_p2 = create_build(pipeline2, 'test') - create_build(pipeline2, 'test2') - - expect(project.latest_successful_build_for_sha(build1_p2.name)) - .to eq(build1_p2) - end - end - - context 'with succeeded pipeline' do - let!(:build) { create_build } - - context 'standalone pipeline' do - it 'returns builds for ref for default_branch' do - expect(project.latest_successful_build_for_sha(build.name)) - .to eq(build) - end + it_behaves_like 'latest successful build for sha or ref' - it 'returns empty relation if the build cannot be found' do - expect(project.latest_successful_build_for_sha('TAIL')) - .to be_nil - end - end - - context 'with some pending pipeline' do - before do - create_build(create_pipeline(project, 'pending')) - end - - it 'gives the latest build from latest pipeline' do - expect(project.latest_successful_build_for_sha(build.name)) - .to eq(build) - end - end - end - - context 'with pending pipeline' do - it 'returns empty relation' do - pipeline.update(status: 'pending') - pending_build = create_build(pipeline) - - expect(project.latest_successful_build_for_sha(pending_build.name)).to be_nil - end - end + subject { project.latest_successful_build_for_sha(build_name, project.commit.sha) } end describe '#latest_successful_build_for_ref!' do diff --git a/spec/support/shared_examples/project_latest_successful_build_for_examples.rb b/spec/support/shared_examples/project_latest_successful_build_for_examples.rb new file mode 100644 index 00000000000..a9bd23e9fc9 --- /dev/null +++ b/spec/support/shared_examples/project_latest_successful_build_for_examples.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +shared_examples 'latest successful build for sha or ref' do + context 'with many builds' do + let(:other_pipeline) { create_pipeline(project) } + let(:other_build) { create_build(other_pipeline, 'test') } + let(:build_name) { other_build.name } + + before do + pipeline1 = create_pipeline(project) + pipeline2 = create_pipeline(project) + create_build(pipeline1, 'test') + create_build(pipeline1, 'test2') + create_build(pipeline2, 'test2') + end + + it 'gives the latest builds from latest pipeline' do + expect(subject).to eq(other_build) + end + end + + context 'with succeeded pipeline' do + let!(:build) { create_build } + let(:build_name) { build.name } + + context 'standalone pipeline' do + it 'returns builds for ref for default_branch' do + expect(subject).to eq(build) + end + + context 'with nonexistent build' do + let(:build_name) { 'TAIL' } + + it 'returns empty relation if the build cannot be found' do + expect(subject).to be_nil + end + end + end + + context 'with some pending pipeline' do + before do + create_build(create_pipeline(project, 'pending')) + end + + it 'gives the latest build from latest pipeline' do + expect(subject).to eq(build) + end + end + end + + context 'with pending pipeline' do + let!(:pending_build) { create_build(pipeline) } + let(:build_name) { pending_build.name } + + before do + pipeline.update(status: 'pending') + end + + it 'returns empty relation' do + expect(subject).to be_nil + end + end +end -- cgit v1.2.1