diff options
23 files changed, 252 insertions, 205 deletions
diff --git a/app/models/cycle_analytics.rb b/app/models/cycle_analytics.rb index 3ca160252ff..be295487fd2 100644 --- a/app/models/cycle_analytics.rb +++ b/app/models/cycle_analytics.rb @@ -5,55 +5,54 @@ class CycleAnalytics def initialize(project, from:) @project = project @from = from - @summary = Summary.new(project, from: from) end def summary - @summary + @summary ||= Summary.new(@project, from: @from) end def issue calculate_metric(:issue, - TableReferences.issues[:created_at], - [TableReferences.issue_metrics[:first_associated_with_milestone_at], - TableReferences.issue_metrics[:first_added_to_board_at]]) + Issue.arel_table[:created_at], + [Issue::Metrics.arel_table[:first_associated_with_milestone_at], + Issue::Metrics.arel_table[:first_added_to_board_at]]) end def plan calculate_metric(:plan, - [TableReferences.issue_metrics[:first_associated_with_milestone_at], - TableReferences.issue_metrics[:first_added_to_board_at]], - TableReferences.issue_metrics[:first_mentioned_in_commit_at]) + [Issue::Metrics.arel_table[:first_associated_with_milestone_at], + Issue::Metrics.arel_table[:first_added_to_board_at]], + Issue::Metrics.arel_table[:first_mentioned_in_commit_at]) end def code calculate_metric(:code, - TableReferences.issue_metrics[:first_mentioned_in_commit_at], - TableReferences.merge_requests[:created_at]) + Issue::Metrics.arel_table[:first_mentioned_in_commit_at], + MergeRequest.arel_table[:created_at]) end def test calculate_metric(:test, - TableReferences.merge_request_metrics[:latest_build_started_at], - TableReferences.merge_request_metrics[:latest_build_finished_at]) + MergeRequest::Metrics.arel_table[:latest_build_started_at], + MergeRequest::Metrics.arel_table[:latest_build_finished_at]) end def review calculate_metric(:review, - TableReferences.merge_requests[:created_at], - TableReferences.merge_request_metrics[:merged_at]) + MergeRequest.arel_table[:created_at], + MergeRequest::Metrics.arel_table[:merged_at]) end def staging calculate_metric(:staging, - TableReferences.merge_request_metrics[:merged_at], - TableReferences.merge_request_metrics[:first_deployed_to_production_at]) + MergeRequest::Metrics.arel_table[:merged_at], + MergeRequest::Metrics.arel_table[:first_deployed_to_production_at]) end def production calculate_metric(:production, - TableReferences.issues[:created_at], - TableReferences.merge_request_metrics[:first_deployed_to_production_at]) + Issue.arel_table[:created_at], + MergeRequest::Metrics.arel_table[:first_deployed_to_production_at]) end private @@ -69,9 +68,7 @@ class CycleAnalytics cte_table, subtract_datetimes(base_query, end_time_attrs, start_time_attrs, name.to_s)) - median_queries = Array.wrap(median_datetime(cte_table, interval_query, name)) - results = median_queries.map { |query| run_query(query) } - extract_median(results).presence + median_datetime(cte_table, interval_query, name) end # Join table with a row for every <issue,merge_request> pair (where the merge request @@ -79,27 +76,22 @@ class CycleAnalytics # are loaded with an inner join, so issues / merge requests without metrics are # automatically excluded. def base_query - arel_table = TableReferences.merge_requests_closing_issues + arel_table = MergeRequestsClosingIssues.arel_table # Load issues - query = arel_table.join(TableReferences.issues).on(TableReferences.issues[:id].eq(arel_table[:issue_id])). - join(TableReferences.issue_metrics).on(TableReferences.issues[:id].eq(TableReferences.issue_metrics[:issue_id])). - where(TableReferences.issues[:project_id].eq(@project.id)). - where(TableReferences.issues[:deleted_at].eq(nil)). - where(TableReferences.issues[:created_at].gteq(@from)) + query = arel_table.join(Issue.arel_table).on(Issue.arel_table[:id].eq(arel_table[:issue_id])). + join(Issue::Metrics.arel_table).on(Issue.arel_table[:id].eq(Issue::Metrics.arel_table[:issue_id])). + where(Issue.arel_table[:project_id].eq(@project.id)). + where(Issue.arel_table[:deleted_at].eq(nil)). + where(Issue.arel_table[:created_at].gteq(@from)) # Load merge_requests - query = query.join(TableReferences.merge_requests, Arel::Nodes::OuterJoin). - on(TableReferences.merge_requests[:id].eq(arel_table[:merge_request_id])). - join(TableReferences.merge_request_metrics). - on(TableReferences.merge_requests[:id].eq(TableReferences.merge_request_metrics[:merge_request_id])) + query = query.join(MergeRequest.arel_table, Arel::Nodes::OuterJoin). + on(MergeRequest.arel_table[:id].eq(arel_table[:merge_request_id])). + join(MergeRequest::Metrics.arel_table). + on(MergeRequest.arel_table[:id].eq(MergeRequest::Metrics.arel_table[:merge_request_id])) # Limit to merge requests that have been deployed to production after `@from` - query.where(TableReferences.merge_request_metrics[:first_deployed_to_production_at].gteq(@from)) - end - - def run_query(query) - query = query.to_sql unless query.is_a?(String) - ActiveRecord::Base.connection.execute(query) + query.where(MergeRequest::Metrics.arel_table[:first_deployed_to_production_at].gteq(@from)) end end diff --git a/app/models/cycle_analytics/summary.rb b/app/models/cycle_analytics/summary.rb index 04a90a8da49..53b2cacb131 100644 --- a/app/models/cycle_analytics/summary.rb +++ b/app/models/cycle_analytics/summary.rb @@ -6,7 +6,7 @@ class CycleAnalytics end def new_issues - @project.issues.where("created_at > ?", @from).count + @project.issues.created_after(@from).count end def commits diff --git a/app/models/cycle_analytics/table_references.rb b/app/models/cycle_analytics/table_references.rb deleted file mode 100644 index f276723ee0e..00000000000 --- a/app/models/cycle_analytics/table_references.rb +++ /dev/null @@ -1,25 +0,0 @@ -class CycleAnalytics - module TableReferences - class << self - def issues - Issue.arel_table - end - - def issue_metrics - Issue::Metrics.arel_table - end - - def merge_requests - MergeRequest.arel_table - end - - def merge_request_metrics - MergeRequest::Metrics.arel_table - end - - def merge_requests_closing_issues - MergeRequestsClosingIssues.arel_table - end - end - end -end diff --git a/app/models/deployment.rb b/app/models/deployment.rb index 15f349c89bb..07d7e19e70d 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -43,23 +43,30 @@ class Deployment < ActiveRecord::Base project.repository.is_ancestor?(commit.id, sha) end - def update_merge_request_metrics - if environment.update_merge_request_metrics? - merge_requests = project.merge_requests. - joins(:metrics). - where(target_branch: self.ref, merge_request_metrics: { first_deployed_to_production_at: nil }). - where("merge_request_metrics.merged_at <= ?", self.created_at) - - if previous_deployment - merge_requests = merge_requests.where("merge_request_metrics.merged_at >= ?", previous_deployment.created_at) - end + def update_merge_request_metrics! + return unless environment.update_merge_request_metrics? + + merge_requests = project.merge_requests. + joins(:metrics). + where(target_branch: self.ref, merge_request_metrics: { first_deployed_to_production_at: nil }). + where("merge_request_metrics.merged_at <= ?", self.created_at) - # Need to use `map` instead of `select` because MySQL doesn't allow `SELECT`ing from the same table - # that we're updating. - MergeRequest::Metrics. - where(merge_request_id: merge_requests.map(&:id), first_deployed_to_production_at: nil). - update_all(first_deployed_to_production_at: self.created_at) + if previous_deployment + merge_requests = merge_requests.where("merge_request_metrics.merged_at >= ?", previous_deployment.created_at) end + + # Need to use `map` instead of `select` because MySQL doesn't allow `SELECT`ing from the same table + # that we're updating. + merge_request_ids = + if Gitlab::Database.postgresql? + merge_requests.select(:id) + elsif Gitlab::Database.mysql? + merge_requests.map(&:id) + end + + MergeRequest::Metrics. + where(merge_request_id: merge_request_ids, first_deployed_to_production_at: nil). + update_all(first_deployed_to_production_at: self.created_at) end def previous_deployment diff --git a/app/models/issue.rb b/app/models/issue.rb index 6a264ace165..25ba38a1cff 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -24,7 +24,6 @@ class Issue < ActiveRecord::Base has_many :events, as: :target, dependent: :destroy has_many :merge_requests_closing_issues, class_name: 'MergeRequestsClosingIssues' - has_many :closed_by_merge_requests, through: :merge_requests_closing_issues, source: :merge_request validates :project, presence: true @@ -39,6 +38,8 @@ class Issue < ActiveRecord::Base scope :order_due_date_asc, -> { reorder('issues.due_date IS NULL, issues.due_date ASC') } scope :order_due_date_desc, -> { reorder('issues.due_date IS NULL, issues.due_date DESC') } + scope :created_after, -> (datetime) { where("created_at >= ?", datetime) } + attr_spammable :title, spam_title: true attr_spammable :description, spam_description: true diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 0ac291ce1a0..c05718f4d5a 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -17,7 +17,6 @@ class MergeRequest < ActiveRecord::Base has_many :events, as: :target, dependent: :destroy has_many :merge_requests_closing_issues, class_name: 'MergeRequestsClosingIssues' - has_many :issues_closed, through: :merge_requests_closing_issues, source: :issue serialize :merge_params, Hash @@ -524,11 +523,8 @@ class MergeRequest < ActiveRecord::Base # Return the set of issues that will be closed if this merge request is accepted. def closes_issues(current_user = self.author) if target_branch == project.default_branch - messages = if merge_request_diff - commits.map(&:safe_message) << description - else - [description] - end + messages = [description] + messages.concat(commits.map(&:safe_message)) if merge_request_diff Gitlab::ClosingIssueExtractor.new(project, current_user). closed_by_message(messages.join("\n")) diff --git a/app/models/merge_requests_closing_issues.rb b/app/models/merge_requests_closing_issues.rb index cd49076002d..ab597c37947 100644 --- a/app/models/merge_requests_closing_issues.rb +++ b/app/models/merge_requests_closing_issues.rb @@ -2,8 +2,6 @@ class MergeRequestsClosingIssues < ActiveRecord::Base belongs_to :merge_request belongs_to :issue - validates_uniqueness_of :merge_request_id, scope: :issue_id - - validates_presence_of :merge_request - validates_presence_of :issue + validates :merge_request_id, uniqueness: { scope: :issue_id }, presence: true + validates :issue_id, presence: true end diff --git a/app/services/create_deployment_service.rb b/app/services/create_deployment_service.rb index e958e91d612..799ad3e1bd0 100644 --- a/app/services/create_deployment_service.rb +++ b/app/services/create_deployment_service.rb @@ -13,7 +13,7 @@ class CreateDeploymentService < BaseService deployable: deployable ) - deployment.update_merge_request_metrics + deployment.update_merge_request_metrics! deployment end diff --git a/app/views/projects/pipelines/_head.html.haml b/app/views/projects/pipelines/_head.html.haml index fa1470f5fbc..a97674e84ba 100644 --- a/app/views/projects/pipelines/_head.html.haml +++ b/app/views/projects/pipelines/_head.html.haml @@ -20,7 +20,8 @@ %span Environments - = nav_link(controller: %w(cycle_analytics)) do - = link_to project_cycle_analytics_path(@project), title: 'Cycle Analytics' do - %span - Cycle Analytics + - if current_user + = nav_link(controller: %w(cycle_analytics)) do + = link_to project_cycle_analytics_path(@project), title: 'Cycle Analytics' do + %span + Cycle Analytics diff --git a/db/migrate/20160825052008_add_table_merge_request_metrics.rb b/db/migrate/20160825052008_add_table_merge_request_metrics.rb index bde9238b3c6..e01cc5038b9 100644 --- a/db/migrate/20160825052008_add_table_merge_request_metrics.rb +++ b/db/migrate/20160825052008_add_table_merge_request_metrics.rb @@ -29,7 +29,7 @@ class AddTableMergeRequestMetrics < ActiveRecord::Migration t.datetime 'latest_build_started_at' t.datetime 'latest_build_finished_at' - t.datetime 'first_deployed_to_production_at' + t.datetime 'first_deployed_to_production_at', index: true t.datetime 'merged_at' t.timestamps null: false diff --git a/lib/gitlab/database/median.rb b/lib/gitlab/database/median.rb index 94f060bd2c7..9adacd64934 100644 --- a/lib/gitlab/database/median.rb +++ b/lib/gitlab/database/median.rb @@ -3,11 +3,15 @@ module Gitlab module Database module Median def median_datetime(arel_table, query_so_far, column_sym) - if Gitlab::Database.postgresql? - pg_median_datetime(arel_table, query_so_far, column_sym) - elsif Gitlab::Database.mysql? - mysql_median_datetime(arel_table, query_so_far, column_sym) - end + median_queries = + if Gitlab::Database.postgresql? + pg_median_datetime_sql(arel_table, query_so_far, column_sym) + elsif Gitlab::Database.mysql? + mysql_median_datetime_sql(arel_table, query_so_far, column_sym) + end + + results = Array.wrap(median_queries).map { |query| Util.run_query(query) } + extract_median(results).presence end def extract_median(results) @@ -22,7 +26,7 @@ module Gitlab end end - def mysql_median_datetime(arel_table, query_so_far, column_sym) + def mysql_median_datetime_sql(arel_table, query_so_far, column_sym) query = arel_table. from(arel_table.project(Arel.sql('*')).order(arel_table[column_sym]).as(arel_table.table_name)). project(average([arel_table[column_sym]], 'median')). @@ -44,7 +48,7 @@ module Gitlab ] end - def pg_median_datetime(arel_table, query_so_far, column_sym) + def pg_median_datetime_sql(arel_table, query_so_far, column_sym) # Create a CTE with the column we're operating on, row number (after sorting by the column # we're operating on), and count of the table we're operating on (duplicated across) all rows # of the CTE. For example, if we're looking to find the median of the `projects.star_count` @@ -59,10 +63,11 @@ module Gitlab cte = Arel::Nodes::As.new( cte_table, arel_table. - project(arel_table[column_sym].as(column_sym.to_s), - Arel::Nodes::Over.new(Arel::Nodes::NamedFunction.new("row_number", []), - Arel::Nodes::Window.new.order(arel_table[column_sym])).as('row_id'), - arel_table.project("COUNT(1)").as('ct')). + project( + arel_table[column_sym].as(column_sym.to_s), + Arel::Nodes::Over.new(Arel::Nodes::NamedFunction.new("row_number", []), + Arel::Nodes::Window.new.order(arel_table[column_sym])).as('row_id'), + arel_table.project("COUNT(1)").as('ct')). # Disallow negative values where(arel_table[column_sym].gteq(zero_interval))) @@ -70,9 +75,11 @@ module Gitlab # by 'where cte.row_id between cte.ct / 2.0 AND cte.ct / 2.0 + 1'). Find the average of the # selected rows, and this is the median value. cte_table.project(average([extract_epoch(cte_table[column_sym])], "median")). - where(Arel::Nodes::Between.new(cte_table[:row_id], - Arel::Nodes::And.new([(cte_table[:ct] / Arel.sql('2.0')), - (cte_table[:ct] / Arel.sql('2.0') + 1)]))). + where(Arel::Nodes::Between.new( + cte_table[:row_id], + Arel::Nodes::And.new( + [(cte_table[:ct] / Arel.sql('2.0')), + (cte_table[:ct] / Arel.sql('2.0') + 1)]))). with(query_so_far, cte) end diff --git a/lib/gitlab/database/util.rb b/lib/gitlab/database/util.rb new file mode 100644 index 00000000000..12b68deae89 --- /dev/null +++ b/lib/gitlab/database/util.rb @@ -0,0 +1,12 @@ +module Gitlab + module Database + module Util + class << self + def run_query(query) + query = query.to_sql unless query.is_a?(String) + ActiveRecord::Base.connection.execute(query) + end + end + end + end +end diff --git a/spec/models/cycle_analytics/code_spec.rb b/spec/models/cycle_analytics/code_spec.rb index e81c78df8f0..b9381e33914 100644 --- a/spec/models/cycle_analytics/code_spec.rb +++ b/spec/models/cycle_analytics/code_spec.rb @@ -1,19 +1,28 @@ require 'spec_helper' describe 'CycleAnalytics#code', feature: true do + extend CycleAnalyticsHelpers::TestGeneration + let(:project) { create(:project) } let(:from_date) { 10.days.ago } let(:user) { create(:user, :admin) } subject { CycleAnalytics.new(project, from: from_date) } - generate_cycle_analytics_spec(phase: :code, - data_fn: -> (context) { { issue: context.create(:issue, project: context.project) } }, - start_time_conditions: [["issue mentioned in a commit", -> (context, data) { context.create_commit_referencing_issue(data[:issue]) }]], - end_time_conditions: [["merge request that closes issue is created", -> (context, data) { context.create_merge_request_closing_issue(data[:issue]) }]], - post_fn: -> (context, data) do - context.merge_merge_requests_closing_issue(data[:issue]) - context.deploy_master - end) + generate_cycle_analytics_spec( + phase: :code, + data_fn: -> (context) { { issue: context.create(:issue, project: context.project) } }, + start_time_conditions: [["issue mentioned in a commit", + -> (context, data) do + context.create_commit_referencing_issue(data[:issue]) + end]], + end_time_conditions: [["merge request that closes issue is created", + -> (context, data) do + context.create_merge_request_closing_issue(data[:issue]) + end]], + post_fn: -> (context, data) do + context.merge_merge_requests_closing_issue(data[:issue]) + context.deploy_master + end) context "when a regular merge request (that doesn't close the issue) is created" do it "returns nil" do diff --git a/spec/models/cycle_analytics/issue_spec.rb b/spec/models/cycle_analytics/issue_spec.rb index 8d7d03193f0..e9cc71254ab 100644 --- a/spec/models/cycle_analytics/issue_spec.rb +++ b/spec/models/cycle_analytics/issue_spec.rb @@ -1,23 +1,36 @@ require 'spec_helper' describe 'CycleAnalytics#issue', models: true do + extend CycleAnalyticsHelpers::TestGeneration + let(:project) { create(:project) } let(:from_date) { 10.days.ago } let(:user) { create(:user, :admin) } subject { CycleAnalytics.new(project, from: from_date) } - generate_cycle_analytics_spec(phase: :issue, - data_fn: -> (context) { { issue: context.build(:issue, project: context.project) } }, - start_time_conditions: [["issue created", -> (context, data) { data[:issue].save }]], - end_time_conditions: [["issue associated with a milestone", -> (context, data) { data[:issue].update(milestone: context.create(:milestone, project: context.project)) if data[:issue].persisted? }], - ["list label added to issue", -> (context, data) { data[:issue].update(label_ids: [context.create(:label, lists: [context.create(:list)]).id]) if data[:issue].persisted? }]], - post_fn: -> (context, data) do - if data[:issue].persisted? - context.create_merge_request_closing_issue(data[:issue].reload) - context.merge_merge_requests_closing_issue(data[:issue]) - context.deploy_master - end - end) + generate_cycle_analytics_spec( + phase: :issue, + data_fn: -> (context) { { issue: context.build(:issue, project: context.project) } }, + start_time_conditions: [["issue created", -> (context, data) { data[:issue].save }]], + end_time_conditions: [["issue associated with a milestone", + -> (context, data) do + if data[:issue].persisted? + data[:issue].update(milestone: context.create(:milestone, project: context.project)) + end + end], + ["list label added to issue", + -> (context, data) do + if data[:issue].persisted? + data[:issue].update(label_ids: [context.create(:label, lists: [context.create(:list)]).id]) + end + end]], + post_fn: -> (context, data) do + if data[:issue].persisted? + context.create_merge_request_closing_issue(data[:issue].reload) + context.merge_merge_requests_closing_issue(data[:issue]) + context.deploy_master + end + end) context "when a regular label (instead of a list label) is added to the issue" do it "returns nil" do diff --git a/spec/models/cycle_analytics/plan_spec.rb b/spec/models/cycle_analytics/plan_spec.rb index 4a66615e53b..5b8c96dc992 100644 --- a/spec/models/cycle_analytics/plan_spec.rb +++ b/spec/models/cycle_analytics/plan_spec.rb @@ -1,26 +1,38 @@ require 'spec_helper' describe 'CycleAnalytics#plan', feature: true do + extend CycleAnalyticsHelpers::TestGeneration + let(:project) { create(:project) } let(:from_date) { 10.days.ago } let(:user) { create(:user, :admin) } subject { CycleAnalytics.new(project, from: from_date) } - generate_cycle_analytics_spec(phase: :plan, - data_fn: -> (context) do - { - issue: context.create(:issue, project: context.project), - branch_name: context.random_git_name - } - end, - start_time_conditions: [["issue associated with a milestone", -> (context, data) { data[:issue].update(milestone: context.create(:milestone, project: context.project)) }], - ["list label added to issue", -> (context, data) { data[:issue].update(label_ids: [context.create(:label, lists: [context.create(:list)]).id]) }]], - end_time_conditions: [["issue mentioned in a commit", -> (context, data) { context.create_commit_referencing_issue(data[:issue], branch_name: data[:branch_name]) }]], - post_fn: -> (context, data) do - context.create_merge_request_closing_issue(data[:issue], source_branch: data[:branch_name]) - context.merge_merge_requests_closing_issue(data[:issue]) - context.deploy_master - end) + generate_cycle_analytics_spec( + phase: :plan, + data_fn: -> (context) do + { + issue: context.create(:issue, project: context.project), + branch_name: context.random_git_name + } + end, + start_time_conditions: [["issue associated with a milestone", + -> (context, data) do + data[:issue].update(milestone: context.create(:milestone, project: context.project)) + end], + ["list label added to issue", + -> (context, data) do + data[:issue].update(label_ids: [context.create(:label, lists: [context.create(:list)]).id]) + end]], + end_time_conditions: [["issue mentioned in a commit", + -> (context, data) do + context.create_commit_referencing_issue(data[:issue], branch_name: data[:branch_name]) + end]], + post_fn: -> (context, data) do + context.create_merge_request_closing_issue(data[:issue], source_branch: data[:branch_name]) + context.merge_merge_requests_closing_issue(data[:issue]) + context.deploy_master + end) context "when a regular label (instead of a list label) is added to the issue" do it "returns nil" do diff --git a/spec/models/cycle_analytics/production_spec.rb b/spec/models/cycle_analytics/production_spec.rb index 703f8d5f782..1f5e5cab92d 100644 --- a/spec/models/cycle_analytics/production_spec.rb +++ b/spec/models/cycle_analytics/production_spec.rb @@ -1,28 +1,31 @@ require 'spec_helper' describe 'CycleAnalytics#production', feature: true do + extend CycleAnalyticsHelpers::TestGeneration + let(:project) { create(:project) } let(:from_date) { 10.days.ago } let(:user) { create(:user, :admin) } subject { CycleAnalytics.new(project, from: from_date) } - generate_cycle_analytics_spec(phase: :production, - data_fn: -> (context) { { issue: context.build(:issue, project: context.project) } }, - start_time_conditions: [["issue is created", -> (context, data) { data[:issue].save }]], - before_end_fn: lambda do |context, data| - context.create_merge_request_closing_issue(data[:issue]) - context.merge_merge_requests_closing_issue(data[:issue]) - end, - end_time_conditions: - [["merge request that closes issue is deployed to production", -> (context, data) { context.deploy_master }], - ["production deploy happens after merge request is merged (along with other changes)", - lambda do |context, data| - # Make other changes on master - sha = context.project.repository.commit_file(context.user, context.random_git_name, "content", "commit message", 'master', false) - context.project.repository.commit(sha) - - context.deploy_master - end]]) + generate_cycle_analytics_spec( + phase: :production, + data_fn: -> (context) { { issue: context.build(:issue, project: context.project) } }, + start_time_conditions: [["issue is created", -> (context, data) { data[:issue].save }]], + before_end_fn: lambda do |context, data| + context.create_merge_request_closing_issue(data[:issue]) + context.merge_merge_requests_closing_issue(data[:issue]) + end, + end_time_conditions: + [["merge request that closes issue is deployed to production", -> (context, data) { context.deploy_master }], + ["production deploy happens after merge request is merged (along with other changes)", + lambda do |context, data| + # Make other changes on master + sha = context.project.repository.commit_file(context.user, context.random_git_name, "content", "commit message", 'master', false) + context.project.repository.commit(sha) + + context.deploy_master + end]]) context "when a regular merge request (that doesn't close the issue) is merged and deployed" do it "returns nil" do diff --git a/spec/models/cycle_analytics/review_spec.rb b/spec/models/cycle_analytics/review_spec.rb index 100ce11299a..b6e26d8f261 100644 --- a/spec/models/cycle_analytics/review_spec.rb +++ b/spec/models/cycle_analytics/review_spec.rb @@ -1,16 +1,25 @@ require 'spec_helper' describe 'CycleAnalytics#review', feature: true do + extend CycleAnalyticsHelpers::TestGeneration + let(:project) { create(:project) } let(:from_date) { 10.days.ago } let(:user) { create(:user, :admin) } subject { CycleAnalytics.new(project, from: from_date) } - generate_cycle_analytics_spec(phase: :review, - data_fn: -> (context) { { issue: context.create(:issue, project: context.project) } }, - start_time_conditions: [["merge request that closes issue is created", -> (context, data) { context.create_merge_request_closing_issue(data[:issue]) }]], - end_time_conditions: [["merge request that closes issue is merged", -> (context, data) { context.merge_merge_requests_closing_issue(data[:issue]) }]], - post_fn: -> (context, data) { context.deploy_master }) + generate_cycle_analytics_spec( + phase: :review, + data_fn: -> (context) { { issue: context.create(:issue, project: context.project) } }, + start_time_conditions: [["merge request that closes issue is created", + -> (context, data) do + context.create_merge_request_closing_issue(data[:issue]) + end]], + end_time_conditions: [["merge request that closes issue is merged", + -> (context, data) do + context.merge_merge_requests_closing_issue(data[:issue]) + end]], + post_fn: -> (context, data) { context.deploy_master }) context "when a regular merge request (that doesn't close the issue) is created and merged" do it "returns nil" do diff --git a/spec/models/cycle_analytics/staging_spec.rb b/spec/models/cycle_analytics/staging_spec.rb index 105f566f41c..af1c4477ddb 100644 --- a/spec/models/cycle_analytics/staging_spec.rb +++ b/spec/models/cycle_analytics/staging_spec.rb @@ -1,26 +1,41 @@ require 'spec_helper' describe 'CycleAnalytics#staging', feature: true do + extend CycleAnalyticsHelpers::TestGeneration + let(:project) { create(:project) } let(:from_date) { 10.days.ago } let(:user) { create(:user, :admin) } subject { CycleAnalytics.new(project, from: from_date) } - generate_cycle_analytics_spec(phase: :staging, - data_fn: lambda do |context| - issue = context.create(:issue, project: context.project) - { issue: issue, merge_request: context.create_merge_request_closing_issue(issue) } - end, - start_time_conditions: [["merge request that closes issue is merged", -> (context, data) { context.merge_merge_requests_closing_issue(data[:issue])} ]], - end_time_conditions: [["merge request that closes issue is deployed to production", -> (context, data) { context.deploy_master }], - ["production deploy happens after merge request is merged (along with other changes)", - lambda do |context, data| - # Make other changes on master - sha = context.project.repository.commit_file(context.user, context.random_git_name, "content", "commit message", 'master', false) - context.project.repository.commit(sha) - - context.deploy_master - end]]) + generate_cycle_analytics_spec( + phase: :staging, + data_fn: lambda do |context| + issue = context.create(:issue, project: context.project) + { issue: issue, merge_request: context.create_merge_request_closing_issue(issue) } + end, + start_time_conditions: [["merge request that closes issue is merged", + -> (context, data) do + context.merge_merge_requests_closing_issue(data[:issue]) + end ]], + end_time_conditions: [["merge request that closes issue is deployed to production", + -> (context, data) do + context.deploy_master + end], + ["production deploy happens after merge request is merged (along with other changes)", + lambda do |context, data| + # Make other changes on master + sha = context.project.repository.commit_file( + context.user, + context.random_git_name, + "content", + "commit message", + 'master', + false) + context.project.repository.commit(sha) + + context.deploy_master + end]]) context "when a regular merge request (that doesn't close the issue) is merged and deployed" do it "returns nil" do diff --git a/spec/models/cycle_analytics/test_spec.rb b/spec/models/cycle_analytics/test_spec.rb index 79edc29c173..89ace0b2742 100644 --- a/spec/models/cycle_analytics/test_spec.rb +++ b/spec/models/cycle_analytics/test_spec.rb @@ -1,26 +1,27 @@ require 'spec_helper' describe 'CycleAnalytics#test', feature: true do + extend CycleAnalyticsHelpers::TestGeneration + let(:project) { create(:project) } let(:from_date) { 10.days.ago } let(:user) { create(:user, :admin) } subject { CycleAnalytics.new(project, from: from_date) } - generate_cycle_analytics_spec(phase: :test, - data_fn: lambda do |context| - issue = context.create(:issue, project: context.project) - merge_request = context.create_merge_request_closing_issue(issue) - { - pipeline: context.create(:ci_pipeline, ref: merge_request.source_branch, sha: merge_request.diff_head_sha, project: context.project), - issue: issue - } - end, - start_time_conditions: [["pipeline is started", -> (context, data) { data[:pipeline].run! }]], - end_time_conditions: [["pipeline is finished", -> (context, data) { data[:pipeline].succeed! }]], - post_fn: -> (context, data) do - context.merge_merge_requests_closing_issue(data[:issue]) - context.deploy_master - end) + generate_cycle_analytics_spec( + phase: :test, + data_fn: lambda do |context| + issue = context.create(:issue, project: context.project) + merge_request = context.create_merge_request_closing_issue(issue) + pipeline = context.create(:ci_pipeline, ref: merge_request.source_branch, sha: merge_request.diff_head_sha, project: context.project) + { pipeline: pipeline, issue: issue } + end, + start_time_conditions: [["pipeline is started", -> (context, data) { data[:pipeline].run! }]], + end_time_conditions: [["pipeline is finished", -> (context, data) { data[:pipeline].succeed! }]], + post_fn: -> (context, data) do + context.merge_merge_requests_closing_issue(data[:issue]) + context.deploy_master + end) context "when the pipeline is for a regular merge request (that doesn't close an issue)" do it "returns nil" do diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb index a0614abcf62..ff6bad6aaba 100644 --- a/spec/services/merge_requests/create_service_spec.rb +++ b/spec/services/merge_requests/create_service_spec.rb @@ -108,7 +108,7 @@ describe MergeRequests::CreateService, services: true do allow(service).to receive(:execute_hooks) merge_request = service.execute - expect(merge_request.issues_closed).to match_array([first_issue, second_issue]) + expect(merge_request.reload.closes_issues).to match_array([first_issue, second_issue]) end end end diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index 7ecad8d6bad..22cb122526c 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -200,7 +200,7 @@ describe MergeRequests::RefreshService, services: true do allow(refresh_service).to receive(:execute_hooks) refresh_service.execute(@oldrev, @newrev, 'refs/heads/feature') - expect(merge_request.reload.issues_closed).to eq([issue]) + expect(merge_request.reload.closes_issues).to eq([issue]) end end diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 84b0acffaac..68dd0382a3d 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -274,7 +274,7 @@ describe MergeRequests::UpdateService, services: true do allow(service).to receive(:execute_hooks) service.execute(merge_request) - expect(merge_request.reload.issues_closed).to match_array([first_issue, second_issue]) + expect(merge_request.reload.closes_issues).to match_array([first_issue, second_issue]) end it 'removes `MergeRequestsClosingIssues` records when issues are not closed anymore' do @@ -288,13 +288,13 @@ describe MergeRequests::UpdateService, services: true do merge_request = MergeRequests::CreateService.new(project, user, opts).execute - expect(merge_request.issues_closed).to match_array([first_issue, second_issue]) + expect(merge_request.reload.closes_issues).to match_array([first_issue, second_issue]) service = described_class.new(project, user, description: "not closing any issues") allow(service).to receive(:execute_hooks) service.execute(merge_request.reload) - expect(merge_request.reload.issues_closed).to be_empty + expect(merge_request.reload.closes_issues).to be_empty end end end diff --git a/spec/support/cycle_analytics_helpers/test_generation.rb b/spec/support/cycle_analytics_helpers/test_generation.rb index 4d5ce86691f..8e19a6c92e2 100644 --- a/spec/support/cycle_analytics_helpers/test_generation.rb +++ b/spec/support/cycle_analytics_helpers/test_generation.rb @@ -159,7 +159,3 @@ module CycleAnalyticsHelpers end end end - -RSpec.configure do |config| - config.extend CycleAnalyticsHelpers::TestGeneration -end |