From 52c06647b58a8c78c29a79a5647962c065d9ef86 Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Gray Date: Tue, 1 Nov 2016 15:38:44 -0500 Subject: [PATCH 1/9] Start adding environment info --- app/assets/stylesheets/pages/builds.scss | 7 +++++++ app/views/projects/builds/show.html.haml | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/app/assets/stylesheets/pages/builds.scss b/app/assets/stylesheets/pages/builds.scss index f1d311cabbe..ebbec89c58c 100644 --- a/app/assets/stylesheets/pages/builds.scss +++ b/app/assets/stylesheets/pages/builds.scss @@ -40,6 +40,13 @@ margin-bottom: 10px; } } + + .environment-information { + background-color: $background-color; + border: 1px solid $border-color; + padding: 12px $gl-padding; + border-radius: $border-radius-default; + } } .build-header { diff --git a/app/views/projects/builds/show.html.haml b/app/views/projects/builds/show.html.haml index ae7a7ecb392..4614d0af906 100644 --- a/app/views/projects/builds/show.html.haml +++ b/app/views/projects/builds/show.html.haml @@ -26,6 +26,13 @@ = link_to namespace_project_runners_path(@build.project.namespace, @build.project) do Runners page + - if @build.stage == 'deploy' + .prepend-top-default + .environment-information + = ci_icon_for_status(@build.status) + This build is the most recent deployment to + = link_to @build.environment, namespace_project_environment_path(@project.namespace, @project, @build.environment) + .prepend-top-default - if @build.erased? .erased.alert.alert-warning From 73467bd1634898632d1c9c4e5879546ec9f53032 Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Gray Date: Wed, 2 Nov 2016 17:25:19 -0500 Subject: [PATCH 2/9] Add switch statement & last_deployment method --- app/assets/stylesheets/pages/builds.scss | 6 ++++++ app/models/ci/build.rb | 6 ++++++ app/views/projects/builds/show.html.haml | 22 +++++++++++++++++++--- 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/app/assets/stylesheets/pages/builds.scss b/app/assets/stylesheets/pages/builds.scss index ebbec89c58c..dc694d67199 100644 --- a/app/assets/stylesheets/pages/builds.scss +++ b/app/assets/stylesheets/pages/builds.scss @@ -46,6 +46,12 @@ border: 1px solid $border-color; padding: 12px $gl-padding; border-radius: $border-radius-default; + + svg { + position: relative; + top: 1px; + margin-right: 5px; + } } } diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index bf5f92f8462..1b37c70ee4f 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -125,6 +125,12 @@ module Ci !self.pipeline.statuses.latest.include?(self) end + def last_deployment + return @last_deployment if defined?(@last_deployment) + + @last_deployment = Deployment.where(deployable: self).order(id: :desc).last + end + def depends_on_builds # Get builds of the same type latest_builds = self.pipeline.builds.latest diff --git a/app/views/projects/builds/show.html.haml b/app/views/projects/builds/show.html.haml index 4614d0af906..6741be25ace 100644 --- a/app/views/projects/builds/show.html.haml +++ b/app/views/projects/builds/show.html.haml @@ -26,12 +26,28 @@ = link_to namespace_project_runners_path(@build.project.namespace, @build.project) do Runners page - - if @build.stage == 'deploy' + - if @build.deploy .prepend-top-default .environment-information = ci_icon_for_status(@build.status) - This build is the most recent deployment to - = link_to @build.environment, namespace_project_environment_path(@project.namespace, @project, @build.environment) + + - if @build.last_deployment + This build is the most recent deployment to + = link_to @build.environment, namespace_project_environment_path(@project.namespace, @project, @build.environment) + - else + - case @build.status + - when 'failed', 'canceled' + The deployment of this build to + = link_to @build.environment, namespace_project_environment_path(@project.namespace, @project, @build.environment) + did not complete + - when 'pending', 'running' + This build is creating a deployment to + = link_to @build.environment, namespace_project_environment_path(@project.namespace, @project, @build.environment) + and will overwrite the latest deployment + - when 'success' + This build is an out-of-date deployment to + = link_to @build.environment, namespace_project_environment_path(@project.namespace, @project, @build.environment) + View the most recent deployment #24869 .prepend-top-default - if @build.erased? From c70acb57f5adcd4f0a19f8e7d5bcb356464a9c64 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 4 Nov 2016 16:35:22 +0100 Subject: [PATCH 3/9] Expose `last_deployment` on `Ci::Builds` [ci skip] --- app/helpers/environment_helper.rb | 21 +++++++++++++++ app/models/ci/build.rb | 10 ++++--- app/views/projects/builds/show.html.haml | 34 +++++++++++++----------- app/workers/build_success_worker.rb | 4 +-- 4 files changed, 47 insertions(+), 22 deletions(-) create mode 100644 app/helpers/environment_helper.rb diff --git a/app/helpers/environment_helper.rb b/app/helpers/environment_helper.rb new file mode 100644 index 00000000000..1e2a8c7ddbd --- /dev/null +++ b/app/helpers/environment_helper.rb @@ -0,0 +1,21 @@ +module EnvironmentHelper + def environment_for_build(project, build) + return unless build.environment + + environment_name = ExpandVariables.expand(build.environment, build.variables) + project.environments.find_by(name: environment_name) + end + + def environment_link_for_build(project, build) + environment = environment_for_build(project, build) + return unless environment + + link_to environment.name, namespace_project_environment_path(project.namespace, project, environment) + end + + def deployment_link(project, deployment) + return unless deployment + + link_to "##{deployment.id}", [deployment.project.namespace.becomes(Namespace), deployment.project, deployment.deployable] + end +end diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 1b37c70ee4f..09bbea1c653 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -7,6 +7,8 @@ module Ci belongs_to :trigger_request belongs_to :erased_by, class_name: 'User' + has_many :deployments, as: :deployable + serialize :options serialize :yaml_variables @@ -125,10 +127,12 @@ module Ci !self.pipeline.statuses.latest.include?(self) end - def last_deployment - return @last_deployment if defined?(@last_deployment) + def deployable? + self.environment.present? + end - @last_deployment = Deployment.where(deployable: self).order(id: :desc).last + def last_deployment + deployments.order(id: :desc).last end def depends_on_builds diff --git a/app/views/projects/builds/show.html.haml b/app/views/projects/builds/show.html.haml index 6741be25ace..18a336c3fa3 100644 --- a/app/views/projects/builds/show.html.haml +++ b/app/views/projects/builds/show.html.haml @@ -26,28 +26,30 @@ = link_to namespace_project_runners_path(@build.project.namespace, @build.project) do Runners page - - if @build.deploy + - if @build.deployable? .prepend-top-default .environment-information = ci_icon_for_status(@build.status) - - if @build.last_deployment - This build is the most recent deployment to - = link_to @build.environment, namespace_project_environment_path(@project.namespace, @project, @build.environment) - - else - - case @build.status - - when 'failed', 'canceled' + - last_deployment = @build.last_deployment + - if @build.complete? + - if @build.success? + - if last_deployment.try(:last?) + This build is the most recent deployment to + = environment_link_for_build(@build.project, @build) + - else + This build is an out-of-date deployment to + = environment_link_for_build(@build.project, @build) + View the most recent deployment + = deployment_link(@project, last_deployment) + - else The deployment of this build to - = link_to @build.environment, namespace_project_environment_path(@project.namespace, @project, @build.environment) + = environment_link_for_build(@build.project, @build) did not complete - - when 'pending', 'running' - This build is creating a deployment to - = link_to @build.environment, namespace_project_environment_path(@project.namespace, @project, @build.environment) - and will overwrite the latest deployment - - when 'success' - This build is an out-of-date deployment to - = link_to @build.environment, namespace_project_environment_path(@project.namespace, @project, @build.environment) - View the most recent deployment #24869 + - else + This build is creating a deployment to + = environment_link_for_build(@build.project, @build) + and will overwrite the latest deployment .prepend-top-default - if @build.erased? diff --git a/app/workers/build_success_worker.rb b/app/workers/build_success_worker.rb index e0ad5268664..1bc9745ecbc 100644 --- a/app/workers/build_success_worker.rb +++ b/app/workers/build_success_worker.rb @@ -4,15 +4,13 @@ class BuildSuccessWorker def perform(build_id) Ci::Build.find_by(id: build_id).try do |build| - create_deployment(build) + create_deployment(build) if build.deployable? end end private def create_deployment(build) - return if build.environment.blank? - service = CreateDeploymentService.new( build.project, build.user, environment: build.environment, From 1a5a3be84080808554568a8c61a80cc6f3f536ed Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Gray Date: Tue, 8 Nov 2016 16:29:55 -0600 Subject: [PATCH 4/9] First pass at tests --- app/views/projects/builds/show.html.haml | 15 +++--- .../projects/builds/show.html.haml_spec.rb | 50 +++++++++++++++++++ 2 files changed, 58 insertions(+), 7 deletions(-) diff --git a/app/views/projects/builds/show.html.haml b/app/views/projects/builds/show.html.haml index 18a336c3fa3..43c6437f9b1 100644 --- a/app/views/projects/builds/show.html.haml +++ b/app/views/projects/builds/show.html.haml @@ -29,7 +29,10 @@ - if @build.deployable? .prepend-top-default .environment-information - = ci_icon_for_status(@build.status) + - if @build.status == 'success' && (!@build.last_deployment.try(:last?)) + = ci_icon_for_status('success_with_warnings') + - else + = ci_icon_for_status(@build.status) - last_deployment = @build.last_deployment - if @build.complete? @@ -38,18 +41,16 @@ This build is the most recent deployment to = environment_link_for_build(@build.project, @build) - else - This build is an out-of-date deployment to - = environment_link_for_build(@build.project, @build) - View the most recent deployment - = deployment_link(@project, last_deployment) + This build is an out-of-date deployment to #{environment_link_for_build(@build.project, @build)}. View the most recent deployment #{deployment_link(@project, last_deployment)}. - else The deployment of this build to = environment_link_for_build(@build.project, @build) - did not complete + did not complete. - else This build is creating a deployment to = environment_link_for_build(@build.project, @build) - and will overwrite the latest deployment + and will overwrite the + = link_to "latest deployment.", deployment_link(@project, last_deployment) .prepend-top-default - if @build.erased? diff --git a/spec/views/projects/builds/show.html.haml_spec.rb b/spec/views/projects/builds/show.html.haml_spec.rb index da43622d3f9..0e702d80bd3 100644 --- a/spec/views/projects/builds/show.html.haml_spec.rb +++ b/spec/views/projects/builds/show.html.haml_spec.rb @@ -17,6 +17,56 @@ describe 'projects/builds/show' do allow(view).to receive(:can?).and_return(true) end + describe 'environment info in build view' do + context 'build with latest deployment' do + let(:build) { create(:ci_build, :success, environment: 'staging') } + let(:environment) { create(:environment, name: 'staging') } + let!(:deployment) { create(:deployment, deployable: build) } + + it 'shows deployment message' do + expect(rendered).to have_css('.environment-information', text: 'This build is the most recent deployment') + end + end + + context 'build with outdated deployment' do + let(:build) { create(:ci_build, :success, environment: 'staging', pipeline: pipeline) } + let(:environment) { create(:environment, name: 'staging', project: project) } + let!(:deployment) { create(:deployment, environment: environment, deployable: build) } + let!(:newer_deployment) { create(:deployment, environment: environment, deployable: build) } + + before do + assign(:build, build) + assign(:project, project) + + allow(view).to receive(:can?).and_return(true) + render + end + + it 'shows deployment message' do + expect(rendered).to have_css('.environment-information', text: "This build is an out-of-date deployment to #{environment.name}. View the most recent deployment #1") + end + end + + context 'build failed to deploy' do + let(:build) { create(:ci_build, :failed, environment: 'staging') } + let!(:environment) { create(:environment, name: 'staging') } + end + + context 'build will deploy' do + let(:build) { create(:ci_build, :running, environment: 'staging') } + let!(:environment) { create(:environment, name: 'staging') } + end + + context 'build that failed to deploy and environment has not been created' do + let(:build) { create(:ci_build, :failed, environment: 'staging') } + end + + context 'build that will deploy and environment has not been created' do + let(:build) { create(:ci_build, :running, environment: 'staging') } + let!(:environment) { create(:environment, name: 'staging') } + end + end + context 'when build is running' do before do build.run! From d7ba85c7496fb24625f3ebf3e78af42ec23e842e Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 9 Nov 2016 19:40:25 +0100 Subject: [PATCH 5/9] Refine specs for build show page with environments --- spec/spec_helper.rb | 7 ++- .../projects/builds/show.html.haml_spec.rb | 57 ++++++++++++------- 2 files changed, 40 insertions(+), 24 deletions(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 73cf4c9a24c..bead1a006d1 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -26,10 +26,11 @@ RSpec.configure do |config| config.verbose_retry = true config.display_try_failure_messages = true - config.include Devise::Test::ControllerHelpers, type: :controller + config.include Devise::Test::ControllerHelpers, type: :controller + config.include Devise::Test::ControllerHelpers, type: :view config.include Warden::Test::Helpers, type: :request - config.include LoginHelpers, type: :feature - config.include SearchHelpers, type: :feature + config.include LoginHelpers, type: :feature + config.include SearchHelpers, type: :feature config.include StubConfiguration config.include EmailHelpers config.include TestEnv diff --git a/spec/views/projects/builds/show.html.haml_spec.rb b/spec/views/projects/builds/show.html.haml_spec.rb index 0e702d80bd3..98b68e730e6 100644 --- a/spec/views/projects/builds/show.html.haml_spec.rb +++ b/spec/views/projects/builds/show.html.haml_spec.rb @@ -1,15 +1,13 @@ require 'spec_helper' -describe 'projects/builds/show' do - include Devise::Test::ControllerHelpers - +describe 'projects/builds/show', :view do let(:project) { create(:project) } - let(:pipeline) do - create(:ci_pipeline, project: project, - sha: project.commit.id) - end let(:build) { create(:ci_build, pipeline: pipeline) } + let(:pipeline) do + create(:ci_pipeline, project: project, sha: project.commit.id) + end + before do assign(:build, build) assign(:project, project) @@ -19,31 +17,48 @@ describe 'projects/builds/show' do describe 'environment info in build view' do context 'build with latest deployment' do - let(:build) { create(:ci_build, :success, environment: 'staging') } - let(:environment) { create(:environment, name: 'staging') } - let!(:deployment) { create(:deployment, deployable: build) } + let(:build) do + create(:ci_build, :success, environment: 'staging') + end + + before do + create(:environment, name: 'staging') + create(:deployment, deployable: build) + end it 'shows deployment message' do - expect(rendered).to have_css('.environment-information', text: 'This build is the most recent deployment') + expected_text = 'This build is the most recent deployment' + + render + + expect(rendered).to have_css( + '.environment-information', text: expected_text) end end context 'build with outdated deployment' do - let(:build) { create(:ci_build, :success, environment: 'staging', pipeline: pipeline) } - let(:environment) { create(:environment, name: 'staging', project: project) } - let!(:deployment) { create(:deployment, environment: environment, deployable: build) } - let!(:newer_deployment) { create(:deployment, environment: environment, deployable: build) } + let(:build) do + create(:ci_build, :success, environment: 'staging', pipeline: pipeline) + end - before do - assign(:build, build) - assign(:project, project) + let(:environment) do + create(:environment, name: 'staging', project: project) + end - allow(view).to receive(:can?).and_return(true) - render + let!(:first_deployment) do + create(:deployment, environment: environment, deployable: build) + end + + let!(:second_deployment) do + create(:deployment, environment: environment, deployable: build) end it 'shows deployment message' do - expect(rendered).to have_css('.environment-information', text: "This build is an out-of-date deployment to #{environment.name}. View the most recent deployment #1") + expected_text = 'This build is an out-of-date deployment ' \ + "to staging. View the most recent deployment ##{first_deployment.id}" + render + + expect(rendered).to have_css('.environment-information', text: expected_text) end end From 2b8292cd49dbc68b02f46f865b7115191bf2de07 Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Gray Date: Wed, 9 Nov 2016 15:30:58 -0600 Subject: [PATCH 6/9] Finish specs for environment info --- app/views/projects/builds/show.html.haml | 13 ++-- .../projects/builds/show.html.haml_spec.rb | 68 ++++++++++++++++--- 2 files changed, 64 insertions(+), 17 deletions(-) diff --git a/app/views/projects/builds/show.html.haml b/app/views/projects/builds/show.html.haml index 43c6437f9b1..488688a7d59 100644 --- a/app/views/projects/builds/show.html.haml +++ b/app/views/projects/builds/show.html.haml @@ -38,19 +38,14 @@ - if @build.complete? - if @build.success? - if last_deployment.try(:last?) - This build is the most recent deployment to - = environment_link_for_build(@build.project, @build) + This build is the most recent deployment to #{environment_link_for_build(@build.project, @build)}. - else This build is an out-of-date deployment to #{environment_link_for_build(@build.project, @build)}. View the most recent deployment #{deployment_link(@project, last_deployment)}. - else - The deployment of this build to - = environment_link_for_build(@build.project, @build) - did not complete. + The deployment of this build to #{environment_link_for_build(@build.project, @build)} did not complete. - else - This build is creating a deployment to - = environment_link_for_build(@build.project, @build) - and will overwrite the - = link_to "latest deployment.", deployment_link(@project, last_deployment) + This build is creating a deployment to #{environment_link_for_build(@build.project, @build)} and will overwrite the | + = link_to "latest deployment.", deployment_link(@project, last_deployment) | .prepend-top-default - if @build.erased? diff --git a/spec/views/projects/builds/show.html.haml_spec.rb b/spec/views/projects/builds/show.html.haml_spec.rb index 98b68e730e6..fa4d66bb6cb 100644 --- a/spec/views/projects/builds/show.html.haml_spec.rb +++ b/spec/views/projects/builds/show.html.haml_spec.rb @@ -28,7 +28,6 @@ describe 'projects/builds/show', :view do it 'shows deployment message' do expected_text = 'This build is the most recent deployment' - render expect(rendered).to have_css( @@ -63,22 +62,75 @@ describe 'projects/builds/show', :view do end context 'build failed to deploy' do - let(:build) { create(:ci_build, :failed, environment: 'staging') } - let!(:environment) { create(:environment, name: 'staging') } + let(:build) do + create(:ci_build, :failed, environment: 'staging', pipeline: pipeline) + end + + let!(:environment) do + create(:environment, name: 'staging', project: project) + end + + it 'shows deployment message' do + expected_text = 'The deployment of this build to staging did not complete.' + render + + expect(rendered).to have_css( + '.environment-information', text: expected_text) + end end context 'build will deploy' do - let(:build) { create(:ci_build, :running, environment: 'staging') } - let!(:environment) { create(:environment, name: 'staging') } + let(:build) do + create(:ci_build, :running, environment: 'staging', pipeline: pipeline) + end + + let!(:environment) do + create(:environment, name: 'staging', project: project) + end + + it 'shows deployment message' do + expected_text = 'This build is creating a deployment to staging' + render + + expect(rendered).to have_css( + '.environment-information', text: expected_text) + end end context 'build that failed to deploy and environment has not been created' do - let(:build) { create(:ci_build, :failed, environment: 'staging') } + let(:build) do + create(:ci_build, :failed, environment: 'staging', pipeline: pipeline) + end + + let!(:environment) do + create(:environment, name: 'staging', project: project) + end + + it 'shows deployment message' do + expected_text = 'The deployment of this build to staging did not complete' + render + + expect(rendered).to have_css( + '.environment-information', text: expected_text) + end end context 'build that will deploy and environment has not been created' do - let(:build) { create(:ci_build, :running, environment: 'staging') } - let!(:environment) { create(:environment, name: 'staging') } + let(:build) do + create(:ci_build, :running, environment: 'staging', pipeline: pipeline) + end + + let!(:environment) do + create(:environment, name: 'staging', project: project) + end + + it 'shows deployment message' do + expected_text = 'This build is creating a deployment to staging' + render + + expect(rendered).to have_css( + '.environment-information', text: expected_text) + end end end From d6e00f5373e24deaa7f143f5445ae9461ef5f615 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 17 Nov 2016 00:23:05 +0100 Subject: [PATCH 7/9] Improve specs and add missing cases that were not supported --- app/helpers/environment_helper.rb | 20 ++-- app/models/ci/build.rb | 20 +++- app/views/projects/builds/show.html.haml | 27 +++--- app/workers/build_success_worker.rb | 2 +- spec/models/build_spec.rb | 96 +++++++++++++++++++ spec/models/ci/build_spec.rb | 8 ++ .../projects/builds/show.html.haml_spec.rb | 2 +- 7 files changed, 153 insertions(+), 22 deletions(-) diff --git a/app/helpers/environment_helper.rb b/app/helpers/environment_helper.rb index 1e2a8c7ddbd..ea34bce9367 100644 --- a/app/helpers/environment_helper.rb +++ b/app/helpers/environment_helper.rb @@ -2,20 +2,28 @@ module EnvironmentHelper def environment_for_build(project, build) return unless build.environment - environment_name = ExpandVariables.expand(build.environment, build.variables) - project.environments.find_by(name: environment_name) + project.environments.find_by(name: build.expanded_environment_name) end def environment_link_for_build(project, build) environment = environment_for_build(project, build) - return unless environment - - link_to environment.name, namespace_project_environment_path(project.namespace, project, environment) + if environment + link_to environment.name, namespace_project_environment_path(project.namespace, project, environment) + else + content_tag :span, build.expanded_environment_name + end end - def deployment_link(project, deployment) + def deployment_link(deployment) return unless deployment link_to "##{deployment.id}", [deployment.project.namespace.becomes(Namespace), deployment.project, deployment.deployable] end + + def last_deployment_link_for_environment_build(project, build) + environment = environment_for_build(project, build) + return unless environment + + deployment_link(environment.last_deployment) + end end diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 09bbea1c653..e0e7a8caeea 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -127,8 +127,24 @@ module Ci !self.pipeline.statuses.latest.include?(self) end - def deployable? - self.environment.present? + def expanded_environment_name + ExpandVariables.expand(environment, variables) if environment + end + + def starts_environment? + self.environment.present? && self.environment_action == 'start' + end + + def stops_environment? + self.environment.present? && self.environment_action == 'stop' + end + + def environment_action + self.options.fetch(:environment, {}).fetch(:action, 'start') + end + + def outdated_deployment? + success? && !last_deployment.try(:last?) end def last_deployment diff --git a/app/views/projects/builds/show.html.haml b/app/views/projects/builds/show.html.haml index 488688a7d59..d799671058d 100644 --- a/app/views/projects/builds/show.html.haml +++ b/app/views/projects/builds/show.html.haml @@ -26,26 +26,29 @@ = link_to namespace_project_runners_path(@build.project.namespace, @build.project) do Runners page - - if @build.deployable? + - if @build.environment.present? && @build.starts_environment? .prepend-top-default .environment-information - - if @build.status == 'success' && (!@build.last_deployment.try(:last?)) + - if @build.outdated_deployment? = ci_icon_for_status('success_with_warnings') - else = ci_icon_for_status(@build.status) - - last_deployment = @build.last_deployment - - if @build.complete? - - if @build.success? - - if last_deployment.try(:last?) - This build is the most recent deployment to #{environment_link_for_build(@build.project, @build)}. - - else - This build is an out-of-date deployment to #{environment_link_for_build(@build.project, @build)}. View the most recent deployment #{deployment_link(@project, last_deployment)}. + - environment = environment_for_build(@build.project, @build) + - if @build.success? && @build.last_deployment.present? + - if @build.last_deployment.last? + This build is the most recent deployment to #{environment_link_for_build(@build.project, @build)}. - else - The deployment of this build to #{environment_link_for_build(@build.project, @build)} did not complete. + This build is an out-of-date deployment to #{environment_link_for_build(@build.project, @build)}. + - if environment.last_deployment + View the most recent deployment #{deployment_link(environment.last_deployment)}. + - elsif @build.complete? && !@build.success? + The deployment of this build to #{environment_link_for_build(@build.project, @build)} did not complete. - else - This build is creating a deployment to #{environment_link_for_build(@build.project, @build)} and will overwrite the | - = link_to "latest deployment.", deployment_link(@project, last_deployment) | + This build is creating a deployment to #{environment_link_for_build(@build.project, @build)} + - if environment.last_deployment + and will overwrite the + = link_to 'latest deployment', deployment_link(environment.last_deployment) .prepend-top-default - if @build.erased? diff --git a/app/workers/build_success_worker.rb b/app/workers/build_success_worker.rb index 1bc9745ecbc..450ba871c4c 100644 --- a/app/workers/build_success_worker.rb +++ b/app/workers/build_success_worker.rb @@ -4,7 +4,7 @@ class BuildSuccessWorker def perform(build_id) Ci::Build.find_by(id: build_id).try do |build| - create_deployment(build) if build.deployable? + create_deployment(build) if build.environment.present? end end diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index ae185de9ca3..731a2274d9e 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -1052,4 +1052,100 @@ describe Ci::Build, models: true do end end end + + describe '#starts_environment?' do + subject { build.starts_environment? } + + context 'when environment is defined' do + before do + build.update(environment: 'review') + end + + it { is_expected.to be_truthy } + end + + context 'when environment is not defined' do + before do + build.update(environment: nil) + end + + it { is_expected.to be_falsey } + end + end + + describe '#stops_environment?' do + subject { build.stops_environment? } + + context 'when environment is defined' do + before do + build.update(environment: 'review') + end + + context 'no action is defined' do + it { is_expected.to be_falsey } + end + + context 'and stop action is defined' do + before do + build.update(options: { environment: { action: 'stop' } } ) + end + + it { is_expected.to be_truthy } + end + end + + context 'when environment is not defined' do + before do + build.update(environment: nil) + end + + it { is_expected.to be_falsey } + end + end + + describe '#last_deployment' do + subject { build.last_deployment } + + context 'when multiple deployments is created returns latest one' do + let!(:deployment1) { create(:deployment, deployable: build) } + let!(:deployment2) { create(:deployment, deployable: build) } + + it { is_expected.to eq(deployment2) } + end + end + + describe '#outdated_deployment?' do + subject { build.outdated_deployment? } + + context 'when build succeeded' do + let(:build) { create(:ci_build, :success) } + let!(:deployment) { create(:deployment, deployable: build) } + + context 'current deployment is latest' do + it { is_expected.to be_falsey } + end + + context 'current deployment is not latest on environment' do + let!(:deployment2) { create(:deployment, environment: deployment.environment) } + + it { is_expected.to be_truthy } + end + end + + context 'when build failed' do + let(:build) { create(:ci_build, :failed) } + + it { is_expected.to be_falsey } + end + end + + describe '#expanded_environment_name' do + subject { build.expanded_environment_name } + + context 'when environment uses variables' do + let(:build) { create(:ci_build, ref: 'master', environment: 'review/$CI_BUILD_REF_NAME') } + + it { is_expected.to eq('review/master') } + end + end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index a37a00f461a..2efe69d7adc 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -4,6 +4,14 @@ describe Ci::Build, models: true do let(:build) { create(:ci_build) } let(:test_trace) { 'This is a test' } + describe 'ss' do + it { is_expected.to belong_to(:runner) } + it { is_expected.to belong_to(:trigger_request) } + it { is_expected.to belong_to(:erased_by) } + + it { is_expected.to have_many(:deployments) } + end + describe '#trace' do it 'obfuscates project runners token' do allow(build).to receive(:raw_trace).and_return("Test: #{build.project.runners_token}") diff --git a/spec/views/projects/builds/show.html.haml_spec.rb b/spec/views/projects/builds/show.html.haml_spec.rb index fa4d66bb6cb..3b9a9c95daa 100644 --- a/spec/views/projects/builds/show.html.haml_spec.rb +++ b/spec/views/projects/builds/show.html.haml_spec.rb @@ -54,7 +54,7 @@ describe 'projects/builds/show', :view do it 'shows deployment message' do expected_text = 'This build is an out-of-date deployment ' \ - "to staging. View the most recent deployment ##{first_deployment.id}" + "to staging.\nView the most recent deployment ##{second_deployment.id}." render expect(rendered).to have_css('.environment-information', text: expected_text) From 43906336ff24e218cb1f7024d63a22367dd7e09a Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 17 Nov 2016 12:08:28 +0100 Subject: [PATCH 8/9] Fix tests and add has_environment? --- app/models/ci/build.rb | 10 +++++-- app/views/projects/builds/show.html.haml | 2 +- app/workers/build_success_worker.rb | 2 +- spec/models/build_spec.rb | 38 ++++++++++++++++++++++-- spec/models/ci/build_spec.rb | 4 +-- 5 files changed, 45 insertions(+), 11 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index e0e7a8caeea..7aa6d0a6bf2 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -131,12 +131,16 @@ module Ci ExpandVariables.expand(environment, variables) if environment end + def has_environment? + self.environment.present? + end + def starts_environment? - self.environment.present? && self.environment_action == 'start' + has_environment? && self.environment_action == 'start' end def stops_environment? - self.environment.present? && self.environment_action == 'stop' + has_environment? && self.environment_action == 'stop' end def environment_action @@ -148,7 +152,7 @@ module Ci end def last_deployment - deployments.order(id: :desc).last + deployments.last end def depends_on_builds diff --git a/app/views/projects/builds/show.html.haml b/app/views/projects/builds/show.html.haml index d799671058d..efb130a5dd8 100644 --- a/app/views/projects/builds/show.html.haml +++ b/app/views/projects/builds/show.html.haml @@ -26,7 +26,7 @@ = link_to namespace_project_runners_path(@build.project.namespace, @build.project) do Runners page - - if @build.environment.present? && @build.starts_environment? + - if @build.starts_environment? .prepend-top-default .environment-information - if @build.outdated_deployment? diff --git a/app/workers/build_success_worker.rb b/app/workers/build_success_worker.rb index 450ba871c4c..e17add7421f 100644 --- a/app/workers/build_success_worker.rb +++ b/app/workers/build_success_worker.rb @@ -4,7 +4,7 @@ class BuildSuccessWorker def perform(build_id) Ci::Build.find_by(id: build_id).try do |build| - create_deployment(build) if build.environment.present? + create_deployment(build) if build.has_environment? end end diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index 731a2274d9e..ef07f2275b1 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -1053,6 +1053,26 @@ describe Ci::Build, models: true do end end + describe '#has_environment?' do + subject { build.has_environment? } + + context 'when environment is defined' do + before do + build.update(environment: 'review') + end + + it { is_expected.to be_truthy } + end + + context 'when environment is not defined' do + before do + build.update(environment: nil) + end + + it { is_expected.to be_falsey } + end + end + describe '#starts_environment?' do subject { build.starts_environment? } @@ -1061,7 +1081,17 @@ describe Ci::Build, models: true do build.update(environment: 'review') end - it { is_expected.to be_truthy } + context 'no action is defined' do + it { is_expected.to be_truthy } + end + + context 'and start action is defined' do + before do + build.update(options: { environment: { action: 'start' } } ) + end + + it { is_expected.to be_truthy } + end end context 'when environment is not defined' do @@ -1106,11 +1136,13 @@ describe Ci::Build, models: true do describe '#last_deployment' do subject { build.last_deployment } - context 'when multiple deployments is created returns latest one' do + context 'when multiple deployments are created' do let!(:deployment1) { create(:deployment, deployable: build) } let!(:deployment2) { create(:deployment, deployable: build) } - it { is_expected.to eq(deployment2) } + it 'returns the latest one' do + is_expected.to eq(deployment2) + end end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 2efe69d7adc..a7e90c8a381 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -4,14 +4,12 @@ describe Ci::Build, models: true do let(:build) { create(:ci_build) } let(:test_trace) { 'This is a test' } - describe 'ss' do it { is_expected.to belong_to(:runner) } it { is_expected.to belong_to(:trigger_request) } it { is_expected.to belong_to(:erased_by) } it { is_expected.to have_many(:deployments) } - end - + describe '#trace' do it 'obfuscates project runners token' do allow(build).to receive(:raw_trace).and_return("Test: #{build.project.runners_token}") From 9ed7171a6a8c98858949891b298789a97c4f3fba Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Gray Date: Thu, 17 Nov 2016 13:33:53 -0600 Subject: [PATCH 9/9] Fix builds/show spec; use iid instead of id --- app/helpers/environment_helper.rb | 2 +- app/views/projects/builds/show.html.haml | 2 +- spec/views/projects/builds/show.html.haml_spec.rb | 12 ++++++++---- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/app/helpers/environment_helper.rb b/app/helpers/environment_helper.rb index ea34bce9367..27975b7ddb7 100644 --- a/app/helpers/environment_helper.rb +++ b/app/helpers/environment_helper.rb @@ -17,7 +17,7 @@ module EnvironmentHelper def deployment_link(deployment) return unless deployment - link_to "##{deployment.id}", [deployment.project.namespace.becomes(Namespace), deployment.project, deployment.deployable] + link_to "##{deployment.iid}", [deployment.project.namespace.becomes(Namespace), deployment.project, deployment.deployable] end def last_deployment_link_for_environment_build(project, build) diff --git a/app/views/projects/builds/show.html.haml b/app/views/projects/builds/show.html.haml index efb130a5dd8..0eaa602dc71 100644 --- a/app/views/projects/builds/show.html.haml +++ b/app/views/projects/builds/show.html.haml @@ -43,7 +43,7 @@ - if environment.last_deployment View the most recent deployment #{deployment_link(environment.last_deployment)}. - elsif @build.complete? && !@build.success? - The deployment of this build to #{environment_link_for_build(@build.project, @build)} did not complete. + The deployment of this build to #{environment_link_for_build(@build.project, @build)} did not succeed. - else This build is creating a deployment to #{environment_link_for_build(@build.project, @build)} - if environment.last_deployment diff --git a/spec/views/projects/builds/show.html.haml_spec.rb b/spec/views/projects/builds/show.html.haml_spec.rb index 3b9a9c95daa..e0c77201116 100644 --- a/spec/views/projects/builds/show.html.haml_spec.rb +++ b/spec/views/projects/builds/show.html.haml_spec.rb @@ -40,6 +40,10 @@ describe 'projects/builds/show', :view do create(:ci_build, :success, environment: 'staging', pipeline: pipeline) end + let(:second_build) do + create(:ci_build, :success, environment: 'staging', pipeline: pipeline) + end + let(:environment) do create(:environment, name: 'staging', project: project) end @@ -49,12 +53,12 @@ describe 'projects/builds/show', :view do end let!(:second_deployment) do - create(:deployment, environment: environment, deployable: build) + create(:deployment, environment: environment, deployable: second_build) end it 'shows deployment message' do expected_text = 'This build is an out-of-date deployment ' \ - "to staging.\nView the most recent deployment ##{second_deployment.id}." + "to staging.\nView the most recent deployment ##{second_deployment.iid}." render expect(rendered).to have_css('.environment-information', text: expected_text) @@ -71,7 +75,7 @@ describe 'projects/builds/show', :view do end it 'shows deployment message' do - expected_text = 'The deployment of this build to staging did not complete.' + expected_text = 'The deployment of this build to staging did not succeed.' render expect(rendered).to have_css( @@ -107,7 +111,7 @@ describe 'projects/builds/show', :view do end it 'shows deployment message' do - expected_text = 'The deployment of this build to staging did not complete' + expected_text = 'The deployment of this build to staging did not succeed' render expect(rendered).to have_css(