summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/services/projects/update_pages_service.rb11
-rw-r--r--changelogs/unreleased/45481-sane-pages-artifacts.yml6
-rw-r--r--spec/services/projects/update_pages_service_spec.rb51
3 files changed, 17 insertions, 51 deletions
diff --git a/app/services/projects/update_pages_service.rb b/app/services/projects/update_pages_service.rb
index de77f6bf585..bc3eae36574 100644
--- a/app/services/projects/update_pages_service.rb
+++ b/app/services/projects/update_pages_service.rb
@@ -40,7 +40,7 @@ module Projects
rescue InvaildStateError => e
error(e.message)
rescue => e
- error(e.message, false)
+ error(e.message)
raise e
end
@@ -48,17 +48,15 @@ module Projects
def success
@status.success
- delete_artifact!
super
end
- def error(message, allow_delete_artifact = true)
+ def error(message)
register_failure
log_error("Projects::UpdatePagesService: #{message}")
@status.allow_failure = !latest?
@status.description = message
@status.drop(:script_failure)
- delete_artifact! if allow_delete_artifact
super
end
@@ -162,11 +160,6 @@ module Projects
build.artifacts_file.path
end
- def delete_artifact!
- build.reload # Reload stable object to prevent erase artifacts with old state
- build.erase_artifacts! unless build.has_expiring_artifacts?
- end
-
def latest_sha
project.commit(build.ref).try(:sha).to_s
ensure
diff --git a/changelogs/unreleased/45481-sane-pages-artifacts.yml b/changelogs/unreleased/45481-sane-pages-artifacts.yml
new file mode 100644
index 00000000000..b9c68b70012
--- /dev/null
+++ b/changelogs/unreleased/45481-sane-pages-artifacts.yml
@@ -0,0 +1,6 @@
+---
+title: Don't automatically remove artifacts for pages jobs after pages:deploy has
+ run
+merge_request: 18628
+author:
+type: fixed
diff --git a/spec/services/projects/update_pages_service_spec.rb b/spec/services/projects/update_pages_service_spec.rb
index 1b6caeab15d..a418808fd26 100644
--- a/spec/services/projects/update_pages_service_spec.rb
+++ b/spec/services/projects/update_pages_service_spec.rb
@@ -29,25 +29,10 @@ describe Projects::UpdatePagesService do
end
describe 'pages artifacts' do
- context 'with expiry date' do
- before do
- build.artifacts_expire_in = "2 days"
- build.save!
- end
-
- it "doesn't delete artifacts" do
- expect(execute).to eq(:success)
-
- expect(build.reload.artifacts?).to eq(true)
- end
- end
-
- context 'without expiry date' do
- it "does delete artifacts" do
- expect(execute).to eq(:success)
+ it "doesn't delete artifacts after deploying" do
+ expect(execute).to eq(:success)
- expect(build.reload.artifacts?).to eq(false)
- end
+ expect(build.reload.artifacts?).to eq(true)
end
end
@@ -100,25 +85,10 @@ describe Projects::UpdatePagesService do
end
describe 'pages artifacts' do
- context 'with expiry date' do
- before do
- build.artifacts_expire_in = "2 days"
- build.save!
- end
-
- it "doesn't delete artifacts" do
- expect(execute).to eq(:success)
-
- expect(build.artifacts?).to eq(true)
- end
- end
-
- context 'without expiry date' do
- it "does delete artifacts" do
- expect(execute).to eq(:success)
+ it "doesn't delete artifacts after deploying" do
+ expect(execute).to eq(:success)
- expect(build.reload.artifacts?).to eq(false)
- end
+ expect(build.artifacts?).to eq(true)
end
end
@@ -171,13 +141,12 @@ describe Projects::UpdatePagesService do
build.reload
expect(deploy_status).to be_failed
- expect(build.artifacts?).to be_truthy
end
end
context 'when failed to extract zip artifacts' do
before do
- allow_any_instance_of(described_class)
+ expect_any_instance_of(described_class)
.to receive(:extract_zip_archive!)
.and_raise(Projects::UpdatePagesService::FailedToExtractError)
end
@@ -188,21 +157,19 @@ describe Projects::UpdatePagesService do
build.reload
expect(deploy_status).to be_failed
- expect(build.artifacts?).to be_truthy
end
end
context 'when missing artifacts metadata' do
before do
- allow(build).to receive(:artifacts_metadata?).and_return(false)
+ expect(build).to receive(:artifacts_metadata?).and_return(false)
end
- it 'does not raise an error and remove artifacts as failed job' do
+ it 'does not raise an error as failed job' do
execute
build.reload
expect(deploy_status).to be_failed
- expect(build.artifacts?).to be_falsey
end
end
end