From dd9d8ce82e5957974f87401d6dd437e4d2f3ad38 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 1 Mar 2017 11:39:36 +0100
Subject: [PATCH 1/4] Fix runner tags assignment when retrying a job
---
app/services/ci/retry_build_service.rb | 31 +++++++++++---------
spec/factories/ci/builds.rb | 4 +++
spec/services/ci/retry_build_service_spec.rb | 29 ++++++++++++------
3 files changed, 41 insertions(+), 23 deletions(-)
diff --git a/app/services/ci/retry_build_service.rb b/app/services/ci/retry_build_service.rb
index 38ef323f6e5b..001a3e1214af 100644
--- a/app/services/ci/retry_build_service.rb
+++ b/app/services/ci/retry_build_service.rb
@@ -1,18 +1,21 @@
module Ci
class RetryBuildService < ::BaseService
- CLONE_ATTRIBUTES = %i[pipeline project ref tag options commands name
- allow_failure stage stage_idx trigger_request
- yaml_variables when environment coverage_regex]
- .freeze
-
- REJECT_ATTRIBUTES = %i[id status user token coverage trace runner
- artifacts_expire_at artifacts_file
- artifacts_metadata artifacts_size
- created_at updated_at started_at finished_at
- queued_at erased_by erased_at].freeze
-
- IGNORE_ATTRIBUTES = %i[type lock_version gl_project_id target_url
- deploy job_id description].freeze
+ CLONE_ACCESSORS = %i[pipeline project ref tag options commands name
+ allow_failure stage stage_idx trigger_request
+ yaml_variables when environment coverage_regex
+ description tag_list].freeze
+
+ REJECT_ACCESSORS = %i[id status user token coverage trace runner
+ artifacts_expire_at artifacts_file
+ artifacts_metadata artifacts_size
+ created_at updated_at started_at finished_at
+ queued_at erased_by erased_at].freeze
+
+ IGNORE_ACCESSORS = %i[type lock_version target_url gl_project_id
+ deploy job_id base_tags commit_id deployments
+ erased_by_id last_deployment project_id runner_id
+ tag_taggings taggings tags trigger_request_id
+ user_id].freeze
def execute(build)
reprocess(build).tap do |new_build|
@@ -31,7 +34,7 @@ def reprocess(build)
raise Gitlab::Access::AccessDeniedError
end
- attributes = CLONE_ATTRIBUTES.map do |attribute|
+ attributes = CLONE_ACCESSORS.map do |attribute|
[attribute, build.send(attribute)]
end
diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb
index a90534d10ba2..8218293604d3 100644
--- a/spec/factories/ci/builds.rb
+++ b/spec/factories/ci/builds.rb
@@ -76,6 +76,10 @@
manual
end
+ trait :tags do
+ tag_list [:docker, :ruby]
+ end
+
after(:build) do |build, evaluator|
build.project = build.pipeline.project
end
diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb
index d03f7505eacc..f4bc05377dc3 100644
--- a/spec/services/ci/retry_build_service_spec.rb
+++ b/spec/services/ci/retry_build_service_spec.rb
@@ -13,11 +13,11 @@
shared_examples 'build duplication' do
let(:build) do
create(:ci_build, :failed, :artifacts_expired, :erased, :trace,
- :queued, :coverage, pipeline: pipeline)
+ :queued, :coverage, :tags, pipeline: pipeline)
end
describe 'clone attributes' do
- described_class::CLONE_ATTRIBUTES.each do |attribute|
+ described_class::CLONE_ACCESSORS.each do |attribute|
it "clones #{attribute} build attribute" do
expect(new_build.send(attribute)).to eq build.send(attribute)
end
@@ -25,7 +25,7 @@
end
describe 'reject attributes' do
- described_class::REJECT_ATTRIBUTES.each do |attribute|
+ described_class::REJECT_ACCESSORS.each do |attribute|
it "does not clone #{attribute} build attribute" do
expect(new_build.send(attribute)).not_to eq build.send(attribute)
end
@@ -33,12 +33,23 @@
end
it 'has correct number of known attributes' do
- attributes =
- described_class::CLONE_ATTRIBUTES +
- described_class::IGNORE_ATTRIBUTES +
- described_class::REJECT_ATTRIBUTES
-
- expect(build.attributes.size).to eq(attributes.size)
+ known_accessors =
+ described_class::CLONE_ACCESSORS +
+ described_class::IGNORE_ACCESSORS +
+ described_class::REJECT_ACCESSORS
+
+ # :tag_list is a special case, this accessor does not exist
+ # in reflected associations, comes from `act_as_taggable` and
+ # we use it to copy tags, instead of reusing tags.
+ #
+ current_accessors =
+ build.attribute_names.map.map(&:to_sym) +
+ build._reflections.map { |assoc| assoc.first.to_sym } +
+ [:tag_list]
+
+ current_accessors.uniq!
+
+ expect(known_accessors).to contain_exactly(*current_accessors)
end
end
--
GitLab
From 6f84345725ef11493653a888ed59fc6411825779 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 1 Mar 2017 11:57:06 +0100
Subject: [PATCH 2/4] Improve build retry service tests for cloning data
---
spec/factories/ci/builds.rb | 8 ++++++++
spec/services/ci/retry_build_service_spec.rb | 7 +++++--
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb
index 8218293604d3..cabe128acf74 100644
--- a/spec/factories/ci/builds.rb
+++ b/spec/factories/ci/builds.rb
@@ -80,6 +80,14 @@
tag_list [:docker, :ruby]
end
+ trait :on_tag do
+ tag true
+ end
+
+ trait :triggered do
+ trigger_request factory: :ci_trigger_request_with_variables
+ end
+
after(:build) do |build, evaluator|
build.project = build.pipeline.project
end
diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb
index f4bc05377dc3..bcd895446476 100644
--- a/spec/services/ci/retry_build_service_spec.rb
+++ b/spec/services/ci/retry_build_service_spec.rb
@@ -12,13 +12,16 @@
shared_examples 'build duplication' do
let(:build) do
- create(:ci_build, :failed, :artifacts_expired, :erased, :trace,
- :queued, :coverage, :tags, pipeline: pipeline)
+ create(:ci_build, :failed, :artifacts_expired, :erased,
+ :queued, :coverage, :tags, :allowed_to_fail, :on_tag,
+ :teardown_environment, :triggered, :trace,
+ description: 'some build', pipeline: pipeline)
end
describe 'clone attributes' do
described_class::CLONE_ACCESSORS.each do |attribute|
it "clones #{attribute} build attribute" do
+ expect(new_build.send(attribute)).to be_present
expect(new_build.send(attribute)).to eq build.send(attribute)
end
end
--
GitLab
From c3d24d0bdd2559715231eca1cd08986277cb8fc6 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 2 Mar 2017 11:51:24 +0100
Subject: [PATCH 3/4] Move unused consts from retry build service to specs
---
app/services/ci/retry_build_service.rb | 14 +--------
spec/services/ci/retry_build_service_spec.rb | 31 +++++++++++++-------
2 files changed, 22 insertions(+), 23 deletions(-)
diff --git a/app/services/ci/retry_build_service.rb b/app/services/ci/retry_build_service.rb
index 001a3e1214af..ae9ddc354466 100644
--- a/app/services/ci/retry_build_service.rb
+++ b/app/services/ci/retry_build_service.rb
@@ -3,19 +3,7 @@ class RetryBuildService < ::BaseService
CLONE_ACCESSORS = %i[pipeline project ref tag options commands name
allow_failure stage stage_idx trigger_request
yaml_variables when environment coverage_regex
- description tag_list].freeze
-
- REJECT_ACCESSORS = %i[id status user token coverage trace runner
- artifacts_expire_at artifacts_file
- artifacts_metadata artifacts_size
- created_at updated_at started_at finished_at
- queued_at erased_by erased_at].freeze
-
- IGNORE_ACCESSORS = %i[type lock_version target_url gl_project_id
- deploy job_id base_tags commit_id deployments
- erased_by_id last_deployment project_id runner_id
- tag_taggings taggings tags trigger_request_id
- user_id].freeze
+ description tag_list].freeze
def execute(build)
reprocess(build).tap do |new_build|
diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb
index bcd895446476..65af4e13118e 100644
--- a/spec/services/ci/retry_build_service_spec.rb
+++ b/spec/services/ci/retry_build_service_spec.rb
@@ -10,6 +10,20 @@
described_class.new(project, user)
end
+ CLONE_ACCESSORS = described_class::CLONE_ACCESSORS
+
+ REJECT_ACCESSORS =
+ %i[id status user token coverage trace runner artifacts_expire_at
+ artifacts_file artifacts_metadata artifacts_size created_at
+ updated_at started_at finished_at queued_at erased_by
+ erased_at].freeze
+
+ IGNORE_ACCESSORS =
+ %i[type lock_version target_url gl_project_id deploy job_id base_tags
+ commit_id deployments erased_by_id last_deployment project_id
+ runner_id tag_taggings taggings tags trigger_request_id
+ user_id].freeze
+
shared_examples 'build duplication' do
let(:build) do
create(:ci_build, :failed, :artifacts_expired, :erased,
@@ -18,8 +32,8 @@
description: 'some build', pipeline: pipeline)
end
- describe 'clone attributes' do
- described_class::CLONE_ACCESSORS.each do |attribute|
+ describe 'clone accessors' do
+ CLONE_ACCESSORS.each do |attribute|
it "clones #{attribute} build attribute" do
expect(new_build.send(attribute)).to be_present
expect(new_build.send(attribute)).to eq build.send(attribute)
@@ -27,8 +41,8 @@
end
end
- describe 'reject attributes' do
- described_class::REJECT_ACCESSORS.each do |attribute|
+ describe 'reject acessors' do
+ REJECT_ACCESSORS.each do |attribute|
it "does not clone #{attribute} build attribute" do
expect(new_build.send(attribute)).not_to eq build.send(attribute)
end
@@ -36,18 +50,15 @@
end
it 'has correct number of known attributes' do
- known_accessors =
- described_class::CLONE_ACCESSORS +
- described_class::IGNORE_ACCESSORS +
- described_class::REJECT_ACCESSORS
+ known_accessors = CLONE_ACCESSORS + REJECT_ACCESSORS + IGNORE_ACCESSORS
# :tag_list is a special case, this accessor does not exist
# in reflected associations, comes from `act_as_taggable` and
# we use it to copy tags, instead of reusing tags.
#
current_accessors =
- build.attribute_names.map.map(&:to_sym) +
- build._reflections.map { |assoc| assoc.first.to_sym } +
+ Ci::Build.attribute_names.map(&:to_sym) +
+ Ci::Build.reflect_on_all_associations.map(&:name) +
[:tag_list]
current_accessors.uniq!
--
GitLab
From f0ba1ea3c1157d9aae472872481982ac53fb90a8 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 2 Mar 2017 22:14:12 +0100
Subject: [PATCH 4/4] Fix Rubocop offense in build retry service
---
app/services/ci/retry_build_service.rb | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/app/services/ci/retry_build_service.rb b/app/services/ci/retry_build_service.rb
index ae9ddc354466..89da05b72bb5 100644
--- a/app/services/ci/retry_build_service.rb
+++ b/app/services/ci/retry_build_service.rb
@@ -3,7 +3,7 @@ class RetryBuildService < ::BaseService
CLONE_ACCESSORS = %i[pipeline project ref tag options commands name
allow_failure stage stage_idx trigger_request
yaml_variables when environment coverage_regex
- description tag_list].freeze
+ description tag_list].freeze
def execute(build)
reprocess(build).tap do |new_build|
--
GitLab