Commit ad113a3c authored by Nick Thomas's avatar Nick Thomas

Don't automatically remove artifacts for pages jobs after pages:deploy has run

parent 4de3bc5f
......@@ -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
......
---
title: Don't automatically remove artifacts for pages jobs after pages:deploy has
run
merge_request: 18628
author:
type: fixed
......@@ -29,28 +29,13 @@ 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
it "doesn't delete artifacts after deploying" 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)
expect(build.reload.artifacts?).to eq(false)
end
end
end
it 'succeeds' do
expect(project.pages_deployed?).to be_falsey
expect(execute).to eq(:success)
......@@ -100,28 +85,13 @@ 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
it "doesn't delete artifacts after deploying" 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)
expect(build.reload.artifacts?).to eq(false)
end
end
end
it 'succeeds' do
expect(project.pages_deployed?).to be_falsey
expect(execute).to eq(:success)
......@@ -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
......
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