From 92d361c8d34d3ef29ff546ed1f7b74ce9a8f81f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sun, 11 Mar 2018 23:51:10 +0100 Subject: [PATCH 1/4] Add empty repo check before running AutoDevOps pipeline --- .../projects/pipelines_settings_controller.rb | 8 +++-- .../pipelines_settings_controller_spec.rb | 34 +++++++++++++++++-- 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/app/controllers/projects/pipelines_settings_controller.rb b/app/controllers/projects/pipelines_settings_controller.rb index 06ce7328fb5..6ccefb7e562 100644 --- a/app/controllers/projects/pipelines_settings_controller.rb +++ b/app/controllers/projects/pipelines_settings_controller.rb @@ -11,8 +11,12 @@ class Projects::PipelinesSettingsController < Projects::ApplicationController flash[:notice] = "Pipelines settings for '#{@project.name}' were successfully updated." if service.run_auto_devops_pipeline? - CreatePipelineWorker.perform_async(project.id, current_user.id, project.default_branch, :web, ignore_skip_ci: true, save_on_errors: false) - flash[:success] = "A new Auto DevOps pipeline has been created, go to Pipelines page for details".html_safe + if @project.empty_repo? + flash[:warning] = "This repository is currently empty. A new Auto DevOps pipeline will be created after a new file has been pushed to a branch." + else + CreatePipelineWorker.perform_async(project.id, current_user.id, project.default_branch, :web, ignore_skip_ci: true, save_on_errors: false) + flash[:success] = "A new Auto DevOps pipeline has been created, go to Pipelines page for details".html_safe + end end redirect_to project_settings_ci_cd_path(@project) diff --git a/spec/controllers/projects/pipelines_settings_controller_spec.rb b/spec/controllers/projects/pipelines_settings_controller_spec.rb index 1cc488bef32..0dd8575e2b7 100644 --- a/spec/controllers/projects/pipelines_settings_controller_spec.rb +++ b/spec/controllers/projects/pipelines_settings_controller_spec.rb @@ -47,10 +47,38 @@ describe Projects::PipelinesSettingsController do expect_any_instance_of(Projects::UpdateService).to receive(:run_auto_devops_pipeline?).and_return(true) end - it 'queues a CreatePipelineWorker' do - expect(CreatePipelineWorker).to receive(:perform_async).with(project.id, user.id, project.default_branch, :web, any_args) + context 'when the project repository is empty' do + before do + allow_any_instance_of(Project).to receive(:empty_repo?).and_return(true) + end - subject + it 'sets a warning flash' do + expect(subject).to set_flash[:warning] + end + + it 'does not queue a CreatePipelineWorker' do + expect(CreatePipelineWorker).not_to receive(:perform_async).with(project.id, user.id, project.default_branch, :web, any_args) + + subject + end + end + + context 'when the project repository is not empty' do + before do + allow_any_instance_of(Project).to receive(:empty_repo?).and_return(false) + end + + it 'sets a success flash' do + allow(CreatePipelineWorker).to receive(:perform_async).with(project.id, user.id, project.default_branch, :web, any_args) + + expect(subject).to set_flash[:success] + end + + it 'queues a CreatePipelineWorker' do + expect(CreatePipelineWorker).to receive(:perform_async).with(project.id, user.id, project.default_branch, :web, any_args) + + subject + end end end From 1ff339fad3e63a7ec2961de596e7b0fe4272fae9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sun, 11 Mar 2018 23:55:11 +0100 Subject: [PATCH 2/4] Add CHANGELOG --- ...evops-on-an-empty-project-gives-you-wrong-information.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/43482-enabling-auto-devops-on-an-empty-project-gives-you-wrong-information.yml diff --git a/changelogs/unreleased/43482-enabling-auto-devops-on-an-empty-project-gives-you-wrong-information.yml b/changelogs/unreleased/43482-enabling-auto-devops-on-an-empty-project-gives-you-wrong-information.yml new file mode 100644 index 00000000000..889fd008bad --- /dev/null +++ b/changelogs/unreleased/43482-enabling-auto-devops-on-an-empty-project-gives-you-wrong-information.yml @@ -0,0 +1,5 @@ +--- +title: Add empty repo check before running AutoDevOps pipeline +merge_request: 17605 +author: +type: changed From 78cee2c57b49d396c5998becfeb5032de196c939 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Thu, 22 Mar 2018 12:57:56 +0100 Subject: [PATCH 3/4] Refactor AutoDevops pipeline logic into method --- .../projects/pipelines_settings_controller.rb | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/app/controllers/projects/pipelines_settings_controller.rb b/app/controllers/projects/pipelines_settings_controller.rb index 6ccefb7e562..557671ab186 100644 --- a/app/controllers/projects/pipelines_settings_controller.rb +++ b/app/controllers/projects/pipelines_settings_controller.rb @@ -10,14 +10,7 @@ class Projects::PipelinesSettingsController < Projects::ApplicationController if service.execute flash[:notice] = "Pipelines settings for '#{@project.name}' were successfully updated." - if service.run_auto_devops_pipeline? - if @project.empty_repo? - flash[:warning] = "This repository is currently empty. A new Auto DevOps pipeline will be created after a new file has been pushed to a branch." - else - CreatePipelineWorker.perform_async(project.id, current_user.id, project.default_branch, :web, ignore_skip_ci: true, save_on_errors: false) - flash[:success] = "A new Auto DevOps pipeline has been created, go to Pipelines page for details".html_safe - end - end + run_autodevops_pipeline(service) redirect_to project_settings_ci_cd_path(@project) else @@ -28,6 +21,18 @@ class Projects::PipelinesSettingsController < Projects::ApplicationController private + def run_autodevops_pipeline(service) + return unless service.run_auto_devops_pipeline? + + if @project.empty_repo? + flash[:warning] = "This repository is currently empty. A new Auto DevOps pipeline will be created after a new file has been pushed to a branch." + return + end + + CreatePipelineWorker.perform_async(project.id, current_user.id, project.default_branch, :web, ignore_skip_ci: true, save_on_errors: false) + flash[:success] = "A new Auto DevOps pipeline has been created, go to Pipelines page for details".html_safe + end + def update_params params.require(:project).permit( :runners_token, :builds_enabled, :build_allow_git_fetch, From 92d51da23dfa3e83fb87fd381553a1fd93d49614 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Thu, 22 Mar 2018 13:33:24 +0100 Subject: [PATCH 4/4] Use actual repo instead of stubbing method --- .../projects/pipelines_settings_controller_spec.rb | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/spec/controllers/projects/pipelines_settings_controller_spec.rb b/spec/controllers/projects/pipelines_settings_controller_spec.rb index 0dd8575e2b7..913b9bd804a 100644 --- a/spec/controllers/projects/pipelines_settings_controller_spec.rb +++ b/spec/controllers/projects/pipelines_settings_controller_spec.rb @@ -48,10 +48,6 @@ describe Projects::PipelinesSettingsController do end context 'when the project repository is empty' do - before do - allow_any_instance_of(Project).to receive(:empty_repo?).and_return(true) - end - it 'sets a warning flash' do expect(subject).to set_flash[:warning] end @@ -64,9 +60,7 @@ describe Projects::PipelinesSettingsController do end context 'when the project repository is not empty' do - before do - allow_any_instance_of(Project).to receive(:empty_repo?).and_return(false) - end + let(:project) { create(:project, :repository) } it 'sets a success flash' do allow(CreatePipelineWorker).to receive(:perform_async).with(project.id, user.id, project.default_branch, :web, any_args)