Commit 27b4c73d authored by Furkan Ayhan's avatar Furkan Ayhan

Refactor Ci::BuilDependencies methods

- Moved private methods under the private keyword
- Changed the tests according to it
parent 439dd31c
......@@ -14,15 +14,33 @@ module Ci
(local + cross_pipeline + cross_project).uniq
end
def invalid_local
local.reject(&:valid_dependency?)
end
def valid?
valid_local? && valid_cross_pipeline? && valid_cross_project?
end
private
# Dependencies can only be of Ci::Build type because only builds
# can create artifacts
def model_class
::Ci::Build
end
# Dependencies local to the given pipeline
def local
return [] if no_local_dependencies_specified?
return [] unless processable.pipeline_id # we don't have any dependency when creating the pipeline
strong_memoize(:local) do
next [] if no_local_dependencies_specified?
next [] unless processable.pipeline_id # we don't have any dependency when creating the pipeline
deps = model_class.where(pipeline_id: processable.pipeline_id).latest
deps = from_previous_stages(deps)
deps = from_needs(deps)
from_dependencies(deps)
from_dependencies(deps).to_a
end
end
# Dependencies from the same parent-pipeline hierarchy excluding
......@@ -38,22 +56,6 @@ module Ci
[]
end
def invalid_local
local.reject(&:valid_dependency?)
end
def valid?
valid_local? && valid_cross_pipeline? && valid_cross_project?
end
private
# Dependencies can only be of Ci::Build type because only builds
# can create artifacts
def model_class
::Ci::Build
end
def fetch_dependencies_in_hierarchy
deps_specifications = specified_cross_pipeline_dependencies
return [] if deps_specifications.empty?
......
......@@ -9,6 +9,8 @@ module EE
CROSS_PROJECT_LIMIT = ::Gitlab::Ci::Config::Entry::Needs::NEEDS_CROSS_PROJECT_DEPENDENCIES_LIMIT
private
override :cross_project
def cross_project
strong_memoize(:cross_project) do
......@@ -16,8 +18,6 @@ module EE
end
end
private
override :valid_cross_project?
def valid_cross_project?
cross_project.size == specified_cross_project_dependencies.size
......
......@@ -16,6 +16,14 @@ RSpec.describe Ci::BuildDependencies do
status: 'success')
end
let(:pipeline2) do
create(:ci_pipeline,
project: project,
sha: project.commit.id,
ref: project.default_branch,
status: 'success')
end
let!(:job) do
create(:ci_build,
pipeline: pipeline,
......@@ -32,8 +40,8 @@ RSpec.describe Ci::BuildDependencies do
stub_licensed_features(cross_project_pipelines: true)
end
describe '#cross_project' do
subject { described_class.new(job).cross_project }
context 'for cross_project dependencies' do
subject { described_class.new(job).all }
context 'when cross_dependencies are not defined' do
it { is_expected.to be_empty }
......@@ -57,7 +65,7 @@ RSpec.describe Ci::BuildDependencies do
context 'with cross_dependencies to the same project' do
let!(:dependency) do
create(:ci_build, :success,
pipeline: pipeline,
pipeline: pipeline2,
name: 'dependency',
stage_idx: 1,
stage: 'build',
......@@ -70,7 +78,7 @@ RSpec.describe Ci::BuildDependencies do
{
project: project.full_path,
job: 'dependency',
ref: pipeline.ref,
ref: pipeline2.ref,
artifacts: artifacts
}
]
......@@ -231,7 +239,7 @@ RSpec.describe Ci::BuildDependencies do
before do
cross_dependencies_limit.next.times do |index|
create(:ci_build, :success,
pipeline: pipeline, name: "dependency-#{index}",
pipeline: pipeline2, name: "dependency-#{index}",
stage_idx: 1, stage: 'build', user: user
)
end
......@@ -242,7 +250,7 @@ RSpec.describe Ci::BuildDependencies do
{
project: project.full_path,
job: "dependency-#{index}",
ref: pipeline.ref,
ref: pipeline2.ref,
artifacts: true
}
end
......
......@@ -22,8 +22,8 @@ RSpec.describe Ci::BuildDependencies do
stub_feature_flags(ci_validate_build_dependencies_override: false)
end
describe '#local' do
subject { described_class.new(job).local }
context 'for local dependencies' do
subject { described_class.new(job).all }
describe 'jobs from previous stages' do
context 'when job is in the first stage' do
......@@ -52,7 +52,7 @@ RSpec.describe Ci::BuildDependencies do
project.add_developer(user)
end
let(:retried_job) { Ci::Build.retry(rspec_test, user) }
let!(:retried_job) { Ci::Build.retry(rspec_test, user) }
it 'contains the retried job instead of the original one' do
is_expected.to contain_exactly(build, retried_job, rubocop_test)
......@@ -150,7 +150,7 @@ RSpec.describe Ci::BuildDependencies do
end
end
describe '#cross_pipeline' do
context 'for cross_pipeline dependencies' do
let!(:job) do
create(:ci_build,
pipeline: pipeline,
......@@ -160,7 +160,7 @@ RSpec.describe Ci::BuildDependencies do
subject { described_class.new(job) }
let(:cross_pipeline_deps) { subject.cross_pipeline }
let(:cross_pipeline_deps) { subject.all }
context 'when dependency specifications are valid' do
context 'when pipeline exists in the hierarchy' do
......
......@@ -502,8 +502,8 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
expect { request_job }.to exceed_all_query_limit(1).for_model(::Ci::JobArtifact)
end
it 'queries the ci_builds table more than five times' do
expect { request_job }.to exceed_all_query_limit(5).for_model(::Ci::Build)
it 'queries the ci_builds table more than three times' do
expect { request_job }.to exceed_all_query_limit(3).for_model(::Ci::Build)
end
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment